test: add eip-7702 tests #1865
test: add eip-7702 tests #1865
Conversation
WalkthroughAdds EIP‑7702 integration tests and Solidity contracts, updates Web3 v7 API usage and a legacy EIP‑712 encoder, bumps Go/Python dependencies and Nix packaging, and introduces Makefile lint targets for formatting Nix files. Changes
Sequence Diagram(s)%% High-level EIP-7702 flow (new tests)
sequenceDiagram
autonumber
participant Test as Test harness
participant W3 as Web3 client
participant Node as RPC node
participant A as Simple7702Account
participant Target as Target/Utils
Test->>W3: build authorization + tx (authorizationList)
W3->>Node: eth_sendRawTransaction(Web3.to_hex(signed.raw_transaction))
Node-->>W3: txHash
W3-->>Test: receipt (included)
rect rgba(220,240,220,0.18)
Note over Node,A: On inclusion, delegation code is set/executed
Node->>A: execute delegated call(s)
A->>Target: invoke target methods (e.g., getCodeHash, transfers)
Target-->>A: return results
A-->>Node: final execution outcome
end
Test->>W3: verify via eth_getCode / Utils.getCodeHash
W3->>Node: query
Node-->>W3: code / codeHash
W3-->>Test: assert expected state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (2)integration_tests/test_ibc_rly_gas.py (1)
integration_tests/test_ibc_rly.py (1)
⏰ 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). (16)
🔇 Additional comments (2)
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. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
![semgrep-code-crypto-org-chain[bot]](https://avatars.githubusercontent.com/u/78834054?s=60&v=4){kind=small}
…at/7702_tests # Conflicts: # go.mod # go.sum # gomod2nix.toml
XinyuCRO
requested review from
JayT106 and
calvinaco
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
integration_tests/contracts/contracts/TestERC20A.sol (1)
7-9: Removepublicfrom constructor (invalid since Solidity 0.7.x).With
pragma solidity ^0.8.20;,constructor()cannot have a visibility specifier. This will fail to compile under the updated toolchain.Apply:
- constructor() public ERC20("Bitcoin MAX", "MAX") { + constructor() ERC20("Bitcoin MAX", "MAX") {integration_tests/eip712_utils.py (1)
361-447: Potential EIP-712 array encoding mismatch (legacy path).In
encode_structured_data_legacy, arrays are currently encoded viakeccak(encode(data_types, data_hashes)). Per EIP-712 and common implementations (MetaMasketh-sig-util), array values should be hashed as the keccak of the concatenation of each element’s encodeData (not ABI-encoding the element values directly). This difference can cause signature/hash mismatches for arrays of structs and some primitives.References: EIP-712 spec array rule and MetaMask TypedDataUtils. (eips.ethereum.org)
Apply this diff to fix array handling:- if field_type.endswith("]"): - field_type_of_inside_array = field_type[: field_type.rindex("["))] - field_type_value_pairs = [ - encode_field_old(types, name, field_type_of_inside_array, item) - for item in value - ] - if value: - data_types, data_hashes = zip(*field_type_value_pairs) - else: - data_types, data_hashes = [], [] - return ("bytes32", keccak(encode(data_types, data_hashes))) + if field_type.endswith("]"): + inner = field_type[: field_type.rindex("[")] + # Per spec, arrays are hashed as keccak(concat(encodeData(inner, item_i)...)) + concatenated = b"".join(encode_data_old(inner, types, item) for item in value) + return ("bytes32", keccak(concatenated))Optional: add targeted tests for arrays of primitives and structs to lock this down. I can draft them if helpful.
🧹 Nitpick comments (15)
integration_tests/test_filters.py (1)
38-38: Use int forfrom_blockto avoid type quirks.
from_blockin Web3 v7 accepts an int and will handle encoding. Passing a hex string works today but is brittle. Suggest:- current_height = hex(w3.eth.get_block_number()) + current_height = w3.eth.get_block_number() event_filter = mycontract.events.ChangeGreeting.create_filter( - from_block=current_height + from_block=current_height )integration_tests/test_pruned_node.py (1)
48-49: Avoid hardcoding exact gas; derive from the receipt/tx to reduce flakiness across compiler/EVM changes.Gas for ERC20.transfer can shift slightly with compiler/dep bumps. Hardcoding
51531risks brittle tests when contracts/tooling change.Minimal, robust tweak:
- txhash = w3.eth.send_raw_transaction(signed.raw_transaction) - exp_gas_used = 51531 + txhash = w3.eth.send_raw_transaction(signed.raw_transaction) + # derive expected gas dynamically later from the receipt/txThen after obtaining
txreceipt:# Replace the hardcoded assertion: # assert txreceipt.gasUsed == exp_gas_used # With: tx = w3.eth.get_transaction(txhash) assert tx.gas == txreceipt.gasUsed # gas limit equals used in this flowIf you prefer to keep a constant, please confirm this value on CI across all nix profiles before pinning it.
integration_tests/shell.nix (1)
17-17: Add Poetry; consider enabling in-project virtualenvs for reproducible dev shells.Optional, but it helps avoid cross-project venv reuse:
shellHook = '' mkdir -p ./coverage export GOCOVERDIR=./coverage export TMPDIR=/tmp + export POETRY_VIRTUALENVS_IN_PROJECT=true + export POETRY_VIRTUALENVS_CREATE=true '';integration_tests/contracts/contracts/Counter.sol (1)
7-13: Underflow revert on decrease may be intentional; confirm test intent
decrease()will revert at zero under Solidity 0.8+ checked arithmetic. If that’s not the intent, guard it.- function decrease() public { - count--; - } + function decrease() public { + if (count > 0) { + unchecked { count--; } + } + }Makefile (1)
151-155: Make nixfmt invocation safer with null-delimited args and -rAvoid issues with paths containing spaces and no-file cases.
-lint-nix-fix: - find . -name "*.nix" ! -path './integration_tests/contracts/*' ! -path "./contracts/*" | xargs nixfmt +lint-nix-fix: + find . -name "*.nix" ! -path './integration_tests/contracts/*' ! -path "./contracts/*" -print0 | xargs -0 -r nixfmtintegration_tests/utils.py (1)
670-677: Concurrency helper: minor footgun on thread count.
ThreadPoolExecutor(len(raw_transactions))can explode threads if the list ever gets large. Consider capping workers (e.g., min(len(raw_transactions), 16)) to avoid resource spikes in stress tests.- with ThreadPoolExecutor(len(raw_transactions)) as exec: + max_workers = min(len(raw_transactions), 16) + with ThreadPoolExecutor(max_workers) as exec:nix/testenv.nix (1)
35-53: postPatch license edits: consider robustness.Direct
sedlines assume exact matches; upstream pyproject changes may break this silently. Consider guarding with grep checks or usingtomlqif available. Otherwise, acceptable as a pragmatic fix for Poetry license schema.- sed -i '/^license-files = \["LICENSE"\]$/d' pyproject.toml + if grep -q '^license-files = \["LICENSE"\]$' pyproject.toml; then + sed -i '/^license-files = \["LICENSE"\]$/d' pyproject.toml + fiintegration_tests/eip712_utils.py (1)
363-367: Minor: duplicate keccak imports and long exception strings.
- You import
keccakat top and again inside the function; prefer one import to avoid confusion.- Ruff TRY003 flags the long messages in raised exceptions; consider shortening or using exception types without verbose strings.
-from eth_hash.auto import keccak +from eth_hash.auto import keccak ... - from eth_utils import keccak + # reuse module-level keccakintegration_tests/test_eip712.py (1)
74-78: Harden error reporting on non-JSON error bodies.
.json()may raiseValueErroron non-JSON responses; fall back to.text.Apply this diff:
- if not response.ok: - raise Exception( - f"response code: {response.status_code}, " - f"{response.reason}, {response.json()}" - ) + if not response.ok: + try: + err_body = response.json() + except ValueError: + err_body = response.text + raise Exception( + f"response code: {response.status_code}, {response.reason}, {err_body}" + )integration_tests/contracts/contracts/Simple7702Account.sol (2)
19-21: Hard-coded EntryPoint address — confirm network compatibility or make configurable.If tests may run against different networks or EntryPoint deployments, hard-coding can break them. Consider a constructor
immutableor a file-level constant alias for clarity.Example minimal refactor:
- function entryPoint() public pure override returns (IEntryPoint) { - return IEntryPoint(0x4337084D9E255Ff0702461CF8895CE9E3b5Ff108); - } + address constant ENTRY_POINT_ADDR = 0x4337084D9E255Ff0702461CF8895CE9E3b5Ff108; + function entryPoint() public pure override returns (IEntryPoint) { + return IEntryPoint(ENTRY_POINT_ADDR); + }
35-37: Mark ERC-1271 implementation as an override.Be explicit to avoid ambiguity with future base contracts.
- function isValidSignature(bytes32 hash, bytes memory signature) public view returns (bytes4 magicValue) { + function isValidSignature(bytes32 hash, bytes memory signature) public view override(IERC1271) returns (bytes4 magicValue) {integration_tests/test_eip7702.py (4)
13-18: Add minimal input validation to delegation code helper.Defend against malformed addresses to make failures explicit.
-def address_to_delegation(address: str): - return DELEGATION_PREFIX + address[2:] +def address_to_delegation(address: str): + if not (isinstance(address, str) and address.startswith("0x") and len(address) == 42): + raise ValueError(f"invalid address: {address}") + return DELEGATION_PREFIX + address[2:]
76-78: Avoid cross-test coupling: use unique accounts per test.Both tests derive
n=2, which couples nonces/state and can flake with reordering/parallel runs.Apply this diff:
- acc = derive_new_account(n=2) + acc = derive_new_account(n=2002)- account = derive_new_account(n=2) + account = derive_new_account(n=2003)Also applies to: 96-98
110-131: Optional: use estimate_gas for batch to reduce spurious OOG.Keeps tests resilient to opcode repricings.
- ).build_transaction( - { - "from": account.address, - "nonce": w3.eth.get_transaction_count(account.address), - "gas": 1000000, - "gasPrice": w3.eth.gas_price, - } - ) + ) + tx = tx.build_transaction( + { + "from": account.address, + "nonce": w3.eth.get_transaction_count(account.address), + "gas": w3.eth.estimate_gas({"from": account.address, "to": account.address, "data": tx.data}) * 2, + "gasPrice": w3.eth.gas_price, + } + )
176-179: Minor: avoid shadowing built-in name.Rename
exectopoolfor clarity.- with ThreadPoolExecutor(len(providers)) as exec: - tasks = [exec.submit(process, w3, provider) for w3, provider in providers] + with ThreadPoolExecutor(len(providers)) as pool: + tasks = [pool.submit(process, w3, provider) for w3, provider in providers]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
go.sumis excluded by!**/*.sumintegration_tests/contracts/package-lock.jsonis excluded by!**/package-lock.jsonintegration_tests/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
Makefile(1 hunks)go.mod(4 hunks)gomod2nix.toml(4 hunks)integration_tests/contracts/contracts/Counter.sol(1 hunks)integration_tests/contracts/contracts/Gravity.sol(3 hunks)integration_tests/contracts/contracts/Simple7702Account.sol(1 hunks)integration_tests/contracts/contracts/TestERC20A.sol(1 hunks)integration_tests/contracts/contracts/Utils.sol(1 hunks)integration_tests/contracts/hardhat.config.ts(1 hunks)integration_tests/contracts/package.json(1 hunks)integration_tests/eip712_utils.py(2 hunks)integration_tests/network.py(3 hunks)integration_tests/pyproject.toml(1 hunks)integration_tests/shell.nix(1 hunks)integration_tests/test_basic.py(13 hunks)integration_tests/test_eip712.py(2 hunks)integration_tests/test_eip7702.py(1 hunks)integration_tests/test_filters.py(1 hunks)integration_tests/test_gov_update_params.py(2 hunks)integration_tests/test_mempool.py(2 hunks)integration_tests/test_pruned_node.py(1 hunks)integration_tests/test_replay_block.py(1 hunks)integration_tests/test_subscribe.py(1 hunks)integration_tests/test_upgrade.py(2 hunks)integration_tests/utils.py(6 hunks)nix/testenv.nix(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
integration_tests/test_pruned_node.py (1)
integration_tests/network.py (1)
w3(39-42)
integration_tests/test_subscribe.py (1)
integration_tests/utils.py (1)
send_raw_transactions(661-667)
integration_tests/test_eip7702.py (1)
integration_tests/utils.py (4)
deploy_contract(419-439)derive_new_account(483-487)fund_acc(858-863)get_contract(442-447)
integration_tests/utils.py (2)
integration_tests/network.py (1)
w3(39-42)integration_tests/cosmoscli.py (1)
build_evm_tx(1371-1383)
integration_tests/test_basic.py (3)
integration_tests/cosmoscli.py (1)
module_address(37-44)integration_tests/utils.py (2)
module_address(744-746)remove_cancun_prague_params(866-888)integration_tests/network.py (3)
Geth(94-96)setup_custom_cronos(155-197)w3(39-42)
integration_tests/test_eip712.py (1)
integration_tests/eip712_utils.py (1)
encode_structured_data_legacy(362-446)
🪛 Ruff (0.12.2)
integration_tests/eip712_utils.py
367-367: Avoid specifying long messages outside the exception class
(TRY003)
374-377: Avoid specifying long messages outside the exception class
(TRY003)
382-385: Avoid specifying long messages outside the exception class
(TRY003)
403-403: Avoid specifying long messages outside the exception class
(TRY003)
408-412: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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: build (ubuntu-latest)
🔇 Additional comments (52)
go.mod (4)
309-316: Replace Ethermint to newer pseudo-version — confirm gomod2nix and EIP-7702 changes
- go.mod shows: github.com/evmos/ethermint => github.com/crypto-org-chain/ethermint v0.22.1-0.20250911151407-4406f2707ea5 (go.mod:309).
- No matching [mod."github.com/evmos/ethermint"] version entry found in gomod2nix.toml; confirm gomod2nix.toml in this PR is updated to the same pseudo-version and that the commit includes the required EIP‑7702 changes for tests.
37-40: spf13 bumps present—smoke CLI behavior and sync gomod2nix.go.mod shows github.com/spf13/cast v1.10.0, cobra v1.10.1, pflag v1.0.10, viper v1.21.0 (lines 37–40). gomod2nix.toml has no matching spf13 entries; search found no uses of TraverseChildren / SetArgs / SetInheritFlags / AutomaticEnv / BindPFlag / BindEnv.
- Run quick smoke tests for common CLI flows (top-level + nested subcommands) and verify flag vs env precedence, AutomaticEnv behavior, default values, and BindPFlag/BindEnv bindings.
- If you rely on gomod2nix, update/sync its spf13 module versions to match go.mod.
261-263: YAML module duplication — standardize to gopkg.in/yamlRepo-wide import search found no direct imports of go.yaml.in/* or gopkg.in/yaml.v*; the go.yaml.in entries in go.mod look indirect. If nothing in the dependency graph requires go.yaml.in, remove those lines and standardize on gopkg.in/yaml.v3; verify with
go mod why/go mod graphand rungo mod tidy.- go.yaml.in/yaml/v2 v2.4.2 // indirect - go.yaml.in/yaml/v3 v3.0.4 // indirect
44-45: gRPC/protobuf patch bumps — approve (no API breaks expected).Approved as-is. Verified go.mod lists google.golang.org/grpc v1.75.1 and google.golang.org/protobuf v1.36.9; repo contains generated protos (protoc-gen-gogo / grpc-gateway) and plugin references: protoc-gen-validate v1.2.1 (go.mod) and google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0 (go.sum). If you regenerate stubs, ensure the Makefile/scripts (Makefile:325 -> scripts/protoc-swagger-gen.sh) use matching plugin versions.
gomod2nix.toml (4)
616-618: locafero bump synced; OK.Minor update; used under Viper. No action.
625-642: spf13 family versions match go.mod; good.Hashes refreshed appropriately. Approved.
792-796: gRPC/protobuf patch levels synced with go.mod; LGTM.No further action.
318-320: Approve — Ethermint version & hash verified. gomod2nix.toml lists github.com/evmos/ethermint v0.22.1-0.20250911151407-4406f2707ea5 with sha256-7neEhxPIY+OJTqoHOMmdjUPiwx+v2KanLuvTPDKk5s8=; go.mod replace and go.sum entries match. Re-run gomod2nix after a rebase to avoid hash skew.integration_tests/contracts/contracts/TestERC20A.sol (1)
1-1: Pragma bump to ^0.8.20 aligns with OZ v5 and hardhat compiler config.No issues with the pragma itself.
integration_tests/pyproject.toml (1)
16-16: Verify lock resolves with Poetry and confirm aiohttp/setuptools pins don't mask transitive conflictsintegration_tests/pyproject.toml pins: web3 ^7.13.0, eth-account ^0.13, protobuf 4.25.8, aiohttp 3.9.4, setuptools 78.1.1.
Sandbox lacks poetry (cannot run resolver here). Run the resolver sanity check in your environment/CI (poetry --version; poetry lock --no-update) and verify the locked versions for web3, eth-account, aiohttp and transitive deps yarl, multidict, aiosignal; report any conflicts.
integration_tests/test_mempool.py (1)
42-42: Correct move tosigned.raw_transactionfor Web3 v7.Matches the v7 naming and keeps tests consistent with the suite-wide refactor.
Also applies to: 118-118
integration_tests/test_replay_block.py (1)
66-66: LGTM: switch tosigned.raw_transaction.Consistent with the broader migration; no other logic changes.
integration_tests/contracts/package.json (2)
16-18: Checkoverridessupport in your package manager/Node version.
overridesis honored by npm (not yarn classic). Since the scripts usenpx, you’re likely on npm—just ensure the Node/npm in nix supports it so axios actually gets overridden.
9-15: Align TypeChain plugin/core versions in integration_tests/contracts/package.jsonintegration_tests/contracts/package.json lists hardhat-typechain ^0.3.5, typechain ^4.0.3, @typechain/ethers-v5 ^5.0.0 and ts-generator — mixing different majors is likely incompatible and can break npx hardhat typechain.
Verification: running npx hardhat typechain here failed with "No Hardhat config file found" so I could not fully reproduce the runtime failure.
Two safe options:
- Modern: replace hardhat-typechain with @typechain/hardhat and upgrade typechain + @typechain/ethers-v5 to the same major; remove ts-generator; update Hardhat config to use the new plugin.
- Legacy: keep hardhat-typechain but pin typechain and @typechain/ethers-v5 to compatible majors (ensure their majors match).
After changes, run npx hardhat typechain with a valid Hardhat config to verify.
integration_tests/test_upgrade.py (1)
14-14: Adopt Web3 v7 exception class — LGTM (verified)
integration_tests/pyproject.toml pins web3 = "^7.13.0" (satisfies web3 >= 7).integration_tests/contracts/hardhat.config.ts (1)
6-14: solc 0.8.28 supports evmVersion "prague" (experimental) — no fallback requiredVerified in the Solidity docs and v0.8.27 release notes; you can keep evmVersion: "prague". Optional: add a brief inline comment that it's experimental if you want to signal compatibility risk for older toolchains.
integration_tests/contracts/contracts/Utils.sol (1)
1-11: Minimal helper looks correct
extcodehashusage is fine for test utilities.integration_tests/test_gov_update_params.py (3)
2-2: Web3 v7 exceptions import — LGTMUsing
from web3 import exceptionsaligns with the suite-wide change.
42-47: Switch to Web3RPCError — LGTMExpectation updates for
randomTokenId()anddeploy_contract()match Web3 v7 RPC error surfacing.
51-58: Incorrect — split string is valid Python; no syntax error found.
python -m py_compile integration_tests/test_gov_update_params.py returned no errors; adjacent string literals are implicitly concatenated so no critical fix is required. Optional: join the fragments or wrap them in parentheses for readability.Likely an incorrect or invalid review comment.
integration_tests/network.py (3)
10-10: POA middleware migration — LGTMUsing
ExtraDataToPOAMiddlewareis the correct replacement forgeth_poa_middlewarein Web3 v7.
132-132: Injecting POA middleware for geth — LGTMCorrect layer injection for the geth test node.
46-48: LegacyWebSocketProvider — verify Web3 pin; sandbox couldn't verify
- Sandbox check failed: ModuleNotFoundError: 'web3'.
- Ensure integration_tests/network.py (lines 46–48) runs against a Web3 release that exposes web3.providers.LegacyWebSocketProvider (pin Web3 v7.x) or replace with a supported provider.
- Verify locally with:
python - <<'PY' import web3 print("web3", web3.__version__) print("LegacyWebSocketProvider present:", hasattr(web3.providers, "LegacyWebSocketProvider")) PY
- Optional: prefer HTTP by default and enable WS only for tests that require subscriptions to reduce CI WS flakiness.
integration_tests/contracts/contracts/Gravity.sol (3)
8-9: OZ v5 import paths — LGTM
PausableandReentrancyGuardlocations match OpenZeppelin v5.
790-791: Initialize Ownable with relayerAdmin — good ownership semanticsPassing
relayerAdmintoOwnableensures the expected owner from deployment.
828-828: Approved — _grantRole used; OpenZeppelin Contracts v5 confirmed (^5.1.0).
ACL bootstrap for RELAYER_ADMIN is correct; no changes required.integration_tests/utils.py (7)
26-31: Imports updated for Web3 v7: looks good.Switching to
eth_utils.abi.abi_to_signatureand importingWeb3plusfind_matching_event_abialigns with the newer Web3/eth-utils APIs.
68-71: New test contracts registered.The TEST_CONTRACTS additions are consistent with the new 7702-related tests and utilities.
616-619: Hex-encoding raw tx for Cosmos CLI is correct.
Web3.to_hex(signed.raw_transaction)produces the expected0x-prefixed payload forbuild_evm_tx. LGTM.
688-713: Consistent raw tx usage.Appending
signed.raw_transactionmirrors the rest of the suite and avoids mixed attribute styles. Good.
784-797: Event signature derivation modernized.
abi_to_signature+event_signature_to_log_topicis the right pairing. Good improvement.
802-804: debug_traceTransaction: explicit hex is correct.Passing
Web3.to_hex(h)avoids ambiguity vs HexBytes.
466-467: raw_transaction usage OK — integration_tests pins Web3 v7.13+integration_tests/pyproject.toml pins web3 = ^7.13.0 and eth-account = ^0.13; signed.raw_transaction in integration_tests/utils.py (lines 466–467, 472–474) is consistent with those pinned deps.
integration_tests/test_subscribe.py (2)
140-141: raw_transaction migration is consistent.Keeps attribute naming uniform across tests/utilities.
145-146: Compare topics as hex strings (correct for subscriptions).
eth_subscriptionresults surface topics as hex strings;Web3.to_hex(TEST_EVENT_TOPIC)is the right check.integration_tests/test_basic.py (7)
13-17: Imports align with Web3 v7.Using
from web3 import Web3, exceptionsand importingsetup_custom_cronosis appropriate.
123-123: Gas used expectation changed — confirm determinism.
exp_gas_used=564532can be sensitive to compiler/settings changes (e.g., Solidity 0.8.28, optimizer). If flaky across environments, consider asserting within a tolerance/bounds instead of a fixed value.
162-173: Exception expectations updated for Web3 v7 — good, and matches docs.
- JSON-RPC errors now raise
exceptions.Web3RPCError.- Web3-internal
ValueErrorbecameexceptions.Web3ValueError.These updates follow the v7 migration guidance. LGTM.
References: web3.py v7 migration on exception changes and
Web3RPCErrorintroduction. (web3py.readthedocs.io)Also applies to: 436-447, 450-461, 463-475, 477-487, 1122-1127
574-575: Normalize to hex for trace input — good.
Web3.to_hex(receipts[i]["transactionHash"])avoids type mismatches with clients.
677-683: Pre-Cancun fixture and params removal flow look sound.Module-scoped
custom_cronosplusremove_cancun_prague_params(custom_cronos)is a clean way to validate pre-6780 semantics.Also applies to: 691-693
802-804: debug_traceTransaction call formatting is consistent.Explicit hex encoding for each tx hash is correct.
Also applies to: 868-870
1056-1057: raw_transaction usage matches the rest of the suite.
w3.eth.send_raw_transaction(signed.raw_transaction)is correct post-migration.nix/testenv.nix (2)
14-17: Add poetry-core to cprotobuf build systems.Reasonable if upstream switched to PEP 517/Poetry metadata. Ensure this does not mask other build backends in transitive deps.
28-33: Map override helper is tidy.
lib.mapAttrsto append build tools is a neat consolidation.integration_tests/test_eip712.py (2)
8-13: Switch to legacy EIP-712 encoder: please verify digest parity with previous flow.Confirm that
encode_structured_data_legacyyields the same signable payload (EIP-191 0x01 || domain_hash || message_hash) the tests relied on previously, across your supportedeth-accountversions. Also ensure downstream consumers ofsignature_to_web3_extensionremain unaffected.Would you like me to add a small test vector to assert the typed data digest against a known-good hash?
53-55: LGTM: using SignableMessage with Account.sign_message.
encode_structured_data_legacy(...)returns aSignableMessage, soAccount.sign_message(...)remains correct here.integration_tests/contracts/contracts/Simple7702Account.sol (3)
39-41: Signature model is appropriate for 7702 “EOA-at-same-address”.Recovering to
address(this)matches the delegated-EOA model EIP-7702 enables.
43-49: Execute gating looks correct.Restricting to
selforEntryPointaligns with ERC-4337 expectations while allowing direct calls when deployed via 7702 at the EOA address.
51-58: Interface support set is sensible.Covers IERC165/IAccount/IERC1271 and token receivers as advertised.
integration_tests/test_eip7702.py (3)
20-42: Authorization nonce semantics: double-check against EIP-7702 and provider versions.Using
auth["nonce"] = tx_nonce + 1is consistent with drafts, but some client builds deviated during development. Please confirm this holds for your Cronos/Geth versions under CI.Would you like me to add a guard that retries with
nonceornonce+1based on a feature probe?
61-65: Confirm expected nonce bump (+2) across clients.Some implementations only bump once on failure paths. Your later test expects status 0 yet code+nonce updates; verify parity on both nodes to avoid flakes.
180-181: Order-agnostic check is fine.Comparing both statuses to 0 regardless of completion order is appropriate here.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go.mod (1)
3-3: Invalidgodirective: use major.minor (optionally addtoolchainfor patch).
go 1.23.12will be rejected. Usego 1.23and (optionally) atoolchainline for the exact patch.-go 1.23.12 +go 1.23 +toolchain go1.23.12
🧹 Nitpick comments (3)
integration_tests/test_eip7702.py (3)
18-65: Verify the authorization nonce calculation and consider extracting magic numbers.The function implements EIP-7702 transaction creation correctly, but there are a few areas that need verification:
- Authorization nonce logic: Line 25 uses
nonce + 1for the authorization. This assumes the authorization should use the account's next nonce after the transaction itself.- Magic numbers: The gas values (50000, 10000000000000, 10000) are hardcoded without explanation.
Please verify the authorization nonce calculation is correct according to EIP-7702 specification:Based on the search results, the authorization nonce calculation in EIP-7702 follows a specific pattern: when the same account both sends the transaction and authorizes the delegation, you must use the account's current nonce plus one (current_nonce + 1) in the signed authorization message. The current implementation appears to be correct for this scenario.
Consider extracting the magic numbers to named constants:
+# Gas configuration constants +DEFAULT_GAS_LIMIT = 50000 +MAX_FEE_PER_GAS = 10000000000000 +MAX_PRIORITY_FEE_PER_GAS = 10000 + def send_eip7702_transaction( w3, account, target_address, verify=True, provider="cronos" ): nonce = w3.eth.get_transaction_count(account.address) auth = { "chainId": w3.eth.chain_id, "address": target_address, "nonce": nonce + 1, } signed_auth = account.sign_authorization(auth) signed_tx = account.sign_transaction( { "chainId": w3.eth.chain_id, "type": 4, "to": account.address, - "gas": 50000, - "maxFeePerGas": 10000000000000, - "maxPriorityFeePerGas": 10000, + "gas": DEFAULT_GAS_LIMIT, + "maxFeePerGas": MAX_FEE_PER_GAS, + "maxPriorityFeePerGas": MAX_PRIORITY_FEE_PER_GAS, "nonce": nonce, "authorizationList": [signed_auth], "data": b"", } )
81-152: LGTM with minor suggestions!This comprehensive test effectively demonstrates EIP-7702's batch transaction capabilities by:
- Deploying a Simple7702Account contract
- Delegating account control to it
- Executing batch transfers
- Revoking delegation and verifying code hash
The test flow follows the example from eip7702.io and validates all expected behaviors. The code hash verification using the Utils contract is a good practice for ensuring proper delegation revocation.
Consider extracting the empty account code hash to a named constant:
+# Expected code hash for empty accounts (Keccak256 of empty byte array) +EMPTY_ACCOUNT_CODE_HASH = "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470" +Then use it in the assertion:
- assert code_hash == HexBytes( - "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470" - ) + assert code_hash == HexBytes(EMPTY_ACCOUNT_CODE_HASH)
153-153: Missing test for delegation to contract without fallback.The AI summary mentions a third test
test_eip7702_delegate_contract_without_fallback(cronos, geth)that should test delegation to a contract without fallback, but it's not present in the provided code. This test would be valuable for validating error handling scenarios.Would you like me to help implement the missing test function? This would complete the test coverage by validating that transactions fail appropriately when delegating to contracts without fallback functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.mod(4 hunks)gomod2nix.toml(4 hunks)integration_tests/test_eip7702.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
integration_tests/test_eip7702.py (1)
integration_tests/utils.py (4)
deploy_contract(419-439)derive_new_account(483-487)fund_acc(858-863)get_contract(442-447)
🔇 Additional comments (10)
integration_tests/test_eip7702.py (4)
1-9: LGTM!The imports are well-organized and include all necessary dependencies for EIP-7702 testing functionality.
11-11: LGTM!The delegation prefix
0xef0100correctly follows the EIP-7702 specification for delegated account code.
14-15: LGTM!The helper function correctly constructs the delegation code by combining the prefix with the target address (removing the '0x' prefix).
68-78: LGTM!The basic test correctly demonstrates EIP-7702 delegation functionality by creating a new account, funding it, and performing delegation to a target address. The use of a new account (n=2) is appropriate to avoid conflicts with genesis accounts that have different characteristics.
go.mod (2)
37-40: Dependency bumps LGTM.spf13 stack, grpc, and protobuf updates look consistent with the rest of the PR.
Also applies to: 44-45
309-309: Replace to new Ethermint fork version — lockfile alignedgo.mod (line 309) and gomod2nix.toml ([mod."github.com/evmos/ethermint"] lines 317–320) both pin github.com/crypto-org-chain/ethermint v0.22.1-0.20250918004400-f674c513792f.
gomod2nix.toml (4)
318-320: Ethermint lock updated — matches replace intent.Version/hash updated and
replacedpoints to the fork. Looks good.
615-617: Lockfile refresh for locafero, conc, and spf13 stack — OK.Hashes and versions align with go.mod updates.
Also applies to: 625-626, 628-642
746-748: Remove duplicate YAML family here; keep onlygopkg.in/yaml.*blocks.This mirrors a prior review: having both
go.yaml.in/yaml/v3andgopkg.in/yaml.v3(plus v2) is redundant. Delete thego.yaml.in/yaml/*mod blocks to avoid resolver drift.- [mod."go.yaml.in/yaml/v3"] - version = "v3.0.4" - hash = "sha256-NkGFiDPoCxbr3LFsI6OCygjjkY0rdmg5ggvVVwpyDQ4="Also remove the nearby
go.yaml.in/yaml/v2block (lines 743–745) to rely solely ongopkg.in/yaml.v2/v3.
792-796: gRPC and Protobuf bumps — LGTM.Consistent with go.mod; hashes refreshed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
integration_tests/contracts/contracts/Simple7702Counter.sol (1)
11-13: Optional: Guarddecrease()against zero underflowCalling
decrease()whencountis zero currently triggers aPanic(0x11)due to the implicit underflow check in Solidity 0.8+. If tests (or future callers) ever hit that path you’ll see an opaque revert instead of a controlled error. Consider adding an explicitrequire(count > 0, "...")(or equivalent) before the decrement for a clearer failure mode.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
integration_tests/contracts/contracts/Counter.sol(1 hunks)integration_tests/contracts/contracts/Simple7702Counter.sol(1 hunks)integration_tests/test_eip7702.py(1 hunks)integration_tests/utils.py(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- integration_tests/contracts/contracts/Counter.sol
- integration_tests/test_eip7702.py
- integration_tests/utils.py
⏰ 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). (19)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: build (macos-14)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: unittest
- GitHub Check: gomod2nix
- GitHub Check: Run golangci-lint
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
integration_tests/contracts/contracts/Simple7702Counter.sol (2)
11-13: Document the underflow behavior.The
decrease()function will revert ifcountis zero due to Solidity 0.8's built-in underflow protection. Consider adding a comment to clarify this behavior for future maintainers.function decrease() public { + // Note: Will revert on underflow if count is 0 count--; }
7-13: Consider emitting events for state changes.The
increase()anddecrease()functions modify state but don't emit events, making off-chain tracking and indexing more difficult. For a test contract this is acceptable, but consider adding events if this pattern is used in production code.integration_tests/test_eip7702.py (2)
70-80: Document the hardcoded target address.Line 73 uses a hardcoded target address
0x4Cd241E8d1510e30b2076397afc7508Ae59C66c9without explanation. Consider adding a comment explaining what this address represents (e.g., is it an arbitrary address for testing, a specific contract, etc.).+# Target address for basic delegation test (arbitrary address) target_address = "0x4Cd241E8d1510e30b2076397afc7508Ae59C66c9"
143-154: Document the empty account code hash.Lines 152-154 verify against a hardcoded hash value
0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470. This is the Keccak-256 hash of an empty byte array, representing an empty account. Consider adding a comment to explain this for future maintainers.+# Expected code hash for an empty account (keccak256 of empty bytes) code_hash = utils.functions.getCodeHash(account.address).call() assert code_hash == HexBytes( "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
integration_tests/contracts/contracts/Counter.sol(1 hunks)integration_tests/contracts/contracts/Simple7702Counter.sol(1 hunks)integration_tests/test_eip7702.py(1 hunks)integration_tests/utils.py(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- integration_tests/contracts/contracts/Counter.sol
🧰 Additional context used
🧬 Code graph analysis (2)
integration_tests/test_eip7702.py (1)
integration_tests/utils.py (5)
deploy_contract(420-440)derive_new_account(484-488)fund_acc(859-864)get_contract(443-448)sign_transaction(451-457)
integration_tests/utils.py (2)
integration_tests/network.py (1)
w3(39-42)integration_tests/cosmoscli.py (1)
build_evm_tx(1371-1383)
⏰ 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). (20)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: unittest
- GitHub Check: integration_tests (unmarked)
- GitHub Check: build (macos-latest)
- GitHub Check: build (macos-14)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: gomod2nix
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (ibc)
- GitHub Check: Run golangci-lint
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (9)
integration_tests/contracts/contracts/Simple7702Counter.sol (1)
15-16: LGTM: Empty payable handlers are intentional for EIP-7702.The empty
receive()andfallback()functions are appropriate for EIP-7702 delegation scenarios where the contract needs to accept ether without executing logic.integration_tests/utils.py (3)
26-26: LGTM: Import updates align with web3.py v7 migration.The new imports (
abi_to_signature,Web3, and updated path forfind_matching_event_abi) are consistent with web3.py v7 API changes.Also applies to: 29-30
68-71: LGTM: New test contracts added for EIP-7702 testing.The four new contract entries (
Simple7702Account,Simple7702Counter,Utils,Counter) align with the PR's objective to add EIP-7702 integration tests.
467-467: LGTM: Correct migration to web3.py v7 snake_case API.All occurrences of
signed.rawTransactionhave been correctly migrated tosigned.raw_transaction, consistent with web3.py v7's breaking API changes. The usage ofWeb3.to_hex(signed.raw_transaction)at lines 617-619 is also appropriate.Based on learnings.
Also applies to: 473-473, 617-619, 677-677, 709-709
integration_tests/test_eip7702.py (5)
13-17: LGTM: Correct EIP-7702 delegation prefix.The
DELEGATION_PREFIXvalue0xef0100matches the EIP-7702 specification for delegation designator code, and theaddress_to_delegationhelper correctly constructs the delegation code by appending the address.
46-67: LGTM: Thorough verification of delegation state.The verification logic correctly checks both the delegated code and nonce increment, with proper handling of the zero-address revocation case.
83-141: LGTM: Comprehensive batch execution and balance verification.The test correctly validates the batch execution functionality of the delegated account contract, including proper balance transfers and state verification.
157-214: LGTM: Robust concurrent test for storage persistence.The test effectively validates that storage is preserved across delegation resets using concurrent execution on both Cronos and Geth, ensuring consistent behavior across implementations.
20-29: Authorization nonce calculation is correct. The authorization tuple nonce must becurrent_nonce + 1for non-sponsored EIP-7702 transactions, as the authorization list is processed after the sender’s nonce is incremented. (eips.ethereum.org)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
integration_tests/test_eip7702.py (3)
35-37: Consider parameterizing gas values for flexibility.The function uses hardcoded gas parameters:
gas: 50000 (may be insufficient for complex delegations)maxFeePerGas: 10000000000000 (10,000 Gwei - extremely high)maxPriorityFeePerGas: 10000 (0.00001 Gwei - very low)While these fixed values work for basic tests, consider adding optional parameters to allow callers to override them for different test scenarios.
Apply this diff to add optional gas parameters:
def send_eip7702_transaction( - w3, account, target_address, verify=True, provider="cronos" + w3, account, target_address, verify=True, provider="cronos", + gas=50000, max_fee_per_gas=10000000000000, max_priority_fee_per_gas=10000 ): nonce = w3.eth.get_transaction_count(account.address) auth = { "chainId": w3.eth.chain_id, "address": target_address, "nonce": nonce + 1, } signed_auth = account.sign_authorization(auth) signed_tx = account.sign_transaction( { "chainId": w3.eth.chain_id, "type": 4, "to": account.address, - "gas": 50000, + "gas": gas, - "maxFeePerGas": 10000000000000, + "maxFeePerGas": max_fee_per_gas, - "maxPriorityFeePerGas": 10000, + "maxPriorityFeePerGas": max_priority_fee_per_gas, "nonce": nonce, "authorizationList": [signed_auth], "data": b"", } )
43-67: Consider adding transaction status check.The function waits for the receipt but doesn't verify
receipt.status == 1. While the tests may check this later, thesend_eip7702_transactionhelper should ensure the transaction succeeded before performing verification assertions, otherwise the verification errors could be misleading.Apply this diff to add a status check:
tx_hash = w3.eth.send_raw_transaction(signed_tx.raw_transaction) receipt = w3.eth.wait_for_transaction_receipt(tx_hash) + + assert receipt.status == 1, f"Transaction failed with status {receipt.status}, {provider}" if not verify: return receipt
210-214: Consider asserting successful completion of concurrent tasks.The concurrent execution collects results but only checks the count. Consider verifying that all tasks completed successfully without exceptions.
Apply this diff to check for exceptions:
providers = [cronos.w3, geth.w3] with ThreadPoolExecutor(len(providers)) as exec: tasks = [exec.submit(process, w3) for w3 in providers] - res = [future.result() for future in as_completed(tasks)] - assert len(res) == len(providers) + for future in as_completed(tasks): + # Re-raise any exception that occurred in the worker + future.result()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
integration_tests/test_eip7702.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
integration_tests/test_eip7702.py (1)
integration_tests/utils.py (4)
deploy_contract(420-440)derive_new_account(484-488)fund_acc(859-864)get_contract(443-448)
⏰ 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). (19)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: Run golangci-lint
- GitHub Check: build (macos-latest)
- GitHub Check: gomod2nix
- GitHub Check: build (macos-14)
- GitHub Check: unittest
- GitHub Check: build (ubuntu-latest)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (6)
integration_tests/test_eip7702.py (6)
1-13: LGTM!The imports are appropriate for EIP-7702 integration testing, and the
DELEGATION_PREFIXconstant correctly matches the EIP-7702 specification for delegation designator.
16-17: LGTM!The helper correctly constructs the EIP-7702 delegation code format by prepending the delegation prefix to the address.
70-80: LGTM!The basic delegation test is straightforward and correctly demonstrates the fundamental EIP-7702 functionality. The account derivation index (n=2) is now unique compared to the other tests.
83-154: LGTM!The test comprehensively validates EIP-7702 delegation with a batching account implementation. The account derivation index has been updated to n=3 (line 96) per previous review feedback, and the test correctly demonstrates delegation, execution, and revocation workflows.
157-209: LGTM!The test effectively validates that storage persists across delegation resets, which is a critical EIP-7702 property. The concurrent execution against both Cronos and Geth providers ensures consistent behavior across implementations.
1-215: AI summary mentions missing test function.The AI-generated summary references a test function
test_eip7702_delegate_contract_without_fallback(cronos, geth)that is not present in this file. Verify whether this test should be included or if the summary is outdated.
This reverts commit 6021c9f.
Signed-off-by: JayT106 <[email protected]>
* test: update py deps in integration tests * fmt: nix * fix: py deps * fix: lint nix * fix: py dep * fix: py dep * fix: tests * test: add batch tx test for 7702 * fix: audit * fix: tests * fix: gas * fix: deps * test: add revoke logic * fix: set hooks cause the states failed to commit * deps: * fix: tests * fix: audit * fix: deps * test: remove unsupported test * test: * fix comments * test * Revert "test" This reverts commit 6021c9f. --------- Signed-off-by: JayT106 <[email protected]> Co-authored-by: Thomas N. <[email protected]> Co-authored-by: Thomas Nguy <[email protected]> Co-authored-by: JayT106 <[email protected]>
Summary by CodeRabbit
New Features
Tests
Chores