fix(p2p): fail node startup when p2p service fails to start - #25177
Merged
Conversation
This was referenced Aug 11, 2026
spalladino
added
port-to-next
When the p2p client starts while there are blocks to sync, the libp2p service start is deferred to `startServiceIfSynched`, which runs inside the block stream event handler. That handler catches and only logs errors, so a TCP bind failure (e.g. `ERR_NO_VALID_ADDRESSES` after a restart) was swallowed. On top of that, the sync promise awaited by node creation was resolved before the service was started, so startup succeeded regardless and the node kept running with a dead p2p stack and zero peers. The service is now started before the sync promise settles, and a failure logs at fatal level and rejects the stored sync promise (rather than throwing, which the block stream would eat) so node creation fails and the process exits, letting the orchestrator restart it. Part of A-1701.
spalladino
force-pushed
the
spl/p2p-fatal-bind-failure
branch
from
August 12, 2026 12:33
2ecd150 to
ecd0f11
Compare
Collaborator
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
fcarreiro
approved these changes
Aug 12, 2026
spalladino
added a commit
that referenced
this pull request
Aug 12, 2026
) A validator could run for hours with a dead libp2p stack - zero peers and zero gossip - without anything in its logs flagging the condition. The failure was only reconstructable after the fact from the absence of activity, rather than from any explicit signal. The peer manager heartbeat now tracks consecutive zero-peer heartbeats. After 3 of them (so startup and transient dips stay quiet) it logs a warning that the node has no connected peers and can neither gossip nor propagate txs, with the zero-peer heartbeat count, the heartbeat interval, time since a peer was last connected, and the cached peer count. The warning then repeats about once a minute, derived from peerCheckIntervalMS, instead of on every heartbeat. When peers come back, a single info log reports connectivity restored and the new peer count. Connected peers are counted via libp2p getPeers(), matching what getP2PConnectivity reports. A peer count gauge already existed (PEER_MANAGER_PEER_COUNT, recorded in discover()), so no metric was added - this change is logs only. Part of A-1701. --- Related PRs from the same incident (all independent, all targeting `merge-train/spartan-v5`): #25177 (fail startup when the p2p service fails to start), #25185 (connectivity signal + slasher/proposer/health/sendTx gates), #25183 (periodic zero-peers warning).
Merged
spalladino
added a commit
that referenced
this pull request
Aug 14, 2026
…tivity (#25185) Gates p2p-dependent operations on actual gossip connectivity, so a node running with a dead libp2p stack (zero peers) can no longer file false slashing accusations, burn its proposer slots, or silently blackhole txs. Follows an incident where a validator whose TCP listener failed to bind kept running peerless and voted `DATA_WITHHOLDING` against entire committees. One commit per concern (squashed on merge): 1. **`feat(p2p): expose p2p connectivity`** — new `getP2PConnectivity(): { enabled, connectedPeers }` on the p2p service, client, and node RPC API. The dummy (p2p-disabled) implementation reports `enabled: false`, so sandbox and single-node setups are vacuously healthy for every gate below. 2. **`fix(slasher): do not file data-withholding offenses while peerless`** — the data-withholding watcher infers offenses from the *absence* of txs in the local pool, which is invalid evidence without gossip connectivity. It now permanently skips (never backfills) slots probed while p2p is enabled with zero peers — after reconnecting, mined txs may already be evicted from the pool, so a late probe would still false-positive. 3. **`fix(sequencer): skip proposing when node has no connected peers`** — a peerless proposer broadcasts into the void (burns the slot) or, with enough own committee seats, publishes a checkpoint whose data nobody has. It now skips building below `minPeersToPropose` (default 1, `SEQ_MIN_PEERS_TO_PROPOSE`, 0 disables) while keeping L1 duties (votes, prune, invalidation) via the existing fallback. 4. **`feat(node): report per-component health on GET /status`** — `/status` now returns `{ ok, components: { p2p: { healthy, enabled, connectedPeers }, ... } }`. The `p2pHealthMinPeers` floor (`P2P_HEALTH_MIN_PEERS`) defaults to 0 (report-only) so fresh-network bootstrap nodes aren't failed by default; operators opt in. 5. **`fix(node): reject sendTx when node has no peers`** — instead of accepting a tx and gossiping it to nobody, the node rejects immediately so callers can retry against a healthy node. 6. **`test: keep single-node mock-gossip e2e working`** — `registerPhantomGossipPeer` test helper so the one single-node e2e test on the in-memory mock gossip bus presents one connected peer to the gates. 7. **`fix(slasher): do not file data-withholding offenses when p2p is disabled`** — a node with no p2p stack at all sees strictly less than a peerless one, so it must not slash either. The runtime gate drops `enabled` (any node with zero peers skips), and the watcher isn't constructed when `p2pEnabled` is false. Unlike the other gates, this one infers guilt from *absence*, which is never valid without gossip; the propose/`/status`/`sendTx` gates keep treating `enabled: false` as vacuously fine because they gate the ability to *act*, which a deliberately p2p-less node has by design. Strongly suggest to **review commit by commit**. Related PRs (independent, same incident): #25177 (fail node startup when the p2p service fails to start), #25183 (periodic zero-peers warning). Part of A-1701.
aminsammara
added a commit
that referenced
this pull request
Aug 17, 2026
Promotes `v5-next` onto `v5` for the **v5.2.0** release. Frozen at `ee5d2d367e` — the `v5-next` tip at cut time. Both `merge-train/spartan-v5` and `merge-train/fairies-v5` are drained to that same commit, so nothing staged is left behind. ### Testnet validation This branch was cut at `a4db216abf`, which is byte-identical to the `v5.2.0-nightly.20260815` tag — same commit, same tree (`0b22572eace2419ede6bbab173514d6965d4e0d0`). That nightly's CI3 run is green, its artifacts are published to npm and Docker Hub, and it has been **running healthily on testnet since 2026-08-15**: | Component | Image / reported version | |---|---| | validators, prover node, prover broker, prover agents (`testnet` ns) | `aztecprotocol/aztec:5.2.0-nightly.20260815` | | `v5.testnet.rpc.aztec-labs.com` | `nodeVersion=5.2.0-nightly.20260815` | | `canonical.testnet.rpc.aztec-labs.com` | `nodeVersion=5.2.0-nightly.20260815` | Chain advancing normally, no prunes or reorgs, no WARN/ERROR across node pods since rollout. The branch has since been fast-forwarded to `ee5d2d367e` to pick up #25242 (configurable RPC server HTTP timeouts and CORS allowed-headers). That is the only delta from the soaked tree — 8 files, +131/−10, no nightly covers it yet. Its config defaults were checked against Node's built-ins (`keepAliveTimeout` 5000 ms, `headersTimeout` 60000 ms) and match exactly, and the CORS default path resolves to the same `cors()` call as before, so a node that sets none of the new env vars behaves identically. It touches no circuits, protocol contracts, or generated constants. ### Manifest `.release-please-manifest.json` reads `5.2.0` on this branch, which is the released version — correct as-is, no change needed in this PR. `v5-next` moves to `5.3.0` separately in #25240; this branch is frozen and cannot pick that up, so the two can merge in either order. `v5` is a strict ancestor of `v5-next` this cycle, so there was no manifest conflict to pre-resolve. ### Scope 139 commits (86 non-merge, 23 PR-level) spanning 2026-07-14 to 2026-08-17. | Area | Theme | |---|---| | `prover-node` / `prover-client` | Epoch-proving robustness: retry-to-converge with failure declared only at submission-window expiry, ticker-driven expiry, per-checkpoint post-mortem upload, checkpoint-only re-proving, prune-induced fault handling (#24678, #24982, #24983, #24990, #25027) | | `pxe` | Sync performance: hash-pinned node read cache (#24969), anchor-bounded tag log caching (#25074), note/event validation from cached tx data (#25076), constrained tag sync (#24275), sender tagging finalization from log blocks (#25045) | | `p2p` / `validator-client` | Gossip tx validation no longer stalls behind tx-pool finalization (#25148); startup fails when p2p fails to start (#25177); slashing/proposing/health/`sendTx` gated on p2p connectivity (#25185); duplicate time-sensitive proposal validation removed (#25207); `ValidatedProposal` branding (#25222) | | `ethereum` / `aztec-node` | L1 watchers poll `getLogs` instead of `eth_newFilter` (#25176); block stream sync no longer resolves against an earlier pass (#25206) | | `stdlib` / `foundation` | Deserialization bounds hardening (#25026, #25028, #25029, #25109); checkpoint block-shape and block-count validation (#25229); JSON-RPC cookies (#25231) | | `archiver` | Removed-block cleanup and ownership-checked tx-effect deletes (#24765); L2→L1 witness from a single store snapshot (#24754) | | `aztec` | Declarative deployment framework at `@aztec/aztec/deploy` (#24685) | | `slasher` | Own-validator slash-target warnings and metrics (#25058) | | telemetry | JSON-RPC metrics (#25159) | | JSON-RPC server | Configurable HTTP keep-alive / headers timeouts and CORS allowed-headers, defaults preserving current behaviour (#25242) | | toolchain | Noir bumped to `v1.0.0-beta.25` (#24907) | **One breaking change**, inherited from the Noir bump: note types declared directly inside a `contract` module must now be `pub`. Everything else is additive or internal. ### Protocol constants Built from source and compared against `v5` — a cache-free rebuild of every protocol circuit with each ref's own nargo and bb, then regeneration of `vk_tree.ts` and `protocol_contract_data.ts`: - `vkTreeRoot` = `0x2b3b6ea4412b9c8f6457a37f91a2870306f8641e07e16a49b68bda6f8bc02892` — unchanged from v5.1.0 - `protocolContractsHash` = `0x2c075866eafc88a1f6f9addc7e337c6e64e45d1cb7fd7c0d612ebcec72aab2ca` — unchanged from v5.1.0 The Noir beta.24 → beta.25 bump does not reach the circuits: the release build consumes the committed `pinned-build.tar.gz`, which is bit-identical between v5.1.0 and this commit (blob `3bedcb1fd1…`), so the protocol-circuit bytecode is frozen rather than recompiled. The 47 verification keys were recomputed locally from that pinned bytecode with the cache disabled, and all 47 `check_pinned_vk` checks passed. `protocolContractsHash` is likewise backed by the new `pinned-protocol-contracts.tar.gz`, whose three artifacts were byte-compared against the build. Both values were also confirmed inside the published `@aztec/protocol-contracts` and `@aztec/noir-protocol-circuits-types` packages for `5.2.0-nightly.20260815`. That makes v5.2.0 a drop-in upgrade against the current rollup rather than a coordinated one. ✅ **Gate re-run against `ee5d2d367e` (the current head) and passed** — both values reproduced exactly from a build at this commit, with `vkTreeRoot` numerically evaluated rather than inferred. `29556326ce..ee5d2d3` touches no `noir-projects/**`, `l1-contracts/src/**`, `ConstantsGen.sol`, `constants.gen.ts` or `constants.nr`. Scope of the check: it verifies that the pinned circuit bytecode plus locally recomputed VKs agree with the pin — not that a from-source recompile reproduces v5.1.0's bytecode. <details> <summary>Commits added after the original constants check at 2955632</summary> `#25207` `#25222` `#25206` `#25185` `#25176` `#25163` `#25229` `#25159` `#25162` `#25231` `#25224` `#25228` `#25230` `#25242` plus their merge commits. </details> ### Known gap Migration notes carry entries under `## TBD` that arrived with the docs baseline backport (#25017) and describe `next`-line changes not present on the v5 line — protocol contracts removed from `@aztec/noir-contracts.js`, and the `at(wallet)` → `withWallet(wallet)` deprecation. Neither exists on this branch. There is also no `## 5.1.0` heading, and the entries that ship in v5.2.0 sit under `## TBD` rather than a version heading. This ships in the release docs, so it is worth correcting on `v5-next` and backporting to `v5` before the tag, rather than after.
spalladino
added a commit
that referenced
this pull request
Aug 18, 2026
Forward-ports the fixes merged into `merge-train/spartan-v5` over the last two weeks onto the `next` line. One commit per original PR, each carrying a `cherry picked from` trailer. ## Ported - #25177 — fix(p2p): fail node startup when p2p service fails to start - #25183 — feat(p2p): warn periodically while node has zero connected peers - #25202 — fix: spurious insufficient-funds rejection when simulating L1 calls - #25207 — fix(validator): remove duplicate time-sensitive validation of block proposals - #25222 — refactor(p2p): brand ingress-validated proposals with a ValidatedProposal type - #25229 — fix: checkpoint block-shape and block-count validation gaps - #25206 — fix: block stream sync can resolve against a pass that predates it - #25185 — feat(p2p): gate slashing, proposing, health, and sendTx on p2p connectivity - #25176 — fix: poll getLogs instead of eth_newFilter for L1 event watchers Commits are ordered as they merged into the v5 line, since several build on each other (#25177 → #25183 → #25185 and #25207 → #25222 → #25229). ## Not ported - #25148 (gossip tx validation stalls behind tx pool finalization) — already on this line via #25189, which covers the same ground. The cherry-pick came out empty. - #25155 (standard-contracts historical artifacts in release image) — the whitelist entry is already present in `release-image/Dockerfile.dockerignore`. ## Conflict resolutions worth a look Four commits conflicted; all others applied clean. - **`ethereum/src/l1_tx_utils/l1_tx_utils.ts`** (#25202) — the rename #25202 called out in its own description: `getGasPrice` is `getFeesPerGas` on this line. Took the fix's side (fee fields dropped from the simulated call), so the local `feesPerGas` read is gone along with them. - **`archiver/src/modules/data_store_updater.ts`** (#25229) — the v5 version of this method also passes `maxBlocksPerCheckpoint: MAX_CAPACITY_BLOCKS_PER_CHECKPOINT` on the ingest path, which is a separate v5-only change that has not been ported. Only #25229's actual change is taken here: the shared `validateOpts` object plus `allowEmptyNonFirstBlocks: true`. Ingest on this line therefore still validates against the attestable limit; raising it to the capacity limit is a separate port. - **`ethereum/src/contracts/slashing_proposer.ts`** (#25176) — `listenToVoteCast` carries a `voteIndex` argument on this line (from #25068), which the v5 version predates. Combined both: the new `watchContractEvent` polling mechanism with the `voteIndex` callback argument retained. - **Import blocks** (#25185) — `safe_json_rpc_server.test.ts` keeps `import http` as a value import, since #25243 added a runtime `http.createServer()` call to that file. `stdlib/src/interfaces/p2p.test.ts` takes only `P2PConnectivity`; the `TxHash` / `GetTxByHashOptions` imports on the v5 side belong to v5-only API methods and would be unused here. ## Verification `yarn build` produces a byte-identical error set to `origin/merge-train/spartan` built in the same working tree (46 errors, all from stale cross-line generated artifacts locally — Noir circuit artifacts, verification-key lengths, `withWallet` on contract types). No new compile errors from the port. Unit tests for every package with a hand-resolved conflict pass: `stdlib` (52), `ethereum` (61), `foundation` (47), `slasher` (140), `validator-client` (39). Two suites (`archiver/data_store_updater.test.ts`, `sequencer-client/checkpoint_proposal_job.test.ts`) fail to *load* in this working tree on stale cross-line artifacts; the archiver one was confirmed to fail identically with the base version of the file, so CI is the first real run for those two. Labeled `ci-no-squash` to preserve one commit per ported PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the p2p client starts while there are blocks to sync, the libp2p service start is deferred to
startServiceIfSynched, which runs inside the block stream event handler. That handler catches andonly logs errors, so a TCP bind failure (e.g.
ERR_NO_VALID_ADDRESSESafter a restart) was swallowed.On top of that, the sync promise awaited by node creation was resolved before the service was started,
so startup succeeded regardless and the node kept running with a dead p2p stack and zero peers.
The service is now started before the sync promise settles, and a failure logs at fatal level and
rejects the stored sync promise (rather than throwing, which the block stream would eat) so node
creation fails and the process exits, letting the orchestrator restart it.
Part of A-1701.
Related PRs from the same incident (all independent, all targeting
merge-train/spartan-v5): #25177 (fail startup when the p2p service fails to start), #25185 (connectivity signal + slasher/proposer/health/sendTx gates), #25183 (periodic zero-peers warning).