Skip to content

Fix/delegated code failed witness missing#1270

Merged
georgehao merged 8 commits into
developfrom
fix/delegated_code_failed_witness_missing
Jan 19, 2026
Merged

Fix/delegated code failed witness missing#1270
georgehao merged 8 commits into
developfrom
fix/delegated_code_failed_witness_missing

Conversation

@georgehao
Copy link
Copy Markdown
Member

@georgehao georgehao commented Jan 14, 2026

1. Purpose or design rationale of this PR

Essentially, the setting of witness is different from upstream

  • Upstream ProcessBlock -> statedb.StartPrefetcher("chain", witness, ...), it can GetCode to put the pre-check failure to witness
  • But srcroll's old logic is call statedb.WithWitness -> blockchain.Processor().Process to load witness, if the pre-check failed, will return and don't add the failed delegated code to witness.

2. PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

  • build: Changes that affect the build system or external dependencies (example scopes: yarn, eslint, typescript)
  • ci: Changes to our CI configuration files and scripts (example scopes: vercel, github, cypress)
  • docs: Documentation-only changes
  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that doesn't fix a bug, or add a feature, or improves performance
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • test: Adding missing tests or correcting existing tests

3. Deployment tag versioning

Has the version in params/version.go been updated?

  • This PR doesn't involve a new deployment, git tag, docker image tag, and it doesn't affect traces
  • Yes

4. Breaking change label

Does this PR have the breaking-change label?

  • This PR is not a breaking change
  • Yes

Summary by CodeRabbit

  • Chores

    • Bumped patch version.
  • Bug Fixes

    • When a transfer would exceed the sender’s balance, the runtime now materializes delegated contract code before returning an insufficient-balance error (per EIP-7702), ensuring delegated code is available even on balance-failure paths.

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

@georgehao georgehao requested a review from Thegaram January 14, 2026 08:35
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

Bumps VersionPatch (2 → 3) and, in core/vm/evm.go, on Call failing for insufficient balance, reads msg.To code, parses for a delegation target, and materializes delegated code into the witness before returning ErrInsufficientBalance. (No other control-flow changes.)

Changes

Cohort / File(s) Summary
Version bump
params/version.go
Incremented VersionPatch from 2 to 3, updating derived version strings (Version, VersionWithMeta, ArchiveVersion).
EVM: delegated-code materialization on balance-failure
core/vm/evm.go
In Call, on insufficient-balance error path, read code at msg.To, parse for delegation target, and load delegated target code into the witness before returning ErrInsufficientBalance (introduces a side-effect on the error path; no other control-flow changes).

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant EVM
  participant State
  participant Parser

  Caller->>EVM: Call(msg)
  EVM->>State: Check balance(msg.From)
  alt insufficient balance
    EVM->>State: Read code at msg.To
    State->>Parser: Provide code
    Parser->>Parser: Parse for delegation target
    Parser->>State: Load delegated target code into witness
    EVM-->>Caller: return ErrInsufficientBalance
  else sufficient balance
    EVM->>State: proceed with normal call flow
    EVM-->>Caller: return result
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

bump-version

Suggested reviewers

  • Thegaram
  • jonastheis

Poem

🐇
I hop from two to three with glee,
When funds run short, I still go see.
I fetch the code the call might need,
So even failed hops leave a deed. 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title 'Fix/delegated code failed witness missing' does not follow the conventional commits format required by the template. Update the title to follow conventional commits format, e.g., 'fix: add delegated code to witness on failed balance check' or similar.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description includes all required sections: purpose/design rationale, conventional commit type selection (fix chosen), deployment versioning confirmation, and breaking change assessment.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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

@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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
core/vm/evm.go (1)

279-293: Add EIP-7702 delegation witness loading to CallCode for parity with Call.

The CallCode method at line 291 is missing the EIP-7702 witness fix present in the Call method. When the balance check fails, CallCode should load delegated code into the witness before returning ErrInsufficientBalance, consistent with the logic at lines 193-199 in Call:

if !evm.Context.CanTransfer(evm.StateDB, caller.Address(), value) {
    // EIP-7702: Ensure delegated code is loaded into witness even when balance check fails.
    if code := evm.StateDB.GetCode(addr); len(code) > 0 {
        if target, ok := types.ParseDelegation(code); ok {
            evm.StateDB.GetCode(target)
        }
    }
    return nil, gas, ErrInsufficientBalance
}
🧹 Nitpick comments (1)
core/vm/evm.go (1)

193-199: Consider adding IsEuclidV2 guard for consistency.

The resolveCode and resolveKeccakCodeHash methods (lines 555-580) both guard delegation parsing with evm.chainRules.IsEuclidV2. This new code path parses delegation without the version check.

While ParseDelegation would return false for non-delegation code, adding the guard maintains consistency and avoids unnecessary work pre-EuclidV2:

Suggested fix
 	// Fail if we're trying to transfer more than the available balance
 	if value.Sign() != 0 && !evm.Context.CanTransfer(evm.StateDB, caller.Address(), value) {
 		// EIP-7702: Ensure delegated code is loaded into witness even when balance check fails.
 		// revm always loads the code regardless of balance check result.
-		if code := evm.StateDB.GetCode(addr); len(code) > 0 {
-			if target, ok := types.ParseDelegation(code); ok {
-				evm.StateDB.GetCode(target)
+		if evm.chainRules.IsEuclidV2 {
+			if code := evm.StateDB.GetCode(addr); len(code) > 0 {
+				if target, ok := types.ParseDelegation(code); ok {
+					evm.StateDB.GetCode(target)
+				}
 			}
 		}
 		return nil, gas, ErrInsufficientBalance
 	}
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c0d0ad0 and c68d6d5.

📒 Files selected for processing (1)
  • core/vm/evm.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). (4)
  • GitHub Check: build-and-push
  • GitHub Check: semgrep/ci
  • GitHub Check: test
  • GitHub Check: Analyze (go)

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Thegaram
Thegaram previously approved these changes Jan 19, 2026
…m:scroll-tech/go-ethereum into fix/delegated_code_failed_witness_missing
@georgehao georgehao merged commit 3d0604b into develop Jan 19, 2026
14 checks passed
@georgehao georgehao deleted the fix/delegated_code_failed_witness_missing branch January 19, 2026 13:59
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.

3 participants