refactor: use l1/eth types in non-L1 callers

[brbrr]

Switches the eth types usage from geth to our own defined types on non-l1 callers.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.61%. Comparing base (a79a303) to head (217ddd2).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3704      +/-   ##
==========================================
- Coverage   76.62%   76.61%   -0.02%     
==========================================
  Files         409      409              
  Lines       37030    37030              
==========================================
- Hits        28376    28370       -6     
- Misses       6666     6672       +6     
  Partials     1988     1988              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Claude finished @brbrr's task in 3m 44s —— View job

---

PR Review: use l1/eth types in non-L1 callers

• Read context & gather the diff
• Review core / blockchain / rpc / adapters / l1 changes
• Check new wire_parity_test.go files
• Verify build & post feedback

---

This is a clean, well-scoped mechanical refactor swapping go-ethereum/common.{Address,Hash} for the in-repo l1/eth equivalents across non-L1 callers. Go compilation passes for all touched packages (the only local build failure is the cgo link step for cmd/juno, which needs prebuilt Rust artifacts — an environment issue, not from this PR). No blockers or important issues found.

A few things I specifically checked and confirmed are sound:

• JSON wire parity — MsgToL1/MsgFromL1 address fields change type, but eth.Address.MarshalText emits lowercase, non-checksummed 0x-hex, matching what geth's common.Address produced via encoding/json (geth uses MarshalText, not the EIP-55 String()). The new wire_parity_test.go files pin this byte-for-byte, and l1/eth/address_test.go proves parity against geth directly. 👍
• eth.Address(coreContractAddress) conversion in l1/eth_subscriber.go — valid, both are [20]byte.
• validate:"required" on the retyped fields — behavior is unchanged since both old and new types are fixed-size [20]byte arrays.
• HexToAddress on user config (cmd/juno) — silently crops/pads malformed input, but that matches the prior geth behavior, so no regression.
• Callers of L1HandlerTxnHash (rpc/v{8,9,10}/l1.go, mock) all updated consistently to pass msgHash.Bytes().

Findings

Nit

• blockchain/blockchain.go:231 — L1HandlerTxnHash now takes []byte + a runtime length check, where the old *common.Hash enforced 32 bytes at compile time. Using eth.Hash would keep that invariant compile-time and remove the L1HandlerMsgHashLength branch. Non-blocking; current callers can't trip the check. (left inline)

Nice touch adding the wire-parity tests alongside the migration — that's exactly the safety net this kind of change needs.
· migrate/eth-types-on-non-l1-callers

[github-actions]

Nit (design): the previous *common.Hash parameter guaranteed a 32-byte hash at compile time. Switching to []byte moves that guarantee to a runtime length check, which all current callers (rpc/v{8,9,10}/l1.go passing msgHash.Bytes()) can never trip. Since this PR introduces the l1/eth types, using eth.Hash here would keep the invariant compile-time-enforced and drop the need for L1HandlerMsgHashLength + the error branch entirely:

func (b *Blockchain) L1HandlerTxnHash(msgHash *eth.Hash) (felt.Felt, error) {
    b.listener.OnRead("L1HandlerTxnHash")
    return core.GetL1HandlerTxnHashByMsgHash(b.database, msgHash.Bytes())
}

Not blocking — the []byte + validation approach is safe given the callers convert from a fixed-size common.Hash. Just flagging that the typed variant is stronger and fits the PR's direction.

[Copilot]

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.Address with l1/eth.Address across RPC v8/v9/v10 message DTOs and core message types.
• Refactor L1HandlerTxnHash plumbing 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]

if v8 is going to deprecated meaning v8 will eventually be deleted, shouldn't the rationale go in the latest rpc version?

[github-actions]

Claude finished @brbrr's task in 4m 35s —— View job

---

PR Review: use l1/eth types in non-L1 callers

• Read context & gather the diff
• Review core / blockchain / rpc / adapters / l1 changes
• Verify build / vet on touched packages
• Post review feedback

The migration itself is clean and well-scoped, but it does not compile against the current main — this looks like it regressed after the PR was retargeted/rebased.

Blocker

• Build broken: undefined eth.HexToAddress / eth.HexToHash / eth.BytesToAddress (inline). The merged l1/eth package (feat(l1): Add L1 Eth types #3700) named its constructors AddressFromString / AddressFromBytes / HashFromString / HashFromBytes, but this PR uses the geth-style names at 31 call sites. Verified:

  $ go build ./blockchain/networks/
  blockchain/networks/network.go:80:28: undefined: eth.HexToAddress
  ...
  

  The fix is a mechanical rename:

  this PR uses	should be	files
  eth.HexToAddress	eth.AddressFromString	network.go/_test.go, sn2core.go, vm2core.go/_test.go, cmd/juno/juno.go/_test.go, rpc/v{8,9,10}/wire_parity_test.go
  eth.HexToHash	eth.HashFromString	blockchain_test.go, migration/deprecated/migration_pkg_test.go
  eth.BytesToAddress	eth.AddressFromBytes	adapters/p2p2core/felt.go

  Earlier CI on this PR passed because the original base (add/l1-eth-types) still exposed the geth-style names; the rename landed in feat(l1): Add L1 Eth types #3700 on main.

Nits (non-blocking)

• blockchain/blockchain.go:231 — L1HandlerTxnHash([]byte) trades a compile-time 32-byte guarantee (*common.Hash) for a runtime length check. Using eth.Hash would keep the invariant compile-time and drop L1HandlerMsgHashLength + the error branch (already flagged in the prior review). The added negative tests are a nice safety net for the []byte approach if you keep it.
• @rodrodros raised a fair point on wire_parity_test.go placement — if v8 is slated for deletion, the documenting rationale is better anchored in the latest RPC version (v10). Worth a follow-up, not a blocker.

Things I confirmed are sound

• JSON wire parity for MsgToL1/MsgFromL1: eth.Address.MarshalText emits lowercase non-checksummed 0x-hex, matching geth's encoding/json output. The new parity tests pin this byte-for-byte. 👍
• common.Address(coreContractAddress) bridge in l1/eth_subscriber.go — valid [20]byte conversion.
• validate:"required" on retyped fields — unchanged behavior (both are fixed-size arrays).

Once the constructor names are fixed, go build ./... / go vet ./... should pass and this is good to go.
· migrate/eth-types-on-non-l1-callers

[github-actions]

Blocker — does not compile. eth.HexToAddress is undefined. The merged l1/eth package (#3700) names its constructors AddressFromString / AddressFromBytes / HashFromString / HashFromBytes, not the geth-style HexToAddress / BytesToAddress / HexToHash this PR uses.

$ go build ./blockchain/networks/
blockchain/networks/network.go:80:28: undefined: eth.HexToAddress
...

This breaks 31 call sites across the PR. The mapping is mechanical:

• eth.HexToAddress(s) → eth.AddressFromString(s) — network.go, network_test.go, sn2core.go, vm2core.go/_test.go, cmd/juno/juno.go/_test.go, rpc/v{8,9,10}/wire_parity_test.go
• eth.HexToHash(s) → eth.HashFromString(s) — blockchain_test.go, migration/deprecated/migration_pkg_test.go
• eth.BytesToAddress(b) → eth.AddressFromBytes(b) — adapters/p2p2core/felt.go

Likely the PR was written against the original add/l1-eth-types base (which used geth-style names) and the rename to *FromString/*FromBytes landed in #3700 before this was retargeted onto main.

Suggested change

	CoreContractAddress: eth.HexToAddress("0xc662c410C0ECf747543f5bA90660f6ABeBD9C8c4"),
	CoreContractAddress: eth.AddressFromString("0xc662c410C0ECf747543f5bA90660f6ABeBD9C8c4"),

Fix this →

[github-actions]

PR Re-review: use l1/eth types in non-L1 callers

• Gather diff & check prior blocker status
• Verify build / vet on touched packages
• Re-check correctness (wire parity, conversions, length checks)
• Post review feedback

The previous blocker (undefined eth.HexToAddress/HexToHash/BytesToAddress) appears resolved — all call sites now use the *FromString/*FromBytes constructors. Verifying the build now.

View job run

[github-actions]

Claude finished @brbrr's task in 3m 59s —— View job

---

PR Re-review: use l1/eth types in non-L1 callers

• Gather diff & check prior blocker status
• Verify build / vet on touched packages
• Re-check correctness (wire parity, conversions, length checks)
• Post review feedback

This is now clean — no blockers, no important issues. Good to merge. ✅

Verification

• Build passes — go build ./blockchain/... ./adapters/... ./core/... ./rpc/... ./l1/... ./migration/... succeeds. The previous blocker (undefined eth.HexToAddress/HexToHash/BytesToAddress) is fully resolved — all 31 call sites now use eth.AddressFromString / eth.AddressFromBytes / eth.HashFromString.
• go vet passes on all touched packages.
• go test is blocked in this sandbox, but compilation + vet are green and CI/Codecov are passing on the head commit.

Things I confirmed are sound

• Conversion semantics match geth. eth.AddressFromString → AddressFromBytes → SetBytes crops from the left for oversized input and left-pads with zeros for undersized — identical to geth's HexToAddress/BytesToAddress. So sn2core, vm2core, p2p2core, network.go, and cmd/juno config parsing keep their prior behavior (including the silent crop/pad on malformed config, which is a pre-existing trait, not a regression).
• JSON wire parity for MsgToL1/MsgFromL1: eth.Address.MarshalText emits lowercase non-checksummed 0x-hex, matching geth's encoding/json output. The new wire_parity_test.go files (v8/v9/v10) pin this byte-for-byte, including the omitempty nil-From case. 👍
• common.Address(coreContractAddress) bridge in l1/eth_subscriber.go — valid [20]byte conversion to feed the geth filterer.
• validate:"required" on the retyped fields — unchanged behavior (both are fixed-size arrays).

Previously-raised items, now addressed

• ✅ @rodrodros (wire-parity rationale placement) — commit 217ddd2f3 moved the documenting rationale to the v10 file, the latest RPC version.
• ✅ L1HandlerTxnHash([]byte) nit — moot now: blockchain/blockchain.go, blockchain_test.go, the rpc l1.go/l1_test.go files, and the mock are no longer part of this PR, so the compile-time-vs-runtime-length-check tradeoff no longer applies here.

Clean, well-scoped mechanical migration with a solid test safety net.
· migrate/eth-types-on-non-l1-callers