[DONOT MERGE]: Hotfix hoodi submitter blob fee calculate#793
[DONOT MERGE]: Hotfix hoodi submitter blob fee calculate#793
Conversation
WalkthroughGo module dependency versions updated (toolchain to Go 1.24.0 and multiple direct/indirect bumps) and rollup service logic adjusted: loop form changed, logging labels revised, excess blob gas handling added, and blob-fee computation replaced with a new Taylor-approximation helper plus a 3× multiplier. Changes
Sequence Diagram(s)sequenceDiagram
participant RollupSvc as RollupService
participant Head as HeadInfo
participant FeeCalc as FeeCalculator
Note over RollupSvc: GetGasTipAndCap flow
RollupSvc->>Head: read BaseFee, Tip, ExcessBlobGas
alt ExcessBlobGas == nil
RollupSvc->>FeeCalc: use existing calc (eip4844.CalcBlobFee)
FeeCalc-->>RollupSvc: blobFee
else ExcessBlobGas != nil
RollupSvc->>FeeCalc: fakeExponential(minBlobGasPrice, ExcessBlobGas, 5007716)
FeeCalc-->>RollupSvc: approxBlobFee
RollupSvc->>RollupSvc: apply multiplier (*3)
end
RollupSvc-->>Caller: return baseFee, tip, blobFee
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tx-submitter/services/rollup.go (1)
300-300: Consider reverting to the more concise range syntax.The change from
for i := range 3tofor i := 0; i < 3; i++is functionally equivalent but less idiomatic for Go 1.22+. The original range-based syntax is more concise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tx-submitter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
tx-submitter/go.mod(1 hunks)tx-submitter/services/rollup.go(5 hunks)tx-submitter/services/rollup_test.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
tx-submitter/services/rollup.go (1)
1248-1248: LGTM - Improved logging clarity.The updated log messages provide better context for fee-related information.
Also applies to: 1258-1258, 1281-1281
tx-submitter/go.mod (1)
10-10: Verify whether using this development snapshot is intentional.The version exists and is accessible, but this is a development snapshot created today (2025-10-30), not a stable release. Additionally, the eip4844 refactor in
rollup.gowas intentional—the code deliberately removed thego-ethereum/consensus/misc/eip4844import and replacedeip4844.CalcBlobFee()with a customfakeExponential()implementation. This refactor was not forced by breaking changes from the dependency update.Confirm whether pinning to a development version from a custom fork is appropriate for production use, or whether a stable tagged release should be used instead.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tx-submitter/services/rollup.go (2)
1282-1283: Previous critical issue remains unresolved: incorrect constant in blob fee calculation.The hardcoded constant
5007716on line 1283 was flagged in a previous review as incorrect. According to EIP-4844, the target blob gas per block is393216, not5007716. This issue was marked as addressed but the code still contains the incorrect value.The custom
fakeExponentialcalculation may produce results that diverge from the standardeip4844.CalcBlobFee, potentially causing:
- Transaction rejections if fees are too low
- Overpayment if fees are too high
- Network compatibility issues
Required actions:
- Replace
5007716with the correct constant fromparams(e.g.,params.BlobTxTargetBlobGasPerBlock) or document why this specific value is intentional- Consider using the standard
eip4844.CalcBlobFeeinstead of a custom approximation- If the custom calculation must remain, add unit tests comparing it against the standard implementation
1907-1922: Previous major issue remains unresolved: fakeExponential lacks critical safeguards.The
fakeExponentialfunction was previously flagged for lacking safeguards, but the issues remain unaddressed:
- Unbounded loop (line 1914): No iteration limit exists. For pathological inputs or precision issues, this could run indefinitely
- No overflow protection: Large inputs could cause integer overflow in intermediate multiplications before division
- Missing convergence handling: If the loop doesn't converge, there's no error indication or fallback
Required actions:
Add an iteration limit and convergence failure handling:func fakeExponential(factor, numerator, denominator *big.Int) *big.Int { + const maxIterations = 100 // Safeguard against unbounded loops var ( output = new(big.Int) accum = new(big.Int).Mul(factor, denominator) ) - for i := 1; accum.Sign() > 0; i++ { + for i := 1; accum.Sign() > 0 && i < maxIterations; i++ { output.Add(output, accum) accum.Mul(accum, numerator) accum.Div(accum, denominator) accum.Div(accum, big.NewInt(int64(i))) } + if accum.Sign() > 0 { + log.Warn("fakeExponential failed to converge", "iterations", maxIterations) + return big.NewInt(0) + } return output.Div(output, denominator) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tx-submitter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
tx-submitter/go.mod(3 hunks)tx-submitter/services/rollup.go(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
🔇 Additional comments (6)
tx-submitter/services/rollup.go (3)
300-300: Loop syntax update improves Go version compatibility.The change from
for i := range 3tofor i := 0; i < 3; i++is functionally equivalent and improves compatibility with Go versions prior to 1.22.
1248-1248: Improved logging labels for better observability.The updated labels distinguish between base fee and tip fee information, making the logs clearer. The addition of
.String()for the tip value ensures consistent formatting.Also applies to: 1258-1258
1281-1281: Good addition for blob fee observability.Logging the excess blob gas value helps with debugging and understanding blob fee calculations.
tx-submitter/go.mod (3)
12-12: Update testify to v1.10.0.The update from
v1.9.0tov1.10.0is a minor version bump and is typically safe. Confirm that the test suite passes with this newer version of testify.
22-22: Dependency updates verified as backwards-compatible; no breaking changes detected.Go module verification passed with all dependencies successfully resolved and verified. Web search confirms that the major cryptographic library updates contain no breaking changes:
- consensys/gnark-crypto v0.16.0 lists only performance improvements and fixes with no breaking changes
- consensys/bavard v0.1.27 is a backwards-compatible feature/patch release
- golang.org/x/crypto v0.35.0 contains no API breaking changes, with updates primarily addressing security fixes including CVE-2025-22869
The dependency updates are safe to proceed with.
3-3: Verify Go 1.24.0 compatibility through full build and test suite before merging.Go 1.24.0 is available, but further verification is required:
- gnark-crypto (v0.16.0): Not explicitly tested for Go 1.24 per project documentation, which states testing against only the last two major releases. This is a compatibility gap that needs confirmation.
- blst (v0.3.16-0.20250831170142-f48500c1fdbe): The pseudo-version is very recent (2025-08-31) and may include Go 1.24 fixes, but requires testing to confirm.
- Go 1.24 breaking changes: The upgrade introduces breaking changes in the compiler (cgo-generated type restrictions), runtime (Swiss Tables map implementation, new mutex), and stricter vet analyzers—these can expose latent bugs in code or tests.
Before merging, build and run the full test suite with Go 1.24.0 to ensure no regressions or build failures arise from the toolchain and dependency versions.
| module morph-l2/tx-submitter | ||
|
|
||
| go 1.22 | ||
| go 1.24.0 |
There was a problem hiding this comment.
Flag: PR marked "[DONOT MERGE]"—do not merge in current state.
This pull request is explicitly marked as not ready for merging. Please confirm whether all intended changes are complete and this is ready for review, or whether additional changes are pending before this should be considered for merge.
🤖 Prompt for AI Agents
In tx-submitter/go.mod around lines 3 to 3, the PR is marked "[DONOT MERGE]"
which blocks merging; confirm whether work is complete and either remove the
"[DONOT MERGE]" marker and update the PR title/description to explicitly state
readiness for review/merge, or keep the marker and update the PR description
with a clear list of remaining tasks and an estimated timeline; ensure CI passes
and any required reviewers are assigned before removing the block label.
| github.com/holiman/uint256 v1.2.4 | ||
| github.com/morph-l2/externalsign v0.3.1 | ||
| github.com/morph-l2/go-ethereum v1.10.14-0.20251020031512-67087dd3ad49 | ||
| github.com/morph-l2/go-ethereum v1.10.14-0.20251030090323-25b8e120126f |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that the go-ethereum dependency commit contains the intended blob fee calculation fix.
Per the PR title and AI summary, the blob-fee computation should be replaced with a Taylor-approximation helper plus a 3× multiplier. This logic resides in the github.com/morph-l2/go-ethereum dependency (updated to v1.10.14-0.20251030090323-25b8e120126f). Confirm that this specific commit contains the intended blob fee changes.
🏁 Script executed:
#!/bin/bash
# Description: Verify that the go-ethereum commit contains blob fee calculation changes.
# Extract the commit hash from the version string
GETH_VERSION="v1.10.14-0.20251030090323-25b8e120126f"
COMMIT_HASH=$(echo "$GETH_VERSION" | grep -oE '[0-9a-f]{12}')
echo "Extracted commit hash: $COMMIT_HASH"
# Check the go-ethereum repository for commits related to blob fees
# This requires access to the morph-l2/go-ethereum repository
curl -s "https://api.github.com/repos/morph-l2/go-ethereum/commits/$COMMIT_HASH" \
| jq '.commit.message' 2>/dev/null || echo "Unable to fetch commit details; verify manually at https://github.com/morph-l2/go-ethereum/commit/$COMMIT_HASH"Length of output: 48
I need to correct the commit hash extraction and query the actual commit. The version string format is a Go pseudo-version where the actual commit hash is the last 12 hexadecimal characters (25b8e120126f), not the timestamp portion.
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Verify blob fee calculation changes in the go-ethereum commit
# Correct extraction of commit hash from pseudo-version
# Format: v<version>-0.<timestamp>-<commit_hash>
GETH_VERSION="v1.10.14-0.20251030090323-25b8e120126f"
COMMIT_HASH="${GETH_VERSION##*-}"
echo "Extracted commit hash: $COMMIT_HASH"
# Fetch commit details from morph-l2/go-ethereum
echo ""
echo "Fetching commit details..."
curl -s "https://api.github.com/repos/morph-l2/go-ethereum/commits/$COMMIT_HASH" \
| jq -r '.commit.message // "Commit not found"'
Length of output: 124
The referenced go-ethereum commit does not contain blob fee calculation changes.
The commit 25b8e120126f in the dependency version v1.10.14-0.20251030090323-25b8e120126f contains only "Rm memsize import (#217)" — removing the github.com/fjl/memsize dependency. There is no evidence of blob fee calculation fixes, Taylor-approximation helpers, or the 3× multiplier mentioned in the PR title. Verify that the correct commit hash is referenced in tx-submitter/go.mod, or confirm the blob fee changes are addressed in a different dependency or PR.
🤖 Prompt for AI Agents
In tx-submitter/go.mod around line 10, the pinned go-ethereum pseudo-version
(v1.10.14-0.20251030090323-25b8e120126f) points to a commit that only removes
the memsize import and does not include the blob fee calculation,
Taylor-approximation helpers, or 3× multiplier changes referenced in the PR;
verify the correct commit that contains those blob fee changes (or identify the
correct dependency/PR that implements them) and update the go.mod entry to the
proper pseudo-version/commit hash or change the dependency to the package that
actually contains the fixes, then run go mod tidy and go mod download to ensure
the module graph is updated and the lockfile reflects the corrected commit.
1 participant
Summary by CodeRabbit