Merge pull request #553 from AztecProtocol/cb/port-field-range-next

ludamad · web-flow · commit 2d6703985f0d · 2026-07-03T07:52:06.000-04:00
feat: reject out-of-range checkpoint header fields at propose (port of #24199 to next)
diff --git a/l1-contracts/scripts/test_rollup_upgrade.sh b/l1-contracts/scripts/test_rollup_upgrade.sh
@@ -51,8 +51,9 @@ if [[ -z "$registry_address" || "$registry_address" == "null" ]]; then
 fi
 
 echo "=== Testing run_rollup_upgrade.sh ==="
-# Use a different genesis to get a different rollup version
-export GENESIS_ARCHIVE_ROOT="0x$(openssl rand -hex 32)"
+# Use a different genesis to get a different rollup version. Only 31 random bytes (top byte zero) so the value is
+# always below the BN254 scalar field modulus; the rollup rejects a genesis archive root >= the field modulus.
+export GENESIS_ARCHIVE_ROOT="0x00$(openssl rand -hex 31)"
 
 ./scripts/run_rollup_upgrade.sh "$registry_address"
 
diff --git a/l1-contracts/src/core/libraries/Errors.sol b/l1-contracts/src/core/libraries/Errors.sol
@@ -98,6 +98,7 @@ library Errors {
   error Rollup__NoBlobsInCheckpoint();
   error Rollup__CannotInvalidateEscapeHatch();
   error Rollup__InvalidEscapeHatchProposer(address expected, address actual);
+  error Rollup__FieldElementOutOfRange(bytes32 value);
 
   // EscapeHatch
   error EscapeHatch__AlreadyInCandidateSet(address candidate);
diff --git a/l1-contracts/src/core/libraries/rollup/FieldLib.sol b/l1-contracts/src/core/libraries/rollup/FieldLib.sol
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2024 Aztec Labs.
+pragma solidity >=0.8.27;
+
+import {Constants} from "@aztec/core/libraries/ConstantsGen.sol";
+import {Errors} from "@aztec/core/libraries/Errors.sol";
+
+/**
+ * @title FieldLib
+ * @author Aztec Labs
+ * @notice Helpers for validating that values stored on L1 are valid BN254 scalar field elements.
+ * @dev Off-chain components decode several L1 storage slots into `Fr`. A value `>=` the field modulus throws on
+ *      conversion and would brick honest archivers' L1 sync, so such values are rejected at write time.
+ */
+library FieldLib {
+  /// @notice Reverts with `Rollup__FieldElementOutOfRange` unless `_value` is a valid field element (`< Constants.P`).
+  function requireValidFieldElement(bytes32 _value) internal pure {
+    require(uint256(_value) < Constants.P, Errors.Rollup__FieldElementOutOfRange(_value));
+  }
+}
diff --git a/l1-contracts/src/core/libraries/rollup/ProposeLib.sol b/l1-contracts/src/core/libraries/rollup/ProposeLib.sol
@@ -12,6 +12,7 @@ import {Errors} from "@aztec/core/libraries/Errors.sol";
 import {CommitteeAttestations} from "@aztec/core/libraries/rollup/AttestationLib.sol";
 import {CoordinationSignatureLib} from "@aztec/core/libraries/rollup/CoordinationSignatureLib.sol";
 import {OracleInput, FeeLib, ManaMinFeeComponents} from "@aztec/core/libraries/rollup/FeeLib.sol";
+import {FieldLib} from "@aztec/core/libraries/rollup/FieldLib.sol";
 import {ValidatorSelectionLib} from "@aztec/core/libraries/rollup/ValidatorSelectionLib.sol";
 import {Timestamp, Slot, Epoch, TimeLib} from "@aztec/core/libraries/TimeLib.sol";
 import {CompressedSlot, CompressedTimeMath} from "@aztec/shared/libraries/CompressedTimeMath.sol";
@@ -125,6 +126,7 @@ library ProposeLib {
    *          - Proposer signature is valid for designated slot proposer:
    *            Errors.ValidatorSelection__MissingProposerSignature
    *          - Inbox hash matches expected value: Errors.Rollup__InvalidInHash
+   *          - Archive root is within the scalar field: Errors.Rollup__FieldElementOutOfRange
    *
    *          Validations NOT performed:
    *          - Committee attestations (only proposer signature verified)
@@ -184,6 +186,10 @@ library ProposeLib {
 
     v.header = _args.header;
 
+    // The new checkpoint archive root is not part of the header, so it is range-checked here rather than in
+    // validateHeader.
+    FieldLib.requireValidFieldElement(_args.archive);
+
     // Compute header hash for computing the payload digest
     v.headerHash = ProposedHeaderLib.hash(v.header);
 
@@ -305,6 +311,7 @@ library ProposeLib {
    *      for proposers to check header validity before submitting transactions
    *
    *      Header validations performed:
+   *      - Fr-encoded header fields are within the scalar field: Errors.Rollup__FieldElementOutOfRange
    *      - Coinbase address is non-zero: Errors.Rollup__InvalidCoinbase
    *      - Mana usage within limits: Errors.Rollup__ManaLimitExceeded
    *      - Builds on correct parent checkpoint (archive root check): Errors.Rollup__InvalidArchive
@@ -319,6 +326,12 @@ library ProposeLib {
    * @param _args Validation arguments including header, digest, mana min fee, and flags
    */
   function validateHeader(ValidateHeaderArgs memory _args) internal view {
+    // Check that header fields that map to an Fr are within range.
+    FieldLib.requireValidFieldElement(_args.header.blockHeadersHash);
+    FieldLib.requireValidFieldElement(_args.header.outHash);
+    FieldLib.requireValidFieldElement(_args.header.feeRecipient);
+    FieldLib.requireValidFieldElement(bytes32(_args.header.accumulatedFees));
+
     require(_args.header.coinbase != address(0), Errors.Rollup__InvalidCoinbase());
     require(_args.header.totalManaUsed <= FeeLib.getManaLimit(), Errors.Rollup__ManaLimitExceeded());
 
diff --git a/l1-contracts/src/core/libraries/rollup/STFLib.sol b/l1-contracts/src/core/libraries/rollup/STFLib.sol
@@ -11,6 +11,7 @@ import {
 import {CompressedFeeHeader, FeeHeaderLib, FeeHeader} from "@aztec/core/libraries/compressed-data/fees/FeeStructs.sol";
 import {ChainTipsLib, CompressedChainTips} from "@aztec/core/libraries/compressed-data/Tips.sol";
 import {Errors} from "@aztec/core/libraries/Errors.sol";
+import {FieldLib} from "@aztec/core/libraries/rollup/FieldLib.sol";
 import {Timestamp, Slot, Epoch, TimeLib} from "@aztec/core/libraries/TimeLib.sol";
 import {CompressedSlot, CompressedTimeMath} from "@aztec/shared/libraries/CompressedTimeMath.sol";
 
@@ -107,6 +108,9 @@ library STFLib {
     rollupStore.config.vkTreeRoot = _genesisState.vkTreeRoot;
     rollupStore.config.protocolContractsHash = _genesisState.protocolContractsHash;
 
+    // The genesis archive root is decoded as an Fr off chain and propagates into the first header's lastArchiveRoot,
+    // so it must be a valid field element.
+    FieldLib.requireValidFieldElement(_genesisState.genesisArchiveRoot);
     rollupStore.archives[0] = _genesisState.genesisArchiveRoot;
   }
 
diff --git a/l1-contracts/test/RollupFieldRange.t.sol b/l1-contracts/test/RollupFieldRange.t.sol
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2024 Aztec Labs.
+pragma solidity >=0.8.27;
+
+import {DecoderBase} from "./base/DecoderBase.sol";
+import {RollupBase, IInstance} from "./base/RollupBase.sol";
+
+import {Constants} from "@aztec/core/libraries/ConstantsGen.sol";
+import {Errors} from "@aztec/core/libraries/Errors.sol";
+import {SafeCast} from "@oz/utils/math/SafeCast.sol";
+
+import {Inbox} from "@aztec/core/messagebridge/Inbox.sol";
+import {TestConstants} from "./harnesses/TestConstants.sol";
+import {RollupBuilder} from "./builder/RollupBuilder.sol";
+import {EthValue, GenesisState} from "@aztec/core/interfaces/IRollup.sol";
+
+import {ProposeArgs, OracleInput, ProposeLib} from "@aztec/core/libraries/rollup/ProposeLib.sol";
+import {ProposedHeader} from "@aztec/core/libraries/rollup/ProposedHeaderLib.sol";
+import {Timestamp, Slot, TimeLib} from "@aztec/core/libraries/TimeLib.sol";
+import {CommitteeAttestations} from "@aztec/core/libraries/rollup/AttestationLib.sol";
+import {Signature} from "@aztec/shared/libraries/SignatureLib.sol";
+import {AttestationLibHelper} from "@test/helper_libraries/AttestationLibHelper.sol";
+
+// solhint-disable comprehensive-interface
+
+/**
+ * @notice Exercises the field-range checks added to ProposeLib.
+ *
+ * A malicious proposer could previously store header or archive field values that are >= the BN254 scalar field
+ * modulus on L1. Off-chain archivers decode those slots into `Fr`, and an out-of-range value bricks their L1 sync.
+ * The checks reject such values at propose time; these tests cover the boundary (`P`), the extreme
+ * (`type(uint256).max`), and that an otherwise-valid header with every checked field at `P - 1` still goes through.
+ */
+contract RollupFieldRangeTest is RollupBase {
+  uint256 internal constant FIELD_MAX = Constants.P - 1;
+
+  string internal constant FIXTURE = "mixed_checkpoint_1";
+
+  uint256 internal SLOT_DURATION;
+
+  modifier setUp() {
+    {
+      DecoderBase.Full memory full = load(FIXTURE);
+      uint256 slotNumber = Slot.unwrap(full.checkpoint.header.slotNumber);
+      uint256 initialTime = Timestamp.unwrap(full.checkpoint.header.timestamp) - slotNumber * SLOT_DURATION;
+      vm.warp(initialTime);
+    }
+
+    RollupBuilder builder =
+      new RollupBuilder(address(this)).setTargetCommitteeSize(0).setProvingCostPerMana(EthValue.wrap(1000));
+    builder.deploy();
+
+    rollup = IInstance(address(builder.getConfig().rollup));
+    inbox = Inbox(address(rollup.getInbox()));
+    _;
+  }
+
+  constructor() {
+    TimeLib.initialize(
+      block.timestamp,
+      TestConstants.AZTEC_SLOT_DURATION,
+      TestConstants.AZTEC_EPOCH_DURATION,
+      TestConstants.AZTEC_PROOF_SUBMISSION_EPOCHS
+    );
+    SLOT_DURATION = TestConstants.AZTEC_SLOT_DURATION;
+  }
+
+  // ----- archive (checked in propose, before validateHeader) -----
+
+  function testRevertsArchiveAtModulus() public setUp {
+    _expectFieldOutOfRange(_baseArgs(Constants.P, false), bytes32(Constants.P));
+  }
+
+  function testRevertsArchiveAtUintMax() public setUp {
+    _expectFieldOutOfRange(_baseArgs(type(uint256).max, false), bytes32(type(uint256).max));
+  }
+
+  // ----- blockHeadersHash -----
+
+  function testRevertsBlockHeadersHashAtModulus() public setUp {
+    ProposeArgs memory args = _baseArgs(0, true);
+    args.header.blockHeadersHash = bytes32(Constants.P);
+    _expectFieldOutOfRange(args, bytes32(Constants.P));
+  }
+
+  function testRevertsBlockHeadersHashAtUintMax() public setUp {
+    ProposeArgs memory args = _baseArgs(0, true);
+    args.header.blockHeadersHash = bytes32(type(uint256).max);
+    _expectFieldOutOfRange(args, bytes32(type(uint256).max));
+  }
+
+  // ----- outHash -----
+
+  function testRevertsOutHashAtModulus() public setUp {
+    ProposeArgs memory args = _baseArgs(0, true);
+    args.header.outHash = bytes32(Constants.P);
+    _expectFieldOutOfRange(args, bytes32(Constants.P));
+  }
+
+  function testRevertsOutHashAtUintMax() public setUp {
+    ProposeArgs memory args = _baseArgs(0, true);
+    args.header.outHash = bytes32(type(uint256).max);
+    _expectFieldOutOfRange(args, bytes32(type(uint256).max));
+  }
+
+  // ----- feeRecipient -----
+
+  function testRevertsFeeRecipientAtModulus() public setUp {
+    ProposeArgs memory args = _baseArgs(0, true);
+    args.header.feeRecipient = bytes32(Constants.P);
+    _expectFieldOutOfRange(args, bytes32(Constants.P));
+  }
+
+  function testRevertsFeeRecipientAtUintMax() public setUp {
+    ProposeArgs memory args = _baseArgs(0, true);
+    args.header.feeRecipient = bytes32(type(uint256).max);
+    _expectFieldOutOfRange(args, bytes32(type(uint256).max));
+  }
+
+  // ----- accumulatedFees -----
+
+  function testRevertsAccumulatedFeesAtModulus() public setUp {
+    ProposeArgs memory args = _baseArgs(0, true);
+    args.header.accumulatedFees = Constants.P;
+    _expectFieldOutOfRange(args, bytes32(Constants.P));
+  }
+
+  function testRevertsAccumulatedFeesAtUintMax() public setUp {
+    ProposeArgs memory args = _baseArgs(0, true);
+    args.header.accumulatedFees = type(uint256).max;
+    _expectFieldOutOfRange(args, bytes32(type(uint256).max));
+  }
+
+  // ----- genesis archive root (checked in STFLib.initialize at deploy time) -----
+
+  function testRevertsGenesisArchiveRootOutOfRange() public {
+    GenesisState memory genesis = TestConstants.getGenesisState();
+    genesis.genesisArchiveRoot = bytes32(Constants.P);
+
+    RollupBuilder builder = new RollupBuilder(address(this)).setTargetCommitteeSize(0).setGenesisState(genesis);
+
+    vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__FieldElementOutOfRange.selector, bytes32(Constants.P)));
+    builder.deploy();
+  }
+
+  // ----- happy path: every checked field at P - 1 must not trip the range check -----
+
+  /**
+   * @notice A fully valid propose where each checked field is at the largest in-range value (`P - 1`) must succeed,
+   *         proving the boundary is exclusive and the checks do not reject legitimate field elements.
+   */
+  function testAcceptsAllFieldsAtModulusMinusOne() public setUp {
+    DecoderBase.Full memory full = load(FIXTURE);
+    ProposedHeader memory header = full.checkpoint.header;
+
+    // Pin the header to a valid slot/fee/inbox/coinbase configuration so the only remaining question is whether the
+    // range checks accept the boundary values below.
+    Slot slotNumber = Slot.wrap(1);
+    Timestamp ts = rollup.getTimestampForSlot(slotNumber);
+    header.timestamp = ts;
+    header.slotNumber = slotNumber;
+    header.coinbase = address(bytes20("sequencer"));
+
+    vm.warp(max(block.timestamp, Timestamp.unwrap(ts)));
+
+    _populateInbox(full.populate.sender, full.populate.recipient, full.populate.l1ToL2Content);
+    header.inHash = rollup.getInbox().getRoot(full.checkpoint.checkpointNumber);
+    header.gasFees.feePerL2Gas = SafeCast.toUint128(rollup.getManaMinFeeAt(ts, true));
+
+    // Every field the range check guards, set to the maximal in-range value.
+    header.blockHeadersHash = bytes32(FIELD_MAX);
+    header.outHash = bytes32(FIELD_MAX);
+    header.feeRecipient = bytes32(FIELD_MAX);
+    header.accumulatedFees = FIELD_MAX;
+
+    vm.blobhashes(this.getBlobHashes(full.checkpoint.blobCommitments));
+
+    ProposeArgs memory args = ProposeArgs({header: header, archive: bytes32(FIELD_MAX), oracleInput: OracleInput(0)});
+
+    rollup.propose(
+      args,
+      AttestationLibHelper.packAttestations(attestations),
+      signers,
+      attestationsAndSignersSignature,
+      full.checkpoint.blobCommitments
+    );
+
+    assertEq(rollup.archive(), args.archive, "archive at P - 1 should have been stored");
+  }
+
+  /// @dev Builds propose args for the boundary tests with a valid slot/timestamp but skipped blob check.
+  function _baseArgs(uint256 _archive, bool _useFixtureArchive) internal returns (ProposeArgs memory) {
+    DecoderBase.Full memory full = load(FIXTURE);
+    ProposedHeader memory header = full.checkpoint.header;
+
+    Slot slotNumber = Slot.wrap(1);
+    Timestamp ts = rollup.getTimestampForSlot(slotNumber);
+    header.timestamp = ts;
+    header.slotNumber = slotNumber;
+
+    vm.warp(max(block.timestamp, Timestamp.unwrap(ts)));
+    skipBlobCheck(address(rollup));
+
+    bytes32 archive = _useFixtureArchive ? full.checkpoint.archive : bytes32(_archive);
+    return ProposeArgs({header: header, archive: archive, oracleInput: OracleInput(0)});
+  }
+
+  function _expectFieldOutOfRange(ProposeArgs memory _args, bytes32 _value) internal {
+    // Resolve everything that needs cheatcodes (loading the fixture, packing attestations) BEFORE arming
+    // expectRevert, otherwise those cheatcode calls are mistaken for the call under test.
+    bytes memory blobCommitments = load(FIXTURE).checkpoint.blobCommitments;
+    CommitteeAttestations memory packed = AttestationLibHelper.packAttestations(attestations);
+    address[] memory localSigners = signers;
+    Signature memory sig = attestationsAndSignersSignature;
+
+    vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__FieldElementOutOfRange.selector, _value));
+    rollup.propose(_args, packed, localSigners, sig, blobCommitments);
+  }
+}
