feat: add EIP-7702 #1112
feat: add EIP-7702 #1112
Conversation
WalkthroughThis pull request updates numerous Ethereum modules to include the block’s timestamp when creating signers via MakeSigner. The changes affect transaction signing across backends, tooling, tests, and APIs. New support is added for a SetCode transaction type and corresponding authorization handling per EIP‑7702, with adjustments to gas and receipt calculations, error variables, and JSON serialization. Additionally, enhancements in the EVM and VM layers (including EuclidV2 rules) extend support for delegation and dynamic gas costs. Changes
Sequence Diagram(s)sequenceDiagram
participant Tx as Transaction
participant B as Block (Number & Time)
participant MS as MakeSigner
participant TP as Transaction Processor
Tx->>B: Retrieve block context
B->>MS: Provide Number & Time
MS-->>TP: Return Signer instance
TP->>Tx: Sign transaction (including EIP‑7702 authorizations)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🔭 Outside diff range comments (1)
eth/gasprice/gasprice.go (1)
315-322: Add explicit error handling for block retrieval.The error from
BlockByNumbershould be checked before proceeding with block operations.Apply this diff to improve error handling:
func (oracle *Oracle) getBlockValues(ctx context.Context, blockNum uint64, limit int, ignoreUnder *big.Int, result chan results, quit chan struct{}) { block, err := oracle.backend.BlockByNumber(ctx, rpc.BlockNumber(blockNum)) - if block == nil { + if err != nil { select { case result <- results{nil, err}: case <-quit: } return } + if block == nil { + select { + case result <- results{nil, fmt.Errorf("block not found")}: + case <-quit: + } + return + }
🧹 Nitpick comments (7)
core/state_transition.go (1)
334-342: Validating code authorizations for non-creation transactions.
Restricting SetCodeAuthorizations to non-creation transactions avoids conflicts with contract creation logic. The separate check for empty authorizations is helpful, though clarifying error messages for both conditions may improve UX even more.core/tx_pool.go (1)
2181-2216: Consider using more efficient data structures for authorities.
The slices.Contains and slices.Index calls run in O(n). This is likely acceptable if the number of authorities remains small, but a map-based approach could be more scalable. Also consider lowering log severity if “Authority with untracked tx” can happen during normal operation.internal/ethapi/api.go (1)
1413-1428: SetCodeTxType support
Adding baseFee and computing GasPrice is appropriate. Confirm that yParity usage doesn’t lose the 27/28 distinction and ensure the AuthorizationList is valid before use.core/types/gen_authorization.go (1)
36-75: Robust UnmarshalJSON for required fields
The explicit nil checks are great. However, there is a mismatch: the error mentions 'yParity' while JSON field is 'v'. Consider aligning the field names in errors to avoid confusion.core/error.go (1)
114-121: Consider adding error codes for better error handling.While the state transition errors are well-defined, consider adding unique error codes to each error for better error handling and logging. This would help with error tracking and debugging.
Example implementation:
// EIP-7702 state transition errors. // Note these are just informational, and do not cause tx execution abort. var ( - ErrAuthorizationWrongChainID = errors.New("EIP-7702 authorization chain ID mismatch") - ErrAuthorizationNonceOverflow = errors.New("EIP-7702 authorization nonce > 64 bit") - ErrAuthorizationInvalidSignature = errors.New("EIP-7702 authorization has invalid signature") - ErrAuthorizationDestinationHasCode = errors.New("EIP-7702 authorization destination is a contract") - ErrAuthorizationNonceMismatch = errors.New("EIP-7702 authorization nonce does not match current account nonce") + ErrAuthorizationWrongChainID = errors.New("EIP-7702-01: authorization chain ID mismatch") + ErrAuthorizationNonceOverflow = errors.New("EIP-7702-02: authorization nonce > 64 bit") + ErrAuthorizationInvalidSignature = errors.New("EIP-7702-03: authorization has invalid signature") + ErrAuthorizationDestinationHasCode = errors.New("EIP-7702-04: authorization destination is a contract") + ErrAuthorizationNonceMismatch = errors.New("EIP-7702-05: authorization nonce does not match current account nonce")core/blockchain.go (1)
1509-1511: Consider performance and concurrency implications.The parallel signature recovery with timestamp parameter:
- May have performance implications due to additional timestamp validation.
- Could introduce race conditions if timestamp handling is not thread-safe.
- Should be tested under high load conditions.
Consider adding benchmarks and stress tests to verify performance characteristics.
core/types/transaction.go (1)
487-490: Consider using pre-allocated slice for better performance.The
appendoperation on a zero-length slice can cause unnecessary allocations.Apply this diff to pre-allocate the slice:
-auths := make([]common.Address, len(setcodetx.SetCodeAuthorizations)) +auths := make([]common.Address, 0, len(setcodetx.SetCodeAuthorizations))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
accounts/abi/bind/backends/simulated.go(3 hunks)accounts/external/backend.go(1 hunks)cmd/evm/internal/t8ntool/execution.go(1 hunks)cmd/evm/internal/t8ntool/transaction.go(2 hunks)cmd/evm/internal/t8ntool/transition.go(1 hunks)common/hexutil/json.go(2 hunks)core/bench_test.go(2 hunks)core/blockchain.go(1 hunks)core/chain_makers.go(1 hunks)core/error.go(1 hunks)core/rawdb/accessors_chain.go(1 hunks)core/state_prefetcher.go(1 hunks)core/state_processor.go(2 hunks)core/state_processor_test.go(1 hunks)core/state_transition.go(8 hunks)core/tx_pool.go(9 hunks)core/types/gen_authorization.go(1 hunks)core/types/l2trace.go(1 hunks)core/types/receipt.go(3 hunks)core/types/receipt_test.go(1 hunks)core/types/setcode_tx.go(1 hunks)core/types/transaction.go(5 hunks)core/types/transaction_marshalling.go(4 hunks)core/types/transaction_signing.go(4 hunks)core/vm/eips.go(2 hunks)core/vm/evm.go(7 hunks)core/vm/interpreter.go(1 hunks)core/vm/jump_table.go(1 hunks)core/vm/operations_acl.go(2 hunks)eth/api.go(1 hunks)eth/catalyst/api.go(1 hunks)eth/downloader/queue_test.go(1 hunks)eth/downloader/testchain_test.go(1 hunks)eth/fetcher/block_fetcher_test.go(1 hunks)eth/gasprice/gasprice.go(4 hunks)eth/state_accessor.go(1 hunks)eth/tracers/api.go(5 hunks)eth/tracers/api_test.go(1 hunks)eth/tracers/internal/tracetest/calltrace_test.go(2 hunks)eth/tracers/js/tracer.go(1 hunks)graphql/graphql.go(3 hunks)internal/ethapi/api.go(13 hunks)internal/ethapi/transaction_args.go(4 hunks)les/downloader/queue_test.go(1 hunks)les/downloader/testchain_test.go(1 hunks)les/fetcher/block_fetcher_test.go(1 hunks)les/odr_test.go(2 hunks)les/state_accessor.go(1 hunks)light/odr_test.go(1 hunks)light/odr_util.go(1 hunks)light/txpool.go(1 hunks)miner/scroll_worker.go(2 hunks)params/protocol_params.go(1 hunks)rollup/ccc/async_checker_test.go(1 hunks)rollup/tracing/tracing.go(2 hunks)tests/state_test_util.go(1 hunks)tests/transaction_test_util.go(1 hunks)
🔇 Additional comments (105)
core/types/setcode_tx.go (5)
36-47: Ensure clarity and coverage for delegation parsing.
The logic for ParseDelegation and AddressToDelegation appears correct, enforcing a prefix length of 3 plus 20 bytes for addresses. However, consider adding thorough unit tests (including invalid prefix lengths and boundary cases) to confirm correctness under all conditions.Would you like me to generate test stubs for these helper functions?
49-67: Struct design appears solid.
The SetCodeTx struct correctly groups all fields required by EIP‑7702, including the specialized slice for SetCodeAuthorizations. Storing BigInts in *uint256.Int is a good performance choice in this repository. No immediate issues found.
108-138: Validate negative scenarios in Authority recovery.
The signature validation and public key recovery logic look standard. Ensure test coverage includes invalid signatures, corrupted recovery IDs, and corner cases such as zeros in R or S.
140-214: Deep copy logic is correct, but be mindful of future nested slices.
The copy method thoroughly allocates new pointers for fields, preventing accidental aliasing. If SetCodeAuthorization grows more nested data fields in future, consider implementing a deeper copy procedure. The encode/decode RLP methods also look standard.
90-106: Minor docstring improvement and usage check.
The comment “SignSetCode creates a signed the SetCode authorization” has a small grammatical slip. Updating it to “SignSetCode creates a signed SetCode authorization” might be clearer. Also, ensure that all call sites properly handle potential errors from crypto.Sign.-// SignSetCode creates a signed the SetCode authorization. +// SignSetCode creates a signed SetCode authorization.To verify the usage, you could run a search for all references:
✅ Verification successful
🏁 Script executed:
#!/bin/bash rg -A 3 "SignSetCode"Length of output: 390
Docstring update is needed in SignSetCode
- The search confirms the function in core/types/setcode_tx.go still uses the outdated comment ("creates a signed the SetCode authorization").
- There is no evidence from the usage search that error handling of crypto.Sign is mismanaged; the error is correctly checked right after the call.
Please update the comment to:
// SignSetCode creates a signed SetCode authorization.core/vm/operations_acl.go (2)
220-225: New EIP-7702 gas variables mirror existing patterns.
Defining gasCallEIP7702 and other variants aligns well with the approach used for EIP-2929. No immediate concerns identified.
227-285: Extended call gas function is consistent with prior EIPs.
makeCallVariantGasCallEIP7702 follows the same design as EIP-2929, adding extra checks for delegation and charging cold/warm costs accordingly. Consider adding test coverage for scenarios with multiple delegations or misaligned code sizes to confirm edge cases.core/state_transition.go (5)
94-96: New interface method supports code authorizations.
Adding SetCodeAuthorizations to the Message interface is coherent with the EIP-7702 flow. No immediate issues spotted. Ensure that implementers of this interface either return nil or a valid slice as appropriate.
382-382: Intrinsic gas inclusion for authorizations.
setCodeAuthorizations are accounted in IntrinsicGas. This is likely per EIP‑7702 specs. Confirm that each authorization truly needs the extra CallNewAccountGas cost, as large authorization lists could inflate gas usage significantly.
418-435: Partial application of authorizations.
Currently, invalid authorizations are silently skipped instead of reverting. Evaluate whether partial success is acceptable or if failing one authorization should revert the entire transaction. This can significantly affect user expectations.
480-507: Robust signature and chain ID checks.
validateAuthorization properly compares chain IDs, checks for valid signatures, and ensures no conflicting on-chain code. However, consider preventing multiple authorizations from the same address with the same nonce in a single transaction if that case is undesired.
510-535: Properly updating state with EIP-7702 authorizations.
applyAuthorization updates the nonce and overwrites contract code with delegation. The refund for existing accounts is a neat optimization. Ensure test coverage includes repetitive authorizations, zero address clears, and negative scenarios (e.g., concurrency).core/types/transaction_signing.go (7)
40-44: Ensure blockTime parameter usage is consistent.
Adding the blockTime parameter to the MakeSigner function is a logical extension for checking EuclidV2 eligibility. However, confirm that all call sites are correctly updated to provide a valid blockTime and handle boundary cases (e.g., blockTime=0).
69-72: Validate EuclidV2Time check condition.
Returning a Euclid V2 signer if EuclidV2Time is non-nil may be misleading if the actual block time isn’t verified here. Make sure the chainConfig uses consistent logic for block time or an alternative check if needed.
100-100: Confirm unconditional Euclid V2 signer usage.
Always returning the Euclid V2 signer for any chainID might introduce unintended behavior if a chain isn’t actually Euclid-based. Consider restricting this or verifying chain support.
197-210: Implementation for SetCodeTxType looks correct.
The special-case logic for 0/1 as the recovery ID aligns with the specification for SetCodeTxType. Good job integrating it with the existing signer flow.
212-215: Equality check is straightforward.
Consistently compares chain IDs and maintains parity with other signers’ approach.
217-230: Signature value extraction is cohesive.
Appropriately delegates to the parent signer if the transaction isn’t a SetCodeTxType, reducing duplicated logic. Looks good.
232-252: Hash calculation is coherent.
The distinct hashing logic for SetCodeTxType closely matches the pattern for other transaction types, ensuring consistency in prefix usage.core/vm/evm.go (3)
27-27: Importing “core/types” is appropriate.
The new import is necessary for referencing delegation utilities in the EVM logic.
233-233: Consistently applying resolveCode/resolveKeccakCodeHash.
Updating the contract invocation and creation paths to call the new resolution methods for code/delegation is consistent. Verify that no usage site still calls StateDB.GetCode/GetKeccakCodeHash directly, to avoid missing the delegation logic.Also applies to: 301-301, 345-345, 401-401, 450-450
542-569: Single-level delegation resolution.
This design only follows one level of delegation. Consider whether multi-level delegation or cycle detection is needed. Repeated or cyclical delegation references may introduce unexpected behaviors.graphql/graphql.go (1)
230-230: Extended logic for SetCodeTxType.
Equating SetCodeTxType with DynamicFeeTxType for gas calculations is consistent if both share the same fee mechanics. Ensure the rest of the codebase also aligns with these assumptions (e.g., base fee usage, priority fees).Also applies to: 266-266, 281-281
core/tx_pool.go (8)
81-83: New error declaration looks good.
Declaring a dedicated error for account-limit violations makes the code more expressive.
98-101: Error for authority conflicts is adequately introduced.
The newly added error for conflicting authority usage provides clear feedback for invalid transactions.
809-827: Authorization conflict checks appear correct.
Calling knownConflicts to gather conflicting addresses helps prevent transaction collisions. Just ensure pool.mu remains locked for thread safety.
828-832: Mandatory authorization constraint looks good.
Requiring at least one tuple for SetCode transactions is consistent with the specification.
847-847: IntrinsicGas call update is aligned with new code authorizations.
Passing tx.SetCodeAuthorizations() ensures correct gas calculation for the new transaction type.
2029-2029: Field initialization is consistent.
Instantiating the new map in txLookup: auths is correct.
2128-2146: addAuthorities and removeAuthorities calls fit into the lifecycle.
These lines correctly integrate the new authority tracking whenever transactions are added or removed.
24-24: Confirm Go version compatibility for "slices" import.
Ensure your build environment uses Go 1.21 or above, since "slices" was introduced in Go 1.21.✅ Verification successful
🌐 Web query:
Which version of Go introduced the "slices" package in the standard library?💡 Result:
The `slices` package was introduced into the Go standard library with **Go 1.21** [8]. This release added generic functions for common slice operations like sorting, searching, and manipulation. Key details: - **Official inclusion**: The `slices` package became part of `std` in Go 1.21 after being tested in `golang.org/x/exp/slices` during earlier versions[5][8]. - **Key features**: - Sorting and binary search (`Sort`, `BinarySearch`)[5][7] - Slice manipulation (`Clip`, `Compact`, `Grow`)[9][10] - Comparisons (`Equal`, `Compare`)[8][10] Earlier versions like Go 1.18–1.20 only included experimental versions via `golang.org/x/exp/slices`[5][9]. Subsequent releases like Go 1.23 later enhanced it with iterator support[3]. Citations: - 1: https://pkg.go.dev/slices - 2: https://tip.golang.org/doc/go1.20 - 3: https://www.bytesizego.com/blog/golang-new-slice-map-features - 4: https://www.dolthub.com/blog/2024-12-20-collection-functions-in-go-1-23/ - 5: https://www.ardanlabs.com/blog/2023/08/golang-slices-binary-search.html - 6: https://www.ompluscator.com/article/golang/standard-lib-slices-part2/ - 7: https://www.ompluscator.com/article/golang/standard-lib-slices-part1/ - 8: https://tip.golang.org/doc/go1.21 - 9: https://pkg.go.dev/golang.org/x/exp/slices - 10: https://www.gopherguides.com/articles/golang-slices-package
Final Verification: Go Version Compatibility Confirmed
The web search confirms that the "slices" package was introduced in Go 1.21. No further modifications are necessary as long as the build environment is set to Go 1.21 or higher.
internal/ethapi/api.go (4)
891-891: Add blockTime to MakeSigner
Including the block’s timestamp helps ensure correctness for EIP-1559 logic. This looks good.
995-995: Use header.Time in MakeSigner
Passing the block timestamp aligns with the updated signature scheme. Good addition.
1392-1392: Verify handling of V for yParity
Using v.Sign() only distinguishes negative, zero, or positive. Verify that this behavior covers legacy V values (27 or 28) correctly.
1441-1445: Fallback for blockTime
If current is nil, blockTime remains zero. This might cause unexpected behavior. Consider a sensible default or a warning log.core/types/gen_authorization.go (1)
16-34: MarshalJSON ensures required fields
The approach for converting each field into a hex-friendly type is good. Consider checking for field bounds (e.g., address length) if invalid data might slip through.rollup/ccc/async_checker_test.go (1)
41-41: Include block.Time in signer
Adding the block timestamp to MakeSigner ensures accurate testing of EIP-1559 transactions.les/state_accessor.go (1)
56-56: LGTM! Block timestamp added to MakeSigner.The addition of
block.Time()to the MakeSigner call aligns with EIP-7702 requirements and maintains consistency with similar changes across the codebase.tests/transaction_test_util.go (1)
58-58: LGTM! SetCodeAuthorizations added to IntrinsicGas calculation.The addition of
tx.SetCodeAuthorizations()to the IntrinsicGas calculation correctly integrates EIP-7702's SetCode transaction type while maintaining existing error handling patterns.core/state_prefetcher.go (1)
58-58: LGTM! Block timestamp added to MakeSigner.The addition of
header.Timeto the MakeSigner call aligns with EIP-7702 requirements and maintains consistency with similar changes across the codebase.core/error.go (1)
109-112: LGTM! Clear message validation errors for EIP-7702.The message validation errors are well-defined and essential for preventing invalid transactions:
ErrEmptyAuthList: Prevents transactions with empty authorization listsErrSetCodeTxCreate: Prevents misuse of EIP-7702 transactions for contract creationcmd/evm/internal/t8ntool/transaction.go (2)
116-116: LGTM: Updated MakeSigner with timestamp parameter.The addition of timestamp parameter (0) aligns with the broader changes for EIP-7702 implementation across the codebase.
143-144: LGTM: Added SetCodeAuthorizations to intrinsic gas calculation.The addition of
tx.SetCodeAuthorizations()parameter toIntrinsicGasis consistent with EIP-7702 requirements for handling set-code transactions.core/types/l2trace.go (1)
148-150: LGTM: Updated NewTransactionData signature with blockTime parameter.The addition of
blockTimeparameter and its usage inMakeSignercall maintains consistency with the broader EIP-7702 implementation changes.les/downloader/testchain_test.go (1)
129-130: LGTM: Updated test to include block timestamp in MakeSigner.The test has been correctly updated to use
block.Time()in theMakeSignercall, maintaining consistency with the implementation changes.eth/downloader/testchain_test.go (1)
129-130: LGTM: Updated test to include block timestamp in MakeSigner.The test has been correctly updated to use
block.Time()in theMakeSignercall, maintaining consistency with the implementation changes.core/state_processor.go (2)
193-193: Block timestamp added to transaction signing.The addition of
header.TimetoMakeSignermaintains consistency with the changes in theProcessfunction.
99-99: Block timestamp added to transaction signing.The addition of
header.TimetoMakeSigneraligns with EIP-7702 requirements for including block timestamps in transaction validation.Let's verify that all transaction signing operations consistently use the block timestamp:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for consistent usage of block timestamp in MakeSigner calls. # Test: Search for MakeSigner calls. Expect: All calls should include block timestamp. rg -A 2 'MakeSigner\(' | grep -v 'header.Time'Length of output: 11439
Consistent usage of block timestamps in production transaction signing verified
Our investigation shows that in production code every MakeSigner call uses a block timestamp—whether from header.Time, block.Time(), or equivalent. (Note that some test files, such as cmd/evm/internal/t8ntool/transaction.go and core/bench_test.go, pass 0 as the timestamp, which appears intentional for testing purposes and does not affect production behavior.)
core/vm/eips.go (2)
41-41: EIP-7702 activator added.The addition of EIP-7702 to the activators map enables its gas cost modifications.
276-282: Gas cost modifications for call operations.The function updates gas costs for CALL, CALLCODE, STATICCALL, and DELEGATECALL operations according to EIP-7702 specifications.
Let's verify that the gas cost functions are properly implemented:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for the implementation of EIP-7702 gas cost functions. # Test: Search for gas cost function implementations. Expect: All referenced functions should be defined. rg -A 5 'gasCallEIP7702|gasCallCodeEIP7702|gasStaticCallEIP7702|gasDelegateCallEIP7702'Length of output: 1060
Gas cost modifications are correctly implemented.
The output confirms that all EIP‑7702 gas cost functions (gasCallEIP7702, gasCallCodeEIP7702, gasStaticCallEIP7702, gasDelegateCallEIP7702) are defined in core/vm/operations_acl.go and properly referenced in core/vm/eips.go. No issues were found.
eth/state_accessor.go (1)
183-183: Block timestamp added to transaction signing.The addition of
block.Time()toMakeSignermaintains consistency with other transaction signing operations.accounts/external/backend.go (1)
221-221: SetCodeTxType added to dynamic fee transactions.The addition of
SetCodeTxTypeto the dynamic fee transaction case ensures consistent gas pricing behavior for the new transaction type.Let's verify that SetCodeTxType is consistently handled across the codebase:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for consistent handling of SetCodeTxType. # Test: Search for SetCodeTxType handling. Expect: All transaction type checks should include SetCodeTxType. rg -A 5 'SetCodeTxType'Length of output: 5331
Subject: Verified: Consistent Handling of SetCodeTxType Across the Codebase
The search results confirm that SetCodeTxType is consistently integrated in all relevant parts of the codebase. In particular:
Transaction Processing:
- In accounts/external/backend.go, dynamic fee transactions now include SetCodeTxType alongside DynamicFeeTxType.
- In core/tx_pool.go, explicit checks for SetCodeTxType ensure proper validation.
Signature & Marshalling:
- In core/types/transaction_signing.go and core/types/transaction_marshalling.go, SetCodeTxType is adequately handled for signature verification and marshalling.
- core/types/receipt.go and core/types/transaction.go include SetCodeTxType in both receipt handling and type definition.
API and GraphQL Integration:
- The endpoints in internal/ethapi/api.go and internal/ethapi/transaction_args.go as well as transaction processing in graphql/graphql.go reflect the updated type handling.
Based on the above evidence, the inclusion of SetCodeTxType is applied consistently throughout the codebase.
core/bench_test.go (2)
133-133: Consider using realistic timestamps in benchmarks.Similar to the previous instance, using a hardcoded timestamp of
0might not provide realistic benchmark results if transaction signing behavior is timestamp-dependent.
88-89: Verify the impact of using hardcoded timestamp in benchmarks.The hardcoded timestamp value of
0inMakeSignermight not accurately represent real-world scenarios where block timestamps are non-zero.Run the following script to check if there are any timestamp-dependent validations in the signer:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Search for timestamp-dependent validations in signer code ast-grep --pattern 'func ($name *Signer) ($method) { $$$ time $$$ $$$ }'Length of output: 80
I'm generating additional shell scripts to inspect the definition of MakeSigner and how the timestamp parameter is used. This will help assess if hardcoding the timestamp to 0 has any impact in terms of validations or logic.
🏁 Script executed:
#!/bin/bash # Search for the definition of MakeSigner with surrounding context. rg -n -A 10 -B 3 "func MakeSigner(" .Length of output: 122
🏁 Script executed:
#!/bin/bash # Identify the first Go file containing the definition of MakeSigner and inspect it for references to "timestamp". file=$(rg --files-with-matches "func MakeSigner(" -t go | head -n 1) if [ -n "$file" ]; then echo "Inspecting file: $file" rg -n "timestamp" "$file" fiLength of output: 171
I'm generating new shell scripts using an escaped pattern to correctly search for the definition of MakeSigner and any references to the timestamp parameter. This will help us verify whether hardcoding 0 for the timestamp might impact any validations in the signer.
🏁 Script executed:
#!/bin/bash # Search for the MakeSigner definition with context using an escaped parenthesis. rg -n -A 10 -B 3 "func MakeSigner\(" -tgo .Length of output: 1137
🏁 Script executed:
#!/bin/bash # Identify the first Go file containing the definition of MakeSigner and inspect it for any use of "timestamp" file=$(rg --files-with-matches "func MakeSigner\\(" -t go | head -n 1) if [ -n "$file" ]; then echo "Inspecting file: $file" rg -n "timestamp" "$file" fiLength of output: 316
Signer Benchmark Timestamp Review
Our investigation shows that the signer code (in core/types/transaction_signing.go) simply forwards the provided blockTime value to the chain configuration functions (e.g. config.IsEuclidV2). There are no explicit validations or adjustments on the timestamp inside the signer code. In benchmarks, passing a hardcoded value of 0 only fixes the branch decision according to the chain configuration. While this may limit the benchmarking scenario to a specific signer branch, it does not trigger any error conditions or validations.
• The MakeSigner function uses the blockTime value only for branch selection based on chain configuration (e.g. config.IsEuclidV2(blockTime)).
• No timestamp-dependent validations were found in the signer code.Given this, the hardcoded 0 in benchmarks does not impact correctness, though you may wish to document or consider parameterizing it if tests need to reflect a wider range of conditions.
core/vm/interpreter.go (1)
77-78: LGTM! Addition of EuclidV2 instruction set follows the established pattern.The implementation correctly adds support for EuclidV2 chain rules by assigning the appropriate instruction set.
eth/catalyst/api.go (1)
164-164: LGTM! Block timestamp correctly included in signer creation.The change properly incorporates the block's timestamp when creating the signer, maintaining consistency with the updated signer interface.
light/odr_util.go (1)
178-178: LGTM! Block timestamp correctly included in receipt field derivation.The change properly incorporates the block's timestamp when deriving receipt fields, maintaining consistency with the updated receipt derivation interface.
internal/ethapi/transaction_args.go (3)
58-59: LGTM: New field for SetCodeTx authorizations.The addition of
AuthorizationListfield to support EIP-7702's SetCodeTx type is well-documented and properly typed.
264-284: LGTM: Comprehensive SetCodeTx handling.The implementation properly handles the SetCodeTx case with all required fields:
- Checks for AuthorizationList presence
- Handles AccessList correctly
- Uses uint256 for numeric fields
- Includes all necessary transaction parameters
255-255: Verify authorization list handling in NewMessage.The authorization list is now passed to
types.NewMessage, but we should verify that upstream code properly handles this parameter.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check NewMessage implementation for authorization list handling ast-grep --pattern 'func NewMessage($$$) types.Message { $$$ }'Length of output: 67
I'll now run a more general search using grep to locate the implementation of NewMessage and verify how the authorization list parameter is handled.
🏁 Script executed:
#!/bin/bash # Searching for the implementation of NewMessage in the codebase. rg -n "func NewMessage" .Length of output: 310
I'll now inspect the implementation of NewMessage to verify how the authorization list parameter (
setCodeAuthorizations) is handled. We need to ensure that the parameter passed from upstream is correctly stored and used by the Message type. I'll run a script to print a snippet of the NewMessage function implementation.
🏁 Script executed:
#!/bin/bash # Print a snippet of the NewMessage function implementation to inspect authorization list handling sed -n '770,900p' core/types/transaction.goLength of output: 3551
Authorization List Handling in NewMessage is Properly Implemented
The NewMessage constructor in core/types/transaction.go directly assigns the provided authorization list (setCodeAuthorizations) to the Message struct, and the accompanying getter returns it unchanged. Upstream code in internal/ethapi/transaction_args.go passes the list correctly. There are no issues with how the authorization list is handled.
light/odr_test.go (1)
203-203: LGTM: Updated signer creation with timestamp.The addition of
header.TimetoMakeSigneraligns with EIP-7702 requirements for including block timestamp in transaction signing.common/hexutil/json.go (2)
231-234: LGTM: Well-defined U256 type.The U256 type is properly defined as a wrapper around uint256.Int with clear documentation.
241-259: Robust JSON unmarshaling implementation.The UnmarshalJSON implementation includes:
- Strict string format checking
- Proper handling of empty strings
- Clear error messages
cmd/evm/internal/t8ntool/execution.go (1)
120-120: LGTM: Consistent signer creation with timestamp.The addition of
pre.Env.TimestamptoMakeSignermaintains consistency with EIP-7702 changes across the codebase.params/protocol_params.go (1)
92-92: LGTM: Gas cost constant for EIP-7702 auth tuple.The gas cost value of 12500 for
TxAuthTupleGasis reasonable and consistent with other similar operations in the protocol.tests/state_test_util.go (1)
358-359: LGTM: Updated message creation for EIP-7702 support.The addition of the nil parameter for auth tuples is consistent with the EIP-7702 implementation.
core/chain_makers.go (1)
153-156: LGTM: Added Time() accessor for block timestamp.The method is well-implemented and provides necessary access to block timestamp for EIP-7702 functionality.
eth/gasprice/gasprice.go (1)
323-324: LGTM: Improved signer handling in getBlockValues.Moving signer creation inside the method improves encapsulation and correctly uses block time for EIP-7702 compatibility.
eth/downloader/queue_test.go (1)
50-50: LGTM! Correctly updated MakeSigner call.The change properly includes the block timestamp in the MakeSigner call, maintaining consistency with EIP-7702 requirements.
les/downloader/queue_test.go (1)
50-50: LGTM! Correctly updated MakeSigner call.The change properly includes the block timestamp in the MakeSigner call, maintaining consistency with EIP-7702 requirements.
cmd/evm/internal/t8ntool/transition.go (1)
244-244: LGTM! Correctly updated MakeSigner call.The change properly includes the prestate environment timestamp in the MakeSigner call, maintaining consistency with EIP-7702 requirements.
eth/tracers/internal/tracetest/calltrace_test.go (2)
170-171: LGTM! Correctly updated MakeSigner call in test setup.The change properly includes the test context timestamp in the MakeSigner call, maintaining consistency with EIP-7702 requirements.
281-281: LGTM! Correctly updated MakeSigner call in benchmark setup.The change properly includes the test context timestamp in the MakeSigner call, maintaining consistency with EIP-7702 requirements.
core/types/receipt.go (2)
239-239: LGTM! Added support for SetCodeTxType.The
SetCodeTxTypeis correctly added to the list of supported transaction types in thedecodeTypedmethod, maintaining consistency with other transaction types likeDynamicFeeTxType,AccessListTxType, etc.
449-450: LGTM! Updated DeriveFields to include block timestamp.The
DeriveFieldsmethod signature is correctly updated to include thetimeparameter, which is then passed toMakeSigner. This change aligns with EIP-7702's requirement to use block timestamp for transaction signing.les/odr_test.go (1)
147-148: LGTM! Updated MakeSigner calls to include block timestamp.The
MakeSignercalls are correctly updated to includeheader.Timeparameter in both the blockchain and light client test paths. This change aligns with EIP-7702's requirement to use block timestamp for transaction signing.Also applies to: 161-162
core/types/transaction_marshalling.go (3)
47-47: LGTM! Added AuthorizationList field.The
AuthorizationListfield is correctly added to thetxJSONstruct with appropriate JSON tags and type.
171-187: LGTM! Added SetCodeTx marshalling support.The
SetCodeTxcase is properly implemented in theMarshalJSONmethod, handling all required fields including the newAuthorizationList.
444-487: LGTM! Added SetCodeTx unmarshalling support with proper validation.The
SetCodeTxcase inUnmarshalJSONincludes:
- Proper field validation
- Overflow checks for numeric fields
- Required field checks including
AuthorizationListcore/state_processor_test.go (1)
304-304: LGTM! Improved error message clarity.The error message for
ErrSenderNoEOAnow shows the length of the code instead of the code hash, making it more informative and easier to understand the reason for the error.eth/tracers/api_test.go (1)
167-167: LGTM! Block timestamp added to signer creation.The addition of
block.Time()toMakeSigneraligns with EIP-7702 requirements for including block timestamp in transaction signing.light/txpool.go (1)
423-423: Verify gas calculation for SetCode transactions.The change from
tx.To() == niltotx.SetCodeAuthorizations()in the intrinsic gas calculation affects how gas is computed for SetCode transactions. This is a significant change that impacts transaction validation.Run the following script to verify the gas calculation logic:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify the gas calculation logic for SetCode transactions. # Search for other instances of IntrinsicGas calls to ensure consistency rg -A 5 "IntrinsicGas\(" # Search for SetCodeAuthorizations usage to understand the context ast-grep --pattern $'tx.SetCodeAuthorizations()'Length of output: 4362
Gas Calculation Consistency Verified
The grep results show that the new usage—passing tx.SetCodeAuthorizations() as the third parameter while still evaluating tx.To() == nil as the fourth—is consistently applied across the codebase (in light/txpool.go, core/tx_pool.go, transaction tests, etc.). This confirms that the change for SetCode transactions in the intrinsic gas calculation is intentional and in line with existing logic.
rollup/tracing/tracing.go (1)
108-108: LGTM! Block timestamp consistently added to transaction processing.The changes correctly incorporate block timestamp in both signer creation and transaction data initialization, maintaining consistency with EIP-7702 implementation.
Also applies to: 528-528
core/types/receipt_test.go (1)
300-307: LGTM! Test cases updated to include block timestamp.The test cases have been properly updated to include block timestamp in receipt derivation and signer creation, ensuring compatibility with EIP-7702 changes.
core/vm/jump_table.go (1)
70-76: LGTM! New instruction set properly enables EIP-7702.The implementation correctly:
- Inherits from
darwinInstructionSet- Enables EIP-7702 Setcode transaction type
- Follows the established pattern for instruction set creation
accounts/abi/bind/backends/simulated.go (2)
642-642: LGTM! Updated signer creation to include block time.The change correctly updates the
MakeSignercall to include the block's timestamp, aligning with EIP-7702 requirements.
812-825: LGTM! Added SetCodeAuthorizations support to callMsg.The implementation correctly adds the
SetCodeAuthorizationsmethod to support EIP-7702's code setting functionality.core/rawdb/accessors_chain.go (1)
589-590: LGTM! Block timestamp added to receipt derivation.The addition of
header.Timeto theDeriveFieldscall aligns with EIP-7702 requirements for including block timestamps in transaction signing and receipt generation.eth/fetcher/block_fetcher_test.go (1)
55-55: LGTM! Test updated to include block time in signer creation.The test code has been correctly updated to match the production code changes for EIP-7702, adding
block.Time()to theMakeSignercall.les/fetcher/block_fetcher_test.go (1)
55-55: LGTM! Light client test updated to include block time in signer creation.The light client test code has been correctly updated to match the production code changes for EIP-7702, adding
block.Time()to theMakeSignercall.eth/api.go (1)
918-918: Verify the use of zero block time for skipped transactions.While the code has been updated to match the new
NewRPCTransactionsignature, using zero as the block time for skipped transactions might not be the best approach. Consider using the block time fromstx.BlockNumberif available.Run the following script to check if skipped transactions store block time:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if skipped transactions store block time information. # Search for block time field in skipped transaction related code rg -A 5 "type SkippedTransaction struct"Length of output: 463
Skipped Transaction Block Time Verification
The output confirms that the SkippedTransaction struct (found in core/rawdb/accessors_skipped_txs.go) does not include any field for block time. Since no block time is stored or available for skipped transactions, using zero is an appropriate placeholder.
eth/tracers/api.go (4)
278-278: LGTM!The addition of
task.block.Time()toMakeSigneris consistent with EIP-7702 implementation.
537-537: LGTM!The addition of
block.Time()toMakeSigneris consistent with EIP-7702 implementation.
605-605: LGTM!The addition of
block.Time()toMakeSigneris consistent with EIP-7702 implementation.
909-909: LGTM!The addition of
block.Time()toMakeSigneris consistent with EIP-7702 implementation.miner/scroll_worker.go (2)
626-626: LGTM!The addition of
w.current.header.TimetoMakeSigneris consistent with EIP-7702 implementation.
666-666: LGTM!The addition of
w.current.header.TimetoMakeSigneris consistent with EIP-7702 implementation.core/blockchain.go (3)
1510-1510: LGTM: Block timestamp added to MakeSigner call.The addition of
chain[0].Time()to theMakeSignerfunction call aligns with EIP-7702 requirements for incorporating block timestamps into the signing process.
1509-1511: Add unit tests for timestamp-based signature recovery.Please add tests to cover:
- Signature recovery with various timestamp values
- Edge cases (zero timestamp, future timestamp)
- Compatibility with pre-existing blocks
- Error handling for invalid timestamps
Would you like me to help generate the test cases for the timestamp-based signature recovery?
1509-1511: Verify signature recovery behavior with timestamp.Please ensure that:
- All signature recovery mechanisms are updated to handle the new timestamp parameter.
- The change maintains backward compatibility with existing blocks.
- The timestamp parameter is properly validated in the signer implementation.
Run this script to check for any inconsistencies in signature recovery implementations:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for MakeSigner usage and verify timestamp parameter handling # Test: Search for all MakeSigner calls to ensure consistent timestamp usage rg -A 5 "MakeSigner.*Time" # Test: Search for signature recovery implementations ast-grep --pattern $'func ($x *$t) RecoverSender($y *$z) { $$$ }'Length of output: 19232
Signature recovery timestamp usage verified.
The shell script output confirms that all MakeSigner invocations consistently pass the block timestamp (e.g., chain[0].Time(), block.Time(), Env.Timestamp, etc.) across various parts of the codebase. The MakeSigner implementation in core/types/transaction_signing.go correctly utilizes the blockTime parameter (e.g., for branch selection like IsEuclidV2). Additionally, there is no conflicting RecoverSender implementation detected that would contradict this behavior.
The change in core/blockchain.go to pass chain[0].Time() aligns with the existing patterns and maintains backward compatibility.
core/types/transaction.go (4)
55-55: LGTM! New transaction type added.The
SetCodeTxTypeconstant is correctly defined with a unique value of0x04, following the sequence of existing transaction types.
206-207: LGTM! SetCodeTx handling added to decodeTyped.The
decodeTypedmethod is correctly updated to handle the newSetCodeTxTypeby instantiating a newSetCodeTxobject.
471-493: LGTM! New methods for handling SetCode authorizations.The implementation of
SetCodeAuthorizationsandAuthoritiesmethods is correct:
SetCodeAuthorizationssafely type asserts and returns authorizationsAuthoritiescorrectly extracts authority addresses from authorizations with proper error handling
770-843: LGTM! Message struct and methods updated.The
Messagestruct and related methods are correctly updated to includesetCodeAuthorizations:
- Field added to struct
NewMessageconstructor updatedAsMessageupdated to populate the field- Getter method added
eth/tracers/js/tracer.go (1)
708-708: LGTM! IntrinsicGas call updated.The
IntrinsicGasfunction call is correctly updated to match the new signature by passingnilfor thesetCodeAuthorizationsparameter.
| type euclidV2Signer struct{ londonSignerWithEIP4844 } | ||
|
|
||
| // NewEuclidV2Signer returns a signer that accepts | ||
| // - EIP-7702 set code transactions | ||
| // - EIP-4844 blob transactions | ||
| // - EIP-1559 dynamic fee transactions | ||
| // - EIP-2930 access list transactions, | ||
| // - EIP-155 replay protected transactions, and | ||
| // - legacy Homestead transactions. | ||
| func NewEuclidV2Signer(chainId *big.Int) Signer { | ||
| signer, _ := NewLondonSignerWithEIP4844(chainId).(londonSignerWithEIP4844) | ||
| return euclidV2Signer{signer} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle the type assertion more explicitly.
The following snippet ignores a possible failed type assertion:
“signer, _ := NewLondonSignerWithEIP4844(chainId).(londonSignerWithEIP4844)”
If that cast fails, signer will be zero-valued. It’s safer to check the boolean and handle the error explicitly:
func NewEuclidV2Signer(chainId *big.Int) Signer {
- signer, _ := NewLondonSignerWithEIP4844(chainId).(londonSignerWithEIP4844)
+ raw := NewLondonSignerWithEIP4844(chainId)
+ signer, ok := raw.(londonSignerWithEIP4844)
+ if !ok {
+ // handle error or panic appropriately
+ panic("unexpected signer type")
+ }
return euclidV2Signer{signer}
}| usedAndLeftSlots := func(addr common.Address) (int, int) { | ||
| var have int | ||
| if list := pool.pending[addr]; list != nil { | ||
| have += list.Len() | ||
| } | ||
| if list := pool.queue[addr]; list != nil { | ||
| have += list.Len() | ||
| } | ||
| if pool.currentState.GetCode(addr) != nil { | ||
| // Allow at most one in-flight tx for delegated accounts. | ||
| return have, max(0, 1-have) | ||
| } | ||
| return have, math.MaxInt | ||
| } | ||
| if used, left := usedAndLeftSlots(from); left <= 0 { | ||
| return fmt.Errorf("%w: pooled %d txs", ErrAccountLimitExceeded, used) | ||
| } |
There was a problem hiding this comment.
Fix undeclared 'max' function and verify code-check logic.
The call to max(0, 1-have) will fail unless a helper function is declared, since there's no built-in int max. Also, confirm that checking pool.currentState.GetCode(addr) != nil correctly identifies deployed code; often, length checks (len(code) > 0) are required.
Apply this diff to define a simple max function and ensure code presence is detected by length:
func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error {
...
+ // Provide an internal max helper for integer types:
+ func intMax(a, b int) int {
+ if a > b {
+ return a
+ }
+ return b
+ }
usedAndLeftSlots := func(addr common.Address) (int, int) {
var have int
if list := pool.pending[addr]; list != nil {
have += list.Len()
}
if list := pool.queue[addr]; list != nil {
have += list.Len()
}
- if pool.currentState.GetCode(addr) != nil {
+ code := pool.currentState.GetCode(addr)
+ if len(code) > 0 {
// Allow at most one in-flight tx for delegated accounts.
- return have, max(0, 1 - have)
+ return have, intMax(0, 1 - have)
}
return have, math.MaxInt
}
...
}|
|
||
| auths map[common.Address][]*types.Transaction // All accounts with a pooled authorization | ||
| } |
There was a problem hiding this comment.
Resolve duplicated field declaration.
There appears to be a leftover conflict line for “auths” declared twice. Remove the duplicate line to avoid compilation errors.
Suggested fix:
type txLookup struct {
slots int
lock sync.RWMutex
locals map[common.Hash]*types.Transaction
remotes map[common.Hash]*types.Transaction
- auths map[common.Address][]*types.Transaction // All accounts with a pooled authorization
// Keep only one declaration of auths:
auths map[common.Address][]*types.Transaction // All accounts with a pooled authorization
}Committable suggestion skipped: line range outside the PR's diff.
1. Purpose or design rationale of this PR
TODO: migrate unit tests.
Only implementations are added, with conflict fixes. Related upstream PRs:
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:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
- Integrated block timestamps into transaction signing and gas cost calculations, enhancing transaction validation and security.
- Introduced a new transaction type that supports code authorizations for more advanced smart contract interactions.
- Improved error messaging for authorization-related issues to deliver clearer user feedback.
Refactor
- Streamlined transaction processing and updated gas calculation logic for more accurate fee assessments.
- Enhanced large number formatting for more consistent data representation across network interfaces.