refactor: use l1/eth types in non-L1 callers#3704
Merged
Merged
Conversation
brbrr
force-pushed
the
migrate/eth-types-on-non-l1-callers
branch
from
e0ffe04 to
3119134
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3704 +/- ##
========================================
Coverage 76.62% 76.63%
========================================
Files 409 410 +1
Lines 37030 37174 +144
========================================
+ Hits 28376 28490 +114
- Misses 6666 6696 +30
Partials 1988 1988 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Claude finished @brbrr's task in 3m 44s —— View job PR Review: use l1/eth types in non-L1 callers
This is a clean, well-scoped mechanical refactor swapping A few things I specifically checked and confirmed are sound:
FindingsNit
Nice touch adding the wire-parity tests alongside the migration — that's exactly the safety net this kind of change needs. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors non-L1 call sites to use the project’s internal l1/eth address/hash types instead of geth’s common.Address/common.Hash, while adding parity tests to lock the JSON-RPC wire format for L1/L2 messaging types.
Changes:
- Replace geth
common.Addresswithl1/eth.Addressacross RPC v8/v9/v10 message DTOs and core message types. - Refactor
L1HandlerTxnHashplumbing to key lookups by raw 32-byte msg-hash bytes (with explicit length validation). - Add “wire parity” tests for RPC v8/v9/v10 to pin JSON output/input shapes after the type migration.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| rpc/v9/wire_parity_test.go | Adds v9 JSON wire-shape parity tests for MsgToL1 / MsgFromL1. |
| rpc/v9/transaction.go | Switches MsgToL1.To from geth common.Address to eth.Address. |
| rpc/v9/l1.go | Updates L1HandlerTxnHash call sites to pass raw bytes via msgHash.Bytes(). |
| rpc/v9/l1_test.go | Updates mocks/expectations for new L1HandlerTxnHash([]byte) signature. |
| rpc/v9/estimate_fee.go | Switches MsgFromL1.From to eth.Address and uses From.Bytes() for calldata. |
| rpc/v8/wire_parity_test.go | Adds v8 JSON wire-shape parity tests for MsgToL1 / MsgFromL1. |
| rpc/v8/transaction.go | Switches MsgToL1.To from geth common.Address to eth.Address. |
| rpc/v8/l1.go | Updates L1HandlerTxnHash call sites to pass raw bytes via msgHash.Bytes(). |
| rpc/v8/l1_test.go | Updates mocks/expectations for new L1HandlerTxnHash([]byte) signature. |
| rpc/v8/estimate_fee.go | Switches MsgFromL1.From to eth.Address and uses From.Bytes() for calldata. |
| rpc/v10/wire_parity_test.go | Adds v10 JSON wire-shape parity tests for MsgToL1 / MsgFromL1. |
| rpc/v10/transaction_types.go | Switches MsgToL1.To from geth common.Address to eth.Address. |
| rpc/v10/l1.go | Updates L1HandlerTxnHash call sites to pass raw bytes via msgHash.Bytes(). |
| rpc/v10/l1_test.go | Updates mocks/expectations for new L1HandlerTxnHash([]byte) signature. |
| rpc/v10/estimate_fee.go | Switches MsgFromL1.From to eth.Address and uses From.Bytes() for calldata. |
| mocks/mock_blockchain.go | Regenerates blockchain.Reader mock for L1HandlerTxnHash([]byte). |
| migration/deprecated/migration_pkg_test.go | Switches test msg-hash type construction to eth.HexToHash and uses .Bytes(). |
| l1/l1_test.go | Updates NewEthSubscriber construction to pass eth.Address{}. |
| l1/eth_subscriber.go | Updates NewEthSubscriber signature to accept eth.Address and bridges to geth common.Address for the filterer. |
| core/transaction.go | Migrates core L1/L2 message address fields to eth.Address. |
| cmd/juno/juno.go | Parses core contract address into eth.Address for network config. |
| cmd/juno/juno_test.go | Updates tests to use eth.HexToAddress for core contract address defaults. |
| blockchain/networks/network.go | Changes Network.CoreContractAddress type to eth.Address and updates defaults. |
| blockchain/networks/network_test.go | Updates tests to assert eth.Address core contract addresses. |
| blockchain/blockchain.go | Changes Reader.L1HandlerTxnHash to accept []byte and validates 32-byte length before DB lookup. |
| blockchain/blockchain_test.go | Updates L1 handler msg-hash lookup test and adds negative tests for non-32-byte inputs. |
| adapters/vm2core/vm2core.go | Switches VM→core L1 address adaptation to eth.HexToAddress. |
| adapters/vm2core/vm2core_test.go | Updates expectations to use eth.Address conversions. |
| adapters/testutils/core_test_utils.go | Updates random L1 address generator to return eth.Address. |
| adapters/sn2core/sn2core.go | Switches Starknet→core L1 address adaptation to eth.HexToAddress. |
| adapters/p2p2core/felt.go | Switches P2P receipt Ethereum address adaptation to return eth.Address. |
Files not reviewed (1)
- mocks/mock_blockchain.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rodrodros
reviewed
Jun 5, 2026
brbrr
force-pushed
the
migrate/eth-types-on-non-l1-callers
branch
from
3119134 to
c718afc
Compare
Contributor
|
Claude finished @brbrr's task in 4m 35s —— View job PR Review: use l1/eth types in non-L1 callers
The migration itself is clean and well-scoped, but it does not compile against the current Blocker
Nits (non-blocking)
Things I confirmed are sound
Once the constructor names are fixed, |
brbrr
force-pushed
the
migrate/eth-types-on-non-l1-callers
branch
from
c718afc to
5da18df
Compare
Contributor
PR Re-review: use l1/eth types in non-L1 callers
The previous blocker (undefined |
Contributor
|
Claude finished @brbrr's task in 3m 59s —— View job PR Re-review: use l1/eth types in non-L1 callers
This is now clean — no blockers, no important issues. Good to merge. ✅ Verification
Things I confirmed are sound
Previously-raised items, now addressed
Clean, well-scoped mechanical migration with a solid test safety net. |
RafaelGranza
approved these changes
Jun 9, 2026
RafaelGranza
left a comment
RafaelGranza
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
One small nit
Contributor
|
Claude finished @brbrr's task in 3m 5s —— View job PR Review: use l1/eth types in non-L1 callers
This is clean — no blockers, no important issues, no nits. ✅ A tidy, well-scoped mechanical migration from Verification
Things I confirmed are sound
Previously-raised items, all addressed
Already approved by @RafaelGranza and @rodrodros — concur, good to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Switches the eth types usage from geth to our own defined types on non-l1 callers.