Skip to content

Update trace#245

Merged
Kukoomomo merged 2 commits intomainfrom
trace_alt
Nov 18, 2025
Merged

Update trace#245
Kukoomomo merged 2 commits intomainfrom
trace_alt

Conversation

@Kukoomomo
Copy link
Copy Markdown
Contributor

@Kukoomomo Kukoomomo commented Nov 18, 2025

Summary by CodeRabbit

  • New Features
    • Added alternative fee token support: transactions can now include fee token identification and optional fee limits.
    • Enhanced L2 Token Registry tracing: expanded intrinsic storage proof collection for richer registry-related data during block tracing.

@Kukoomomo Kukoomomo requested a review from a team as a code owner November 18, 2025 06:00
@Kukoomomo Kukoomomo requested review from twcctop and removed request for a team November 18, 2025 06:00
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 18, 2025

Walkthrough

Added two optional fee-related fields to transaction trace data and expanded intrinsic storage-proof slot definitions for the L2 Token Registry used during block tracing.

Changes

Cohort / File(s) Summary
Transaction tracing fields
core/types/l2trace.go
Added two exported optional fields to TransactionData: FeeTokenID (*uint16) and FeeLimit (*hexutil.Big), with JSON tags feeTokenID and feeLimit. NewTransactionData now conditionally populates them for AltFeeTxType transactions when values are present.
L2 Token Registry storage proofs
rollup/tracing/tracing.go
Replaced the TODO/placeholder entry for the registry with L2TokenRegistryAddress and added multiple intrinsic storage proof slots (e.g., AllowListEnabledSlot, TokenRegistrySlot, TokenRegistrationSlot, PriceRatioSlot, AllowListSlot) to be fetched during tracing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Pay attention to NewTransactionData conditional population logic in core/types/l2trace.go.
  • Verify the correctness and completeness of the added storage slot keys and their mapping to L2TokenRegistryAddress in rollup/tracing/tracing.go.

Possibly related PRs

  • Implement alt token fee #205 — Adds FeeTokenID and FeeLimit to tracing structures and includes related tracing/registry changes; strong overlap with these edits.

Suggested reviewers

  • Web3Jumb0
  • FletcherMan

Poem

🐰 I hopped through code with eager paws,

Fees now told without a pause,
Registry slots from TODO freed,
Traces grow wiser — quick indeed! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Update trace' is vague and generic, failing to convey the specific changes made (adding AltFeeTxType fields and L2 Token Registry storage proofs). Revise the title to be more specific and descriptive, such as 'Add AltFeeTxType support to transaction tracing' or 'Expand L2 Token Registry storage proofs in tracing'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch trace_alt

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 79bc477 and dedef08.

📒 Files selected for processing (1)
  • rollup/tracing/tracing.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • rollup/tracing/tracing.go
⏰ 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). (1)
  • GitHub Check: Analyze (go)

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5297e55 and 79bc477.

📒 Files selected for processing (2)
  • core/types/l2trace.go (2 hunks)
  • rollup/tracing/tracing.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Kukoomomo
Repo: morph-l2/go-ethereum PR: 205
File: core/token_gas.go:140-155
Timestamp: 2025-11-10T06:56:24.937Z
Learning: In the ERC20 fee implementation (core/token_gas.go), ERC20 transfer and balance check calls use math.MaxUint64 gas limit. This is intentional given that fee tokens are restricted to an allowlist (via IsTokenActive checks and AllowListSlot/AllowListEnabledSlot in rollup/rcfg/config.go), so only vetted contracts can be invoked. The team has chosen this approach understanding the tradeoffs versus adding fixed gas stipends.
📚 Learning: 2025-11-10T06:56:24.937Z
Learnt from: Kukoomomo
Repo: morph-l2/go-ethereum PR: 205
File: core/token_gas.go:140-155
Timestamp: 2025-11-10T06:56:24.937Z
Learning: In the ERC20 fee implementation (core/token_gas.go), ERC20 transfer and balance check calls use math.MaxUint64 gas limit. This is intentional given that fee tokens are restricted to an allowlist (via IsTokenActive checks and AllowListSlot/AllowListEnabledSlot in rollup/rcfg/config.go), so only vetted contracts can be invoked. The team has chosen this approach understanding the tradeoffs versus adding fixed gas stipends.

Applied to files:

  • core/types/l2trace.go
🧬 Code graph analysis (2)
core/types/l2trace.go (2)
common/hexutil/json.go (1)
  • Big (155-155)
core/types/transaction.go (1)
  • AltFeeTxType (58-58)
rollup/tracing/tracing.go (1)
rollup/rcfg/config.go (6)
  • L2TokenRegistryAddress (45-45)
  • AllowListEnabledSlot (61-61)
  • TokenRegistrySlot (49-49)
  • TokenRegistrationSlot (52-52)
  • PriceRatioSlot (55-55)
  • AllowListSlot (58-58)
⏰ 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). (1)
  • GitHub Check: Analyze (go)
🔇 Additional comments (2)
core/types/l2trace.go (2)

141-142: LGTM!

The new fields are correctly defined with pointer types and omitempty JSON tags, allowing them to be absent for non-AltFeeTx transactions.


198-207: LGTM!

The population logic correctly guards against populating these fields for non-AltFeeTx transactions and includes appropriate checks for zero/nil values.

Comment thread rollup/tracing/tracing.go
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@Kukoomomo Kukoomomo merged commit 4a63b72 into main Nov 18, 2025
8 checks passed
@Kukoomomo Kukoomomo deleted the trace_alt branch November 18, 2025 06:46
@coderabbitai coderabbitai Bot mentioned this pull request Dec 15, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants