Skip to content

Cap fee components before proposal validation #24713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
AztecBot wants to merge 1 commit into
base: next
Choose a base branch
from
Draft

Cap fee components before proposal validation #24713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions l1-contracts/src/core/libraries/compressed-data/fees/FeeStructs.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,9 @@ library FeeHeaderLib {
// Cap excessMana to uint48 max to prevent overflow during compression.
value |= Math.min(_feeHeader.excessMana, MASK_48_BITS) << 32;
value |= uint256(_feeHeader.ethPerFeeAsset.toUint48()) << 80;
// Cap congestionCost to uint64 max to prevent overflow during compression.
// The uncapped value is still used for fee validation; this only affects storage.
// Keep compression defensive; fee validation should already use capped component values.
value |= Math.min(_feeHeader.congestionCost, MASK_64_BITS) << 128;
// Cap proverCost to uint63 max to prevent overflow during compression.
// Keep compression defensive; fee validation should already use capped component values.
value |= Math.min(_feeHeader.proverCost, MASK_63_BITS) << 192;

// Preheat
Expand Down
16 changes: 13 additions & 3 deletions l1-contracts/src/core/libraries/rollup/FeeLib.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ uint256 constant MIN_PROVING_COST_PER_MANA = 2;
// time to correct from. At the time of this writing, deployed value is 2.5e7, and it is expected
// that proving costs will go down.
uint256 constant MAX_INITIAL_PROVING_COST_PER_MANA = 2e8;
uint256 constant MAX_FEE_HEADER_CONGESTION_COST = type(uint64).max;
uint256 constant MAX_FEE_HEADER_PROVER_COST = (1 << 63) - 1;

struct OracleInput {
int256 feeAssetPriceModifier;
Expand Down Expand Up @@ -312,7 +314,7 @@ library FeeLib {
FeeLib.clampedAdd(parentFeeHeader.getExcessMana() + parentFeeHeader.getManaUsed(), -int256(manaTarget));
uint256 congestionMultiplier_ = congestionMultiplier(excessMana);

EthValue congestionCost =
EthValue congestionCostPerMana =
EthValue.wrap(
Math.mulDiv(EthValue.unwrap(total), congestionMultiplier_, MINIMUM_CONGESTION_MULTIPLIER, Math.Rounding.Floor)
) - total;
Expand All @@ -321,10 +323,18 @@ library FeeLib {
? FeeLib.getEthPerFeeAssetAtCheckpoint(_checkpointOfInterest)
: EthPerFeeAssetE12.wrap(ETH_PER_FEE_ASSET_PRECISION);

uint256 proverCost = FeeAssetValue.unwrap(proverCostPerMana.toFeeAsset(ethPerFeeAsset));
uint256 congestionCost = FeeAssetValue.unwrap(congestionCostPerMana.toFeeAsset(ethPerFeeAsset));

if (_inFeeAsset) {
proverCost = Math.min(proverCost, MAX_FEE_HEADER_PROVER_COST);
congestionCost = Math.min(congestionCost, MAX_FEE_HEADER_CONGESTION_COST);
}

return ManaMinFeeComponents({
sequencerCost: FeeAssetValue.unwrap(sequencerCostPerMana.toFeeAsset(ethPerFeeAsset)),
proverCost: FeeAssetValue.unwrap(proverCostPerMana.toFeeAsset(ethPerFeeAsset)),
congestionCost: FeeAssetValue.unwrap(congestionCost.toFeeAsset(ethPerFeeAsset)),
proverCost: proverCost,
congestionCost: congestionCost,
congestionMultiplier: congestionMultiplier_
});
}
Expand Down
36 changes: 19 additions & 17 deletions l1-contracts/test/fees/FeeHeaderOverflow.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,8 @@ contract FeeHeaderOverflowTest is DecoderBase {
/**
* @notice FeeConfig stores provingCostPerMana as uint64, but FeeHeader compresses
* proverCost as 63 bits. Setting provingCostPerMana between 2^63 and 2^64-1
* produces a proverCost that always overflows during compression.
* This demonstrates that governance can set a valid-looking config value
* that permanently bricks proposal submission.
* would produce a proverCost above the compressed field width unless the
* user-facing fee component is capped before validation.
* The brick can occur even if the "actual" provercostPerMana is in the range,
* because the final proving prover cost also includes the L1 component.
*/
Expand Down Expand Up @@ -163,18 +162,19 @@ contract FeeHeaderOverflowTest is DecoderBase {
// Warp to slot 1
vm.warp(block.timestamp + SLOT_DURATION);

// The fee computation succeeds because intermediate values are uint256
// The fee computation succeeds because intermediate values are uint256, and the
// user-facing fee component is capped to the same value stored for proof time.
ManaMinFeeComponents memory components = rollup.getManaMinFeeComponentsAt(Timestamp.wrap(block.timestamp), true);
uint256 manaMinFee = rollup.getManaMinFeeAt(Timestamp.wrap(block.timestamp), true);

assertTrue(components.proverCost > MAX_PROVER_COST, "proverCost should exceed 63-bit limit");
assertEq(components.proverCost, MAX_PROVER_COST, "proverCost should be capped at 63-bit limit");

(ProposeArgs memory proposeArgs, CommitteeAttestations memory attestations, address[] memory signers) =
_buildProposal(rollup, manaMinFee);

skipBlobCheck(address(rollup));

// propose succeeds because compress() caps proverCost at 63-bit max instead of reverting.
// propose succeeds because fee validation and storage use the same capped prover cost.
rollup.propose(proposeArgs, attestations, signers, Signature({v: 0, r: 0, s: 0}), full.checkpoint.blobCommitments);

// Verify the stored fee header has capped proverCost
Expand Down Expand Up @@ -228,19 +228,20 @@ contract FeeHeaderOverflowTest is DecoderBase {
// Warp to slot 1
vm.warp(block.timestamp + SLOT_DURATION);

// Fee computation succeeds (uint256 intermediates), but congestionCost exceeds uint64
// Fee computation succeeds (uint256 intermediates), and the user-facing component is capped
// to the same value that can be stored for proof-time reward distribution.
ManaMinFeeComponents memory components = rollup.getManaMinFeeComponentsAt(Timestamp.wrap(block.timestamp), true);
uint256 manaMinFee = rollup.getManaMinFeeAt(Timestamp.wrap(block.timestamp), true);

assertTrue(components.congestionCost > MAX_CONGESTION_COST, "congestionCost should exceed 64-bit limit");
assertEq(components.congestionCost, MAX_CONGESTION_COST, "congestionCost should be capped at 64-bit limit");
assertTrue(components.proverCost <= MAX_PROVER_COST, "proverCost should still fit in 63 bits");

(ProposeArgs memory proposeArgs, CommitteeAttestations memory attestations, address[] memory signers) =
_buildProposal(rollup, manaMinFee);

skipBlobCheck(address(rollup));

// propose succeeds because compress() caps congestionCost at 64-bit max instead of reverting.
// propose succeeds because fee validation and storage use the same capped congestion cost.
rollup.propose(proposeArgs, attestations, signers, Signature({v: 0, r: 0, s: 0}), full.checkpoint.blobCommitments);

// Verify the stored fee header has capped congestionCost
Expand All @@ -261,8 +262,8 @@ contract FeeHeaderOverflowTest is DecoderBase {
*
* After fix: Three caps work together to keep the system live:
* 1. congestionMultiplier() caps the exponent at 100 (prevents Taylor series overflow)
* 2. summedMinFee() caps the total fee at uint128 max (ensures header representability)
* 3. compress() caps individual fields (prevents fee header compression overflow)
* 2. getManaMinFeeComponentsAt() caps fee-asset components to their storage widths
* 3. summedMinFee() caps the total fee at uint128 max if the remaining sum still exceeds it
*
* We simulate the accumulated excess by directly writing to the genesis fee header's
* storage slot, which is equivalent to ~1000 consecutive max-capacity checkpoints.
Expand Down Expand Up @@ -307,20 +308,21 @@ contract FeeHeaderOverflowTest is DecoderBase {

// The congestion multiplier is capped (excessMana > denominator * 100 threshold)
assertTrue(components.congestionMultiplier > 0, "congestionMultiplier should be non-zero");
// Individual components exceed their compressed field widths
assertTrue(components.congestionCost > MAX_CONGESTION_COST, "congestionCost exceeds 64-bit limit");
// Individual components are capped to their compressed field widths before fee validation.
assertEq(components.congestionCost, MAX_CONGESTION_COST, "congestionCost should be capped at 64-bit limit");

// summedMinFee caps the total at uint128 max, ensuring the header can represent it
assertEq(manaMinFee, type(uint128).max, "mana min fee should be capped at uint128 max");
// The capped components keep this scenario below the header's uint128 total-fee ceiling.
assertEq(manaMinFee, FeeLib.summedMinFee(components), "mana min fee mismatch");
assertLt(manaMinFee, type(uint128).max, "mana min fee should stay below uint128 max");

// Propose succeeds: all three caps work together
(ProposeArgs memory proposeArgs, CommitteeAttestations memory attestations, address[] memory signers) =
_buildProposal(rollup, manaMinFee);

skipBlobCheck(address(rollup));

// propose succeeds because congestionMultiplier is capped (no Taylor overflow),
// summedMinFee is capped at uint128 max (valid header), and compress caps individual fields.
// propose succeeds because congestionMultiplier is capped (no Taylor overflow), and
// fee validation already uses values that fit the compressed fee header.
rollup.propose(proposeArgs, attestations, signers, Signature({v: 0, r: 0, s: 0}), full.checkpoint.blobCommitments);

// Verify the stored fee header has capped values
Expand Down
Loading