Skip to content

fix(eip1559): prevent divide-by-zero in next base fee calculation#3884

Closed
InoMurko wants to merge 4 commits intoalloy-rs:mainfrom
ConstellationCrypto:inomurko/1.8.2-fix
Closed

fix(eip1559): prevent divide-by-zero in next base fee calculation#3884
InoMurko wants to merge 4 commits intoalloy-rs:mainfrom
ConstellationCrypto:inomurko/1.8.2-fix

Conversation

@InoMurko
Copy link
Copy Markdown
Contributor

@InoMurko InoMurko commented Apr 13, 2026

Summary

Hardens EIP-1559 next-base-fee calculation against divide-by-zero when elasticity, max-change denominator, or effective gas target is zero, so callers do not panic on degenerate parameters.

Problem

calc_next_block_base_fee divides by gas_target * max_change_denominator (and computes gas_target as gas_limit / elasticity). Any of the following can make that denominator zero and panic in debug/release:

  • elasticity_multiplier == 0 or max_change_denominator == 0
  • gas_target == 0 with gas_used > 0 (e.g. gas_limit < elasticity on chains that do not enforce the same gas-limit rules as L1)

Callers that build BaseFeeParams from external or misconfigured values could pass a struct with a zero field even when the other is non-zero; that still flows into calc_next_block_base_fee and can trigger the same failure.

Separately, node logs showing base_fee=0.00Gwei for very small base fees (e.g. 50 wei) are often a display rounding artifact ({:.2} Gwei), not necessarily a zero base fee in the header.

Solution

At the start of calc_next_block_base_fee, return the parent base_fee unchanged when elasticity == 0, max_change_denominator == 0, or gas_target == 0 (using u128 division for gas_limit / elasticity to avoid overflow/truncation surprises). This matches the defensive behavior of Nethermind’s DefaultBaseFeeCalculator for parentGasTarget == 0 / zero denominator.

Testing

  • Unit tests for zero elasticity, zero denominator, and gas_limit < elasticity with gas_used > 0
  • Existing OP/mainnet base-fee vectors unchanged

Notes

This is defense in depth: correct chain config and valid inputs remain the source of truth; these guards prevent crashes and align with other clients’ safe fallbacks for degenerate inputs.

Related

  • EIP-1559 (base fee update rule)
  • Nethermind: DefaultBaseFeeCalculator (parentGasTarget == 0 / zero denominator → unchanged parent base fee)

@InoMurko InoMurko changed the title op-reth alloy panic fix(eip1559): prevent divide-by-zero in next base fee and harden OP Holocene/Jovian params Apr 13, 2026
@InoMurko
Copy link
Copy Markdown
Contributor Author

@DaniPopes
@mattsse
@klkvr
@onbjerg
@zerosnacks
@grandizzy

any interest in reviewing this?

@DaniPopes
Copy link
Copy Markdown
Member

base fee params shouldnt be 0 to begin with, but sure i guess.
i don't think we need the tracing either.

please remove "op reth" mentions from pr, there's no optimism code in here.

@InoMurko InoMurko changed the title fix(eip1559): prevent divide-by-zero in next base fee and harden OP Holocene/Jovian params fix(eip1559): prevent divide-by-zero in next base fee calculation Apr 21, 2026
@InoMurko
Copy link
Copy Markdown
Contributor Author

base fee params shouldnt be 0 to begin with, but sure i guess. i don't think we need the tracing either.

please remove "op reth" mentions from pr, there's no optimism code in here.

Fixed!

@InoMurko InoMurko force-pushed the inomurko/1.8.2-fix branch from b5cad38 to 7cf190a Compare April 24, 2026 09:47
@InoMurko
Copy link
Copy Markdown
Contributor Author

rebased and ready to be reviewed @DaniPopes!

@InoMurko InoMurko force-pushed the inomurko/1.8.2-fix branch from 7cf190a to 82b5807 Compare April 26, 2026 15:28
@InoMurko
Copy link
Copy Markdown
Contributor Author

@DaniPopes
@mattsse
@klkvr
@onbjerg
@zerosnacks
@grandizzy

can this get reviewed?

@InoMurko
Copy link
Copy Markdown
Contributor Author

@DaniPopes
@mattsse
@klkvr
@onbjerg
@zerosnacks
@grandizzy

any interest in reviewing this?

@mattsse
Copy link
Copy Markdown
Member

mattsse commented May 4, 2026

I couldn’t push the formatting fix directly here because maintainer edits are disabled on the source branch. I opened an upstream replacement with the same changes plus the rustfmt fix: #3951.

@github-project-automation github-project-automation Bot moved this to Reviewed in Alloy May 4, 2026
@mattsse
Copy link
Copy Markdown
Member

mattsse commented May 4, 2026

Closed as superseded by #3951, which has been merged.

@mattsse mattsse closed this May 4, 2026
@github-project-automation github-project-automation Bot moved this from Reviewed to Done in Alloy May 4, 2026
@InoMurko
Copy link
Copy Markdown
Contributor Author

InoMurko commented May 4, 2026

I couldn’t push the formatting fix directly here because maintainer edits are disabled on the source branch. I opened an upstream replacement with the same changes plus the rustfmt fix: #3951.

Sounds good. Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants