feat: add setcode transaction type for compatibility#189
Conversation
WalkthroughThis set of changes introduces a new EIP-7702 "SetCode" transaction type, including its data structures, JSON marshaling/unmarshaling logic, and cryptographic signature handling. New types for 256-bit unsigned integers and transaction authorizations are added, along with associated methods for serialization and validation. The transaction processing code is extended to recognize and process the new transaction type, including extracting authorization lists and authorities. Supporting utilities for delegation address parsing and signing are provided, and tests ensure correct delegation parsing. The patch version is incremented to reflect these updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant JSON
participant Transaction
participant SetCodeTx
participant SetCodeAuthorization
User->>JSON: Submit SetCodeTx JSON
JSON->>Transaction: UnmarshalJSON
Transaction->>SetCodeTx: Decode fields
SetCodeTx->>SetCodeAuthorization: Unmarshal authorizations
SetCodeAuthorization->>SetCodeAuthorization: Validate signature fields
SetCodeTx->>Transaction: Return SetCodeTx instance
Transaction->>User: Transaction object with authorizations
sequenceDiagram
participant Signer
participant SetCodeAuthorization
participant ECDSA
Signer->>SetCodeAuthorization: Request signature
SetCodeAuthorization->>ECDSA: Compute sigHash, sign with private key
ECDSA-->>SetCodeAuthorization: Signature (V, R, S)
SetCodeAuthorization-->>Signer: Signed authorization
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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: 4
🧹 Nitpick comments (5)
core/types/tx_setcode_test.go (1)
25-70: Test coverage for delegation address parsing.This test validates the
ParseDelegationfunction which is critical for the SetCode transaction type. The test uses table-driven testing to cover:
- Valid delegation with proper prefix and address
- Various error scenarios (incorrect address lengths, wrong prefixes, missing components)
This helps ensure robust handling of delegation designators in the new transaction type.
However, it would be beneficial to add assertions to verify the actual parsed address value in the success case, not just that parsing succeeded.
- if !ok && tt.want != nil { - t.Fatalf("failed to parse, want %s", tt.want.Hex()) - } + if !ok && tt.want != nil { + t.Fatalf("failed to parse, want %s", tt.want.Hex()) + } else if ok && tt.want != nil && *tt.want != got { + t.Fatalf("parsed %s, want %s", got.Hex(), tt.want.Hex()) + }core/types/gen_authorization.go (1)
18-26: Minor: avoid shadowing the exported type nameThe inner alias
type SetCodeAuthorization struct { … }shadows the exported type name declared below (l.72).
This is harmless but can be confusing when stepping through with a debugger.A more conventional pattern:
type setCodeAuthorizationJSON struct { … }Not a blocker – just a small readability gain.
core/types/transaction_marshalling.go (1)
497-500: Prefer non‑panicking conversion for fee caps
uint256.MustFromBigwill panic on overflow.
For external JSON input this is avoidable foot‑gun – we already call the non‑panicking form (FromBig) a few lines above forchainId.-itx.GasTipCap = uint256.MustFromBig((*big.Int)(dec.MaxPriorityFeePerGas)) -itx.GasFeeCap = uint256.MustFromBig((*big.Int)(dec.MaxFeePerGas)) +if itx.GasTipCap, overflow = uint256.FromBig((*big.Int)(dec.MaxPriorityFeePerGas)); overflow { + return errors.New("'maxPriorityFeePerGas' overflows uint256") +} +if itx.GasFeeCap, overflow = uint256.FromBig((*big.Int)(dec.MaxFeePerGas)); overflow { + return errors.New("'maxFeePerGas' overflows uint256") +}core/types/tx_setcode.go (2)
90-106: Validate signature immediately after signing
SignSetCodereturns an authorisation that might have an invalid (non‑canonical) signature ifcrypto.Signis ever swapped or called with a non‑homestead rule set.
Defensive check keeps invariants local.sig, err := crypto.Sign(sighash[:], prv) if err != nil { return SetCodeAuthorization{}, err } - r, s, _ := decodeSignature(sig) + r, s, _ := decodeSignature(sig) + if !crypto.ValidateSignatureValues(sig[64], r, s, true) { + return SetCodeAuthorization{}, errors.New("signSetCode produced invalid signature values") + }
197-206: Minor optimisation: avoid negative intermediate in gas price calcWhen
gasFeeCap < baseFee,dst.Sub(GasFeeCap, baseFee)produces a negative value only to be discarded.
UsingMath.Min‑style logic is clearer & avoids a heap alloc under the hood.if baseFee == nil || tx.GasFeeCap.CmpUint64(0) == 0 { return dst.Set(tx.GasFeeCap.ToBig()) } cap := tx.GasFeeCap.ToBig() if cap.Cmp(baseFee) < 0 { return dst.Set(cap) // gasFeeCap already lower than baseFee } …
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
common/hexutil/json.go(2 hunks)core/types/gen_authorization.go(1 hunks)core/types/transaction.go(3 hunks)core/types/transaction_marshalling.go(3 hunks)core/types/tx_setcode.go(1 hunks)core/types/tx_setcode_test.go(1 hunks)params/version.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
core/types/gen_authorization.go (2)
core/types/tx_setcode.go (1)
SetCodeAuthorization(72-79)common/hexutil/json.go (2)
U256(233-233)Uint64(275-275)
core/types/tx_setcode.go (6)
common/types.go (1)
BytesToAddress(206-210)common/hexutil/json.go (3)
Bytes(40-40)U256(233-233)Uint64(275-275)core/types/gen_authorization.go (2)
SetCodeAuthorization(18-25)SetCodeAuthorization(38-45)crypto/crypto.go (3)
ValidateSignatureValues(264-275)SignatureLength(41-41)Keccak256(78-86)core/types/transaction.go (3)
ErrInvalidSig(36-36)TxData(80-100)SetCodeTxType(54-54)rlp/decode.go (1)
DecodeBytes(92-106)
🔇 Additional comments (8)
common/hexutil/json.go (3)
27-27: New dependency added for uint256 support.The code adds a dependency on the holiman/uint256 package to provide proper handling of 256-bit unsigned integers. This is important for supporting the new transaction type's authorization fields.
35-35: Added reflection type for uint256.Int.This allows the package to properly handle type errors during unmarshaling operations for the new U256 type, consistent with how other types are handled in this file.
231-271: New U256 type implementation for 256-bit unsigned integer marshaling.The implementation follows the established pattern of other numeric types in this file, providing:
- JSON marshaling/unmarshaling with 0x prefix
- Text marshaling/unmarshaling
- String representation
The code correctly handles edge cases like empty strings and provides proper error handling. This new type is essential for serializing the SetCodeAuthorization fields in the new transaction type.
A few implementation details worth noting:
- Lines 248-253 handle the empty string case which uint256.Int would otherwise reject
- The implementation ensures strict hex format validation
- Appropriate error types are returned for JSON unmarshaling errors
params/version.go (1)
27-27:Details
❓ Verification inconclusive
Version patch increment for the new transaction type.
The patch version is incremented from 3 to 5, reflecting the addition of the EIP-7702 SetCode transaction type and its supporting structures.
Let's verify what was in prior versions:
🏁 Script executed:
#!/bin/bash # Check git history for the version number git log -p -- params/version.go | grep -A 2 "VersionPatch ="Length of output: 0
Manual verification required for VersionPatch bump
The automated
git log -pcheck didn’t return any history forVersionPatch. Please confirm that the patch version inparams/version.gowas intentionally incremented from 3 to 5 to reflect the EIP‑7702 SetCode transaction addition:• File & line:
params/version.go:27
• Suggested commands:
git log --oneline -n 20 -- params/version.gogit blame -L27,27 params/version.gocore/types/transaction.go (4)
54-54: Added SetCodeTxType constant.The new transaction type constant (0x04) is defined for the EIP-7702 SetCode transaction, continuing the sequential numbering pattern of existing transaction types.
208-209: Added support for decoding SetCodeTx transactions.The
decodeTypedmethod is extended to recognize and instantiate the new SetCodeTx type when decoding typed transactions. This is a necessary extension to support the new transaction type.
472-479: Added method to access SetCodeTx authorizations.The
SetCodeAuthorizationsmethod provides access to the authorization list from SetCodeTx transactions, returning nil for other transaction types. This accessor method follows the pattern of existing transaction data accessors.
483-502: Added method to extract unique authority addresses.The
SetCodeAuthoritiesmethod extracts and deduplicates authority addresses from the authorization list, which is useful for validation and processing of SetCode transactions.The implementation correctly:
- Handles non-SetCodeTx transactions by returning nil
- Skips invalid authorizations where
Authority()returns an error- Deduplicates addresses using a map
- Pre-allocates the result slice with a capacity matching the auth list
| if dec.V == nil { | ||
| return errors.New("missing required field 'yParity' for SetCodeAuthorization") | ||
| } | ||
| s.V = uint8(*dec.V) | ||
| if dec.R == nil { |
There was a problem hiding this comment.
Validate yParity range when parsing JSON
yParity is expected to be exactly 0 or 1 (EIP‑2 / EIP‑155 rules).
If a larger value arrives over the wire we silently truncate it with uint8(*dec.V), which can mask malformed or malicious payloads.
s.V = uint8(*dec.V)
+ if s.V != 0 && s.V != 1 {
+ return errors.New("invalid yParity value: must be 0 or 1")
+ }| AuthorizationList []SetCodeAuthorization `json:"authorizationList,omitempty"` | ||
| V *hexutil.Big `json:"v"` | ||
| R *hexutil.Big `json:"r"` | ||
| S *hexutil.Big `json:"s"` | ||
| YParity *hexutil.Uint64 `json:"yParity,omitempty"` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider pointer‑vs‑slice semantics for authorizationList
AuthorizationList []SetCodeAuthorization is a value slice.
During (un)marshal we rely on nil vs. “empty slice” to distinguish “field omitted” from “present but empty”.
Because Go serialises an empty slice as [], clients can still send {"authorizationList":[]} – our later nil check (l.512‑514) will pass even though the list is empty.
If an empty list is invalid you should:
if dec.AuthorizationList == nil || len(dec.AuthorizationList) == 0 {
return errors.New("authorizationList must contain at least one entry")
}| case SetCodeTxType: | ||
| var itx SetCodeTx | ||
| inner = &itx | ||
| if dec.ChainID == nil { | ||
| return errors.New("missing required field 'chainId' in transaction") | ||
| } | ||
| var overflow bool | ||
| itx.ChainID, overflow = uint256.FromBig(dec.ChainID.ToInt()) | ||
| if overflow { | ||
| return errors.New("'chainId' value overflows uint256") | ||
| } | ||
| if dec.Nonce == nil { | ||
| return errors.New("missing required field 'nonce' in transaction") | ||
| } | ||
| itx.Nonce = uint64(*dec.Nonce) | ||
| if dec.To == nil { | ||
| return errors.New("missing required field 'to' in transaction") | ||
| } | ||
| itx.To = *dec.To | ||
| if dec.Gas == nil { | ||
| return errors.New("missing required field 'gas' for txdata") | ||
| } | ||
| itx.Gas = uint64(*dec.Gas) | ||
| if dec.MaxPriorityFeePerGas == nil { | ||
| return errors.New("missing required field 'maxPriorityFeePerGas' for txdata") | ||
| } | ||
| itx.GasTipCap = uint256.MustFromBig((*big.Int)(dec.MaxPriorityFeePerGas)) | ||
| if dec.MaxFeePerGas == nil { | ||
| return errors.New("missing required field 'maxFeePerGas' for txdata") | ||
| } | ||
| itx.GasFeeCap = uint256.MustFromBig((*big.Int)(dec.MaxFeePerGas)) | ||
| if dec.Value == nil { | ||
| return errors.New("missing required field 'value' in transaction") | ||
| } | ||
| itx.Value = uint256.MustFromBig((*big.Int)(dec.Value)) | ||
| if dec.Input == nil { | ||
| return errors.New("missing required field 'input' in transaction") | ||
| } | ||
| itx.Data = *dec.Input | ||
| if dec.AccessList != nil { | ||
| itx.AccessList = *dec.AccessList | ||
| } | ||
| if dec.AuthorizationList == nil { | ||
| return errors.New("missing required field 'authorizationList' in transaction") | ||
| } | ||
| itx.AuthList = dec.AuthorizationList | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sanity‑check each authorisation entry during decoding
We trust every element in authorizationList but never validate:
auth.ChainIDmatches the tx ChainIDauth.Addressis unique (no duplicates)auth.Authority()succeeds and signature is validauth.Noncemonotonicity / replay rules
Adding a tight loop here prevents malformed txs from reaching deeper consensus code.
for i, a := range dec.AuthorizationList {
if a.ChainID.Cmp(itx.ChainID) != 0 {
return fmt.Errorf("authorizationList[%d].chainId mismatch", i)
}
if _, err := a.Authority(); err != nil {
return fmt.Errorf("authorizationList[%d] invalid signature: %w", i, err)
}
// TODO: duplicate / nonce checks as per consensus rules
}| func (tx *SetCodeTx) rawSignatureValues() (v, r, s *big.Int) { | ||
| return tx.V.ToBig(), tx.R.ToBig(), tx.S.ToBig() | ||
| } |
There was a problem hiding this comment.
Guard against nil signature fields
rawSignatureValues() blindly dereferences tx.V/R/S.
A freshly‑constructed SetCodeTx (before signing) will panic.
-func (tx *SetCodeTx) rawSignatureValues() (v, r, s *big.Int) {
- return tx.V.ToBig(), tx.R.ToBig(), tx.S.ToBig()
+func (tx *SetCodeTx) rawSignatureValues() (v, r, s *big.Int) {
+ if tx.V == nil || tx.R == nil || tx.S == nil {
+ return nil, nil, nil
+ }
+ return tx.V.ToBig(), tx.R.ToBig(), tx.S.ToBig()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (tx *SetCodeTx) rawSignatureValues() (v, r, s *big.Int) { | |
| return tx.V.ToBig(), tx.R.ToBig(), tx.S.ToBig() | |
| } | |
| func (tx *SetCodeTx) rawSignatureValues() (v, r, s *big.Int) { | |
| if tx.V == nil || tx.R == nil || tx.S == nil { | |
| return nil, nil, nil | |
| } | |
| return tx.V.ToBig(), tx.R.ToBig(), tx.S.ToBig() | |
| } |
add setcode transaction type
2 participants
1. Purpose or design rationale of this PR
...
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
Bug Fixes
Tests
Chores