feat: merge-train/spartan#23714
Merged
Merged
Conversation
## Motivation `ChonkProof.empty()` returned an array of `CHONK_PROOF_LENGTH` (1632) `Fr.ZERO` fields, which serializes to ~52KB. Empty proofs are only ever placeholders (simulated txs, the fuzzer, the txe, p2p testbench, and proof-stripping for tx archival), so carrying 1632 zeros around — and persisting them to the archive — is pure waste. The goal is for an empty proof to actually be empty. ## Approach `empty()` now holds an empty `Fr[]`, and serialization gains a third format: a leading `uint32` of `0` encodes an empty proof. On read, `0` is recognized as the empty case, `CHONK_PROOF_LENGTH` (1632) stays the legacy field-count format, and any other value remains the compressed byte count. A real compressed proof always has a non-zero byte count, so `0` is an unambiguous sentinel. The constructors accept length `0`, and the public-input helpers guard the empty case so the `length - 1632` math can't go negative. ## Changes - **stdlib (proofs)**: `ChonkProof`/`ChonkProofWithPublicInputs` `empty()` hold `[]`; constructors allow length `0`; `toBuffer`/`fromBuffer` handle the zero-length format; `getPublicInputs`/`removePublicInputs` guard empty. Empty proofs now round-trip in 4 bytes instead of ~52KB. - **stdlib (tests)**: Added assertions that empty proofs hold `[]`, serialize to a single zero length, and round-trip for both classes. All proving/verification paths (rollup `RecursiveProof` construction, the bb verifiers, PXE `removePublicInputs`) only ever receive real proofs from bb with the full field count, so they are unaffected; the only path empty proofs traverse is serialization round-trips (tx archive, tx clone, JSON-RPC).
Adds a missing e2e test to the l2-to-l1 suite to test that two blocks with outgoing messages in the same checkpoint are processed correctly. Also guards against setup flakes by anchoring the pxe to the checkpointed chain (see [this flaked run](http://ci.aztec-labs.com/448641256e72540b))
We were hitting flakes caused by a checkpoint proposal job building for slot N+2 during slot N. Example from [this failed run](http://ci.aztec-labs.com/02e1d4a8c9c9d0c9): ``` 417:15:00:04 [15:00:04.669] INFO sequencer node-0 Preparing checkpoint proposal 7 for target slot 8 during wall-clock slot 6 { "nowSeconds":"1780066811", "syncedToL2Slot":6, "slot":6, "targetSlot":8, "slotTs":"1780066804", "checkpointNumber":7, "hasProposedCheckpoint":true, "proposedCheckpointNumber":6, "checkpointedCheckpointNumber":5, "pipeliningEnabled":true } ``` This could be explained by the two non-atomic calls for getting the current and target slot in the sequencer, which could return N and N+2 if they happened at a slot boundary. This PR changes it so we issue a single call and just derive the target slot from the current one based on pipelining offset.
…k` (#23720) Fixes flake in epochs_invalidate_block test "archiver skips a descendant of an invalid-attestations checkpoint" when one of the malicious nodes was picked as proposer when we expected the chain to advance successfully. Codex analysis below. **Test Summary** `archiver skips a descendant of an invalid-attestations checkpoint` verifies that when P1 posts a checkpoint with insufficient attestations and P2 posts a valid-attestation descendant, archivers reject both, emit `DescendentOfInvalidAttestationsCheckpointDetected`, and the chain later recovers past P2. **Failed Run Error** CI `a4dc267449e28279` timed out at: `TimeoutError: Timeout awaiting archiver advances past P2` **Failed vs Successful Divergence** Both runs correctly skipped P1 and P2. The divergence came afterward: in the failed run, P1 stayed malicious and got scheduled again, publishing more invalid checkpoints before the observer reached checkpoint `3`. In the passing run, healthy proposers recovered the chain first. **Timeline** Failed: - `20:55:51`: P2 skipped as descendant of rejected checkpoint 1. - `20:57:27`: P1 publishes another invalid checkpoint 1. - `20:59:03`: another checkpoint 2 is skipped for insufficient attestations. - `21:00:31`: timeout waiting for checkpointed tip `>= p2Checkpoint + 1`. Successful: - `20:43:25`: P2 skipped as descendant of rejected checkpoint 1. - `20:45:01`: replacement checkpoint 1 downloaded. - `20:45:33`: checkpoint 2 downloaded. - `20:46:05`: checkpoint 3 downloaded; test succeeds. **Hypothesis** High confidence: the test restored only P2 after confirming P1’s bad checkpoint, leaving P1’s `skipCollectingAttestations` and `skipInvalidateBlockAsProposer` config active. If P1 is scheduled again during recovery, the test waits on an accidental scheduling race rather than the archiver behavior. **Evidence** - Logs: failed run shows P1 republishing invalid checkpoints after the intended P1/P2 scenario; passing run reaches checkpoints 1, 2, and 3 before P1 can interfere. - Source: the test intended only P1/P2 to be malicious, then recovery is checked in [epochs_invalidate_block.parallel.test.ts](/home/santiago/Projects/aztec-2/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts:661). Archiver rejected-ancestor behavior is in [l1_synchronizer.ts](/home/santiago/Projects/aztec-2/yarn-project/archiver/src/modules/l1_synchronizer.ts:879). - Skeptic check: confirmed the fix does not weaken the test; keeping P1 malicious after the assertion only tests proposer-schedule luck. **Proposed Fix** Updated [epochs_invalidate_block.parallel.test.ts](/home/santiago/Projects/aztec-2/yarn-project/end-to-end/src/e2e_epochs/epochs_invalidate_block.parallel.test.ts:667) to restore both bad proposers before waiting for chain progress: - P1: `skipCollectingAttestations: false`, `skipInvalidateBlockAsProposer: false` - P2: `skipWaitForValidParentCheckpointOnL1: false`, `skipInvalidateBlockAsProposer: false` **Verification** Passed: ```bash LOG_LEVEL='info; debug:sequencer,archiver,publisher,validator' ANVIL_PORT=8564 yarn workspace @aztec/end-to-end test:e2e e2e_epochs/epochs_invalidate_block.parallel.test.ts -t 'archiver skips a descendant of an invalid-attestations checkpoint' ``` Result: `1 passed, 8 skipped`.
…23721) ## Problem The `Merge branch 'next' into merge-train/spartan` CI run ([ci.aztec-labs.com/1780091707705535](http://ci.aztec-labs.com/1780091707705535)) failed in the `cd l1-contracts && forge test` step: ``` test/staking/setSlasher.t.sol:SetSlasherTest [FAIL: unknown selector `0xbffa7f0f` for ConsoleCalls; counterexample: args=[0x000000000000000000636F6e736F6c652e6c6f67]] test_finalizeSetSlasher_appliesAfterDelay(address) (runs: 104) ``` ## Root cause `0x000000000000000000636F6e736F6c652e6c6f67` is forge-std's console address (ASCII "console.log"). The fuzzer drew it as the `_newSlasher` parameter. Foundry intercepts **every** call to that address and dispatches it to its console handler, which bypasses `vm.mockCall`. So the staking contract's `PROPOSER()` lookup (selector `0xbffa7f0f`) on the fuzzed slasher reverted with "unknown selector for ConsoleCalls" instead of returning the mocked value. Every fuzz test in this file that mocks a slasher and then calls into it shares the same latent flake — `test_finalizeSetSlasher_appliesAfterDelay` is just the one the fuzzer happened to hit this run. ## Fix Call `assumeNotForgeAddress` (forge-std, inherited via `Test`) to exclude the vm / console / Create2Deployer reserved addresses: - in the shared `_mockInitializedSlasher` helper (covers all tests that mock through it), and - in `test_queueSetSlasher_revertsWhenProposerUnset`, which mocks the slasher inline. ## Verification Reproduced the exact CI failure locally by forcing the console address through the original (unfixed) flow, then confirmed the fix: - `forge test --match-contract SetSlasherTest --fuzz-runs 512` → 12 passed, 0 failed - Full `cd l1-contracts && forge test` → **849 passed, 0 failed, 3 skipped** (was 848 passed / 1 failed before the fix) Targeting the `merge-train/spartan` branch directly per the merge-trains runbook (Option 1: direct fix). --- *Created by [claudebox](https://claudebox.work/v2/sessions/0413f464208ab06d) · group: `slackbot`*
Collaborator
Author
|
🤖 Auto-merge enabled after 4 hours of inactivity. This PR will be merged automatically once all checks pass. |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Collaborator
Author
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
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
4 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.
BEGIN_COMMIT_OVERRIDE
feat(stdlib): make empty ChonkProof hold an empty array (#23712)
test(e2e): l2-to-l1 mbps and anchor to checkpointed chain (#23717)
fix(sequencer): atomically query current and target slots (#23716)
test(e2e): restore both malicious nodes to honest in
invalidate_block(#23720)test: exclude forge reserved addresses from setSlasher fuzz inputs (#23721)
END_COMMIT_OVERRIDE