fix(fast-inbox): reject propose against a still-mutable inbox bucket (A-1378)

A bucket keeps absorbing messages for the whole L1 block that opened it, so a
reference to the current bucket is not a reference to any lasting snapshot. A lone
designated proposer could bundle send(A) -> propose(hash-after-A) -> send(B) into one
L1 transaction: L1 accepts the checkpoint, then the trailing send overwrites the
bucket in place, leaving the checkpoint committed to a rolling hash that exists
neither on L1 nor in any node, since nodes only ever observe a bucket's end-of-block
state. No honest node can resolve the consumed position, so the pending chain stalls
until the checkpoint is pruned.

validateInboxConsumption now requires the referenced bucket to be settled, which is
the negation of the Inbox's own rollover condition: the genesis bucket (which never
absorbs), a bucket whose L1 block has passed, or a full bucket whose successor takes
the next message. The per-bucket cap moves to IInbox.sol so the Inbox and the propose
path share one definition without the rollup libraries depending on the Inbox
implementation. Honest proposers never reference a bucket younger than
INBOX_LAG_SECONDS, so nothing legitimate is excluded.

Author: spalladino

l1-contracts/src/core/interfaces/messagebridge/IInbox.sol

@@ -4,6 +4,11 @@ pragma solidity >=0.8.27;
 
 import {DataStructures} from "../../libraries/DataStructures.sol";
 
+// Maximum number of messages a single bucket can hold before further messages in the same L1 block spill over
+// into the next bucket. Matches the number of L1 to L2 messages a single L2 block can insert once the streaming
+// inbox is live, so any one bucket is always consumable by one block.
+uint256 constant MAX_MSGS_PER_BUCKET = 256;
+
 /**
  * @title Inbox
  * @author Aztec Labs

l1-contracts/src/core/libraries/Errors.sol

@@ -60,6 +60,7 @@ library Errors {
   error Rollup__InvalidCheckpointNumber(uint256 expected, uint256 actual); // 0xd1ba9bfa
   error Rollup__InvalidInHash(bytes32 expected, bytes32 actual); // 0xcd6f4233
   error Rollup__InvalidInboxRollingHash(bytes32 expected, bytes32 actual); // 0xed1f7bb5
+  error Rollup__InboxBucketStillMutable(uint256 bucketSeq); // 0x7254b980
   error Rollup__UnconsumedInboxMessages(uint256 nextBucketSeq); // 0x2bd4bf10
   error Rollup__InboxConsumptionBehindParent(uint256 expected, uint256 actual); // 0x54e0c025
   error Rollup__TooManyInboxMessagesConsumed(uint256 consumed); // 0xf76d1426

l1-contracts/src/core/libraries/rollup/ProposeLib.sol

@@ -5,7 +5,7 @@ pragma solidity >=0.8.27;
 import {BlobLib} from "@aztec-blob-lib/BlobLib.sol";
 import {IEscapeHatch} from "@aztec/core/interfaces/IEscapeHatch.sol";
 import {RollupStore, IRollupCore, CheckpointHeaderValidationFlags} from "@aztec/core/interfaces/IRollup.sol";
-import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
+import {IInbox, MAX_MSGS_PER_BUCKET} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
 import {TempCheckpointLog} from "@aztec/core/libraries/compressed-data/CheckpointLog.sol";
 import {FeeHeader} from "@aztec/core/libraries/compressed-data/fees/FeeStructs.sol";
 import {ChainTipsLib, CompressedChainTips} from "@aztec/core/libraries/compressed-data/Tips.sol";
@@ -389,12 +389,14 @@ library ProposeLib {
    *         bucket referenced by `_bucketHint`. The hint is a plain calldata lookup aid, not signed and not
    *         part of the header: a wrong hint cannot change what gets accepted, it only reverts. A checkpoint
    *         that consumes no messages references the same bucket as its parent.
-   *      2. Consumption moves forward: the referenced bucket's cumulative total must be at least the parent
+   *      2. The referenced bucket must be settled: a bucket that can still absorb another message is not a
+   *         snapshot of anything.
+   *      3. Consumption moves forward: the referenced bucket's cumulative total must be at least the parent
    *         checkpoint's (equal consumes nothing; behind is a hard revert). This precedes the subtractions
    *         below, which rely on `bucket.totalMsgCount >= _parentTotalMsgCount` to not underflow.
-   *      3. Cap upper bound: a single checkpoint cannot consume more than MAX_L1_TO_L2_MSGS_PER_CHECKPOINT
+   *      4. Cap upper bound: a single checkpoint cannot consume more than MAX_L1_TO_L2_MSGS_PER_CHECKPOINT
    *         messages, the maximum the circuits can insert.
-   *      4. Mandatory consumption (the censorship assert): the first unconsumed bucket (`_bucketHint + 1`)
+   *      5. Mandatory consumption (the censorship assert): the first unconsumed bucket (`_bucketHint + 1`)
    *         must either not exist, sit past the consumption cutoff, or be cap-escaped — consuming through it
    *         would exceed MAX_L1_TO_L2_MSGS_PER_CHECKPOINT messages since the parent checkpoint's cumulative
    *         total. The cutoff is the start of the checkpoint's build frame minus INBOX_LAG_SECONDS: a
@@ -427,6 +429,17 @@ library ProposeLib {
       Errors.Rollup__InvalidInboxRollingHash(bucket.rollingHash, _inboxRollingHash)
     );
 
+    // A bucket that can still absorb a message mutates in place: a proposer bundling a send after its own propose
+    // in one L1 transaction would leave the checkpoint committed to a rolling hash that exists neither on L1 nor
+    // in any node, which only ever observes a bucket's end-of-block state, and no honest node could then resolve
+    // the consumed position. Settled is the negation of the Inbox's rollover condition: the genesis bucket never
+    // absorbs, a bucket whose L1 block has passed cannot be reopened, and a full bucket spills the next message
+    // into a new one.
+    require(
+      _bucketHint == 0 || bucket.timestamp < block.timestamp || bucket.msgCount == MAX_MSGS_PER_BUCKET,
+      Errors.Rollup__InboxBucketStillMutable(_bucketHint)
+    );
+
     require(
       bucket.totalMsgCount >= _parentTotalMsgCount,
       Errors.Rollup__InboxConsumptionBehindParent(_parentTotalMsgCount, bucket.totalMsgCount)

l1-contracts/src/core/messagebridge/Inbox.sol

@@ -3,7 +3,7 @@
 pragma solidity >=0.8.27;
 
 import {IRollup} from "@aztec/core/interfaces/IRollup.sol";
-import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
+import {IInbox, MAX_MSGS_PER_BUCKET} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
 import {Constants} from "@aztec/core/libraries/ConstantsGen.sol";
 import {FrontierLib} from "@aztec/core/libraries/crypto/FrontierLib.sol";
 import {Hash} from "@aztec/core/libraries/crypto/Hash.sol";
@@ -34,11 +34,6 @@ contract Inbox is IInbox {
   using FrontierLib for FrontierLib.Forest;
   using FrontierLib for FrontierLib.Tree;
 
-  // Maximum number of messages a single bucket can hold before further messages in the same L1 block
-  // spill over into the next bucket. Matches the number of L1 to L2 messages a single L2 block can
-  // insert once the streaming inbox is live, so any one bucket is always consumable by one block.
-  uint256 public constant MAX_MSGS_PER_BUCKET = 256;
-
   address public immutable ROLLUP;
   uint256 public immutable VERSION;
   address public immutable FEE_ASSET_PORTAL;

l1-contracts/test/InboxBuckets.t.sol

@@ -5,7 +5,7 @@ pragma solidity >=0.8.27;
 import {Test} from "forge-std/Test.sol";
 import {TestERC20} from "src/mock/TestERC20.sol";
 import {IERC20} from "@oz/token/ERC20/IERC20.sol";
-import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
+import {IInbox, MAX_MSGS_PER_BUCKET} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
 import {MIN_BUCKET_RING_SIZE} from "@aztec/core/messagebridge/Inbox.sol";
 import {InboxHarness} from "./harnesses/InboxHarness.sol";
 import {TestConstants} from "./harnesses/TestConstants.sol";
@@ -172,7 +172,7 @@ contract InboxBucketsTest is Test {
   }
 
   function testRolloverIntoNextBucket() public {
-    uint256 cap = inbox.MAX_MSGS_PER_BUCKET();
+    uint256 cap = MAX_MSGS_PER_BUCKET;
     for (uint256 i = 0; i < cap; i++) {
       _send(inbox, i);
     }
@@ -266,7 +266,7 @@ contract InboxBucketsTest is Test {
   // Gas cost of a rollover opening mid-block: once a bucket reaches MAX_MSGS_PER_BUCKET, the next
   // message in the same L1 block opens a new bucket even though the timestamp is unchanged.
   function testGasSendRolloverMidBlock() public {
-    uint256 cap = inbox.MAX_MSGS_PER_BUCKET();
+    uint256 cap = MAX_MSGS_PER_BUCKET;
     for (uint256 i = 0; i < cap; i++) {
       _send(inbox, i);
     }

l1-contracts/test/rollup/ProposeInboxConsumption.t.sol

@@ -5,7 +5,7 @@ pragma solidity >=0.8.27;
 import {Test} from "forge-std/Test.sol";
 import {TestERC20} from "src/mock/TestERC20.sol";
 import {IERC20} from "@oz/token/ERC20/IERC20.sol";
-import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
+import {IInbox, MAX_MSGS_PER_BUCKET} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
 import {
   ProposeLib,
   INBOX_LAG_SECONDS,
@@ -79,6 +79,8 @@ contract ProposeInboxConsumptionTest is Test {
   }
 
   function testEmptyInbox() public {
+    // The genesis bucket carries the Inbox's deployment timestamp and never absorbs messages, so it is settled
+    // from the start: referencing it works even in the L1 block the Inbox was deployed in.
     uint256 consumed = rollup.validateInboxConsumption(inbox, bytes32(0), 0, SLOT, 0);
     assertEq(consumed, 0, "consumed nothing on empty inbox");
   }
@@ -218,6 +220,38 @@ contract ProposeInboxConsumptionTest is Test {
     rollup.validateInboxConsumption(ringInbox, bytes32(0), 1, SLOT, 0);
   }
 
+  function testCurrentBlockBucketRejected() public {
+    // A proposer that bundles `sendL2Message` and `propose` into a single L1 transaction can reference a
+    // bucket that is still accumulating and then mutate it with a trailing send. The snapshot the checkpoint
+    // committed to would afterwards exist nowhere: not on L1, where it was overwritten in place, and not in
+    // any node, which only ever observes a bucket's end-of-block state.
+    vm.warp(GENESIS_TIME + Slot.unwrap(SLOT) * SLOT_DURATION);
+    _send(0);
+    bytes32 liveHash = inbox.getBucket(1).rollingHash;
+
+    vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__InboxBucketStillMutable.selector, 1));
+    rollup.validateInboxConsumption(inbox, liveHash, 1, SLOT, 0);
+
+    // Nothing prevents the trailing send that erases the referenced snapshot, so the reference itself is what
+    // has to be rejected.
+    _send(1);
+    assertTrue(inbox.getBucket(1).rollingHash != liveHash, "bucket mutated in place within one L1 block");
+  }
+
+  function testFullBucketInCurrentBlockAccepted() public {
+    // A bucket at the per-bucket cap cannot absorb another message: the next send in the same L1 block opens
+    // a new bucket, so the snapshot is already final and referencing it within that block is safe.
+    vm.warp(GENESIS_TIME + Slot.unwrap(SLOT) * SLOT_DURATION);
+    uint256 cap = MAX_MSGS_PER_BUCKET;
+    _sendMany(cap);
+    assertEq(inbox.getBucket(1).msgCount, cap, "bucket 1 is at the per-bucket cap");
+    assertEq(inbox.getBucket(1).timestamp, block.timestamp, "bucket 1 was opened in this L1 block");
+
+    bytes32 endHash = inbox.getBucket(1).rollingHash;
+    uint256 consumed = rollup.validateInboxConsumption(inbox, endHash, 1, SLOT, 0);
+    assertEq(consumed, cap, "a full bucket is consumable in the block it was opened in");
+  }
+
   function testConsumeBackwardsReverts() public {
     // Buckets 1 and 2 exist with distinct cumulative totals. A proposal that references bucket 1 while its
     // parent already consumed through bucket 2 would move consumption backwards.