update trace

[Kukoomomo]

Summary by CodeRabbit

• New Features
  • Added system-call lifecycle events to tracing so tracers can suspend/resume work during system calls, reducing irrelevant tracing noise and improving tracer control.
• Bug Fixes
  • Improved token bytecode collection to capture token contract and L2 token registry code more reliably across additional transaction paths, enhancing trace completeness.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds AltFeeTx fee-token bytecode collection and introduces system-call tracing hooks; wires OnSystemCallStart/OnSystemCallEnd through the mux, native tracer, and logger, and wraps two EVM token-gas operations with system-call tracing calls.

Changes

Cohort / File(s)	Change Summary
AltFeeTx token bytecode collection rollup/tracing/tracing.go	Adds handling for AltFeeTx with non-zero FeeTokenID: fetches token info and, when valid, collects code, keccak, and poseidon hashes for the token contract and L2 token registry into env.Codes using a local helper, skipping on errors or zero addresses and avoiding duplicates.
System-call tracing hooks (mux & logger) rollup/tracing/mux.go, eth/tracers/logger/logger.go	Wires new hooks OnSystemCallStartV2 / OnSystemCallEnd in the mux; adds OnSystemCallStart(env *tracing.VMContext) and OnSystemCallEnd() on the mux to forward to sub-tracers and the struct logger; adds StructLogger.OnSystemCallStart(env *tracing.VMContext) to set skip behavior.
Native call tracer: skip during system calls eth/tracers/native/call.go	Adds OnSystemCall and OnSystemCallEnd to toggle a skip flag; adds early-return guards in OnEnter, OnExit, OnTxStart, OnTxEnd, and OnLog to bypass tracer work during system calls; wires these hooks in the tracer constructor.
EVM token-gas ops wrapped with system-call tracing core/token_gas.go	Surrounds GetAltTokenBalanceByEVM and transferAltTokenByEVM with tracer calls: invoke OnSystemCallStartV2(evm.GetVMContext()) before the EVM call and defer OnSystemCallEnd() when tracer hooks are present.
Manifest go.mod	Module file present in diff; no functional code changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant TokenGas as core/token_gas
    participant TracerMux as rollup/tracing/mux
    participant SubTracer as eth/tracers/native/call
    participant Logger as StructLogger
    participant EVM as EVM operation

    Note over TokenGas,TracerMux: Before EVM token system call
    TokenGas->>TracerMux: OnSystemCallStartV2(env)
    TracerMux->>SubTracer: OnSystemCallStartV2(env)   %% new/changed
    TracerMux->>Logger: OnSystemCallStart(env)
    Note over SubTracer: sets skip = true
    TokenGas->>EVM: perform balanceOf / transfer (system call)
    EVM-->>TokenGas: result
    TokenGas->>TracerMux: OnSystemCallEnd()
    TracerMux->>SubTracer: OnSystemCallEnd()
    TracerMux->>Logger: OnSystemCallEnd()
    Note over SubTracer: sets skip = false

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Check correctness of bytecode deduplication and poseidon/keccak presence checks in rollup/tracing/tracing.go.
• Verify nil-checks and proper defer usage for OnSystemCallEnd() in core/token_gas.go to avoid imbalance on early returns.
• Confirm skip flag semantics and that all tracer entry points respect the skip guard in eth/tracers/native/call.go.

Possibly related PRs

• Update trace #245: Modifies rollup/tracing/tracing.go around L2 token registry and storage-proof handling — likely related to the added AltFeeTx token bytecode/registry logic.

Suggested reviewers

• twcctop
• FletcherMan

Poem

🐇 In bytecode burrows I hop and peep,
I gather hashes, secrets to keep,
When system calls whisper, I gently pause,
Then hop back bright to log the cause,
A rabbit guard for bytes I keep. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'update trace' is vague and generic, using non-descriptive terms that don't convey meaningful information about what aspect of tracing is being updated.	Consider a more descriptive title that specifies the main change, such as 'Add system call tracing hooks to token operations' or 'Implement bytecode collection for AltFeeTx token contracts'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ 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_code

---

📜 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 6d55dd9 and b3cccaf.

📒 Files selected for processing (1)

• core/token_gas.go (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 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.

⏰ 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/token_gas.go (2)

92-96: LGTM! Tracing hooks correctly instrument the balanceOf system call.

The nil checks are thorough, and the deferred OnSystemCallEnd ensures proper cleanup regardless of the execution path.

---

145-149: Verify the intended scope of transfer tracing.

The tracing hooks correctly instrument the transfer call. However, note that the trace scope includes the post-transfer balance verification (line 166), which will trigger nested tracing (an inner GetAltTokenBalanceByEVM trace within the outer transfer trace).

If the tracing framework's "skip behavior" handles this hierarchically, this is fine. Otherwise, consider whether the transfer trace should end before line 166.

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

rollup/tracing/tracing.go (2)

379-381: Consider expanding the comment to explain L2TokenRegistryAddress collection.

The comment explains why token contract bytecode needs manual collection, but it would be helpful to also mention why the L2 token registry bytecode is collected.

Apply this diff to improve the comment:

-	// For AltFeeTx, manually collect token contract bytecode
-	// since direct storage slot operations don't trigger EVM execution
+	// For AltFeeTx, manually collect token contract and L2 token registry bytecode
+	// since direct storage slot operations don't trigger EVM execution that would
+	// normally capture these contracts via tracing

---

385-404: Consider adding debug logging for observability.

While the implementation is correct, adding debug logging when bytecode is collected would improve observability and help with troubleshooting.

Consider adding logging after successful collection:

collectBytecode := func(addr common.Address) {
	code := statedb.GetCode(addr)
	keccakCodeHash := statedb.GetKeccakCodeHash(addr)
	poseidonCodeHash := statedb.GetPoseidonCodeHash(addr)
	codeSize := statedb.GetCodeSize(addr)

	if poseidonCodeHash != (common.Hash{}) {
		if _, exists := env.Codes[poseidonCodeHash]; !exists {
			env.Codes[poseidonCodeHash] = logger.CodeInfo{
				CodeSize:         codeSize,
				KeccakCodeHash:   keccakCodeHash,
				PoseidonCodeHash: poseidonCodeHash,
				Code:             code,
			}
+			log.Debug("Collected bytecode for AltFeeTx token-related contract", 
+				"address", addr.Hex(), "codeSize", codeSize, "txHash", tx.Hash().Hex())
		}
	}
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d085f8c and 8f063aa.

📒 Files selected for processing (1)

• rollup/tracing/tracing.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

rollup/tracing/tracing.go (5)

core/types/transaction.go (1)

• AltFeeTxType (58-58)

rollup/fees/token_info.go (1)

• GetTokenInfo (90-92)

eth/tracers/logger/logger.go (1)

• CodeInfo (205-210)

crypto/codehash/codehash.go (2)

• KeccakCodeHash (16-18)
• PoseidonCodeHash (12-14)

rollup/rcfg/config.go (1)

• L2TokenRegistryAddress (45-45)

⏰ 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 (1)

rollup/tracing/tracing.go (1)

379-405: Method signatures verified—no issues found.

All type checks are correct:

• tx.Type() returns uint8 and types.AltFeeTxType is 0x7F (uint8 constant)
• tx.FeeTokenID() correctly returns uint16, matching the parameter type expected by fees.GetTokenInfo(state StateDB, tokenID uint16)

The implementation is sound.