feat: Smart contract signature support in SpokePoolPeriphery

[fusmanii]

No description provided.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0cb8ee83c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Dispatch auth overload by signer type, not signature length

The new branch on signature.length == 65 misclassifies smart-contract wallets that produce 65-byte signatures, so depositWithAuthorization/swapAndBridgeWithAuthorization will call the (v,r,s) overload instead of the bytes-signature overload intended for ERC-1271. In that case, contract-owned authorizations can revert on tokens where ERC-1271 validation is only wired through the bytes variant, which breaks the feature this commit adds for a non-trivial class of smart wallets.

Useful? React with 👍 / 👎.

[fusmanii]

@codex re-review

[fusmanii]

@codex re-review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4842085096

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve v/r/s authorization path for contract signers

Routing by extcodesize(from) forces every contract wallet through the bytes-signature receiveWithAuthorization overload, which regresses compatibility with ERC-3009 tokens that only expose the legacy (v,r,s) selector but still validate ERC-1271 signatures there. Before this change, those tokens could be used with 65-byte contract-wallet signatures; now depositWithAuthorization and swapAndBridgeWithAuthorization will revert for that case because the bytes overload is missing. Choose overload based on signature format (or add a fallback) instead of assuming all contract signers require the new selector.

Useful? React with 👍 / 👎.

[fusmanii]

@codex review

[fusmanii]

@claude review

[claude]

Claude finished @fusmanii's task in 2m 48s —— View job

---

Code Review: Smart contract signature support in SpokePoolPeriphery

• Gather context
• Read changed files
• Analyze signature routing logic
• Review tests
• Post review feedback

---

This PR adds ERC-1271 / smart contract wallet support to the ERC-3009 authorization flow in SpokePoolPeriphery, plus EIP-7702 delegated-EOA awareness. Here's my analysis:

---

Design: extcodesize-based routing

SpokePoolPeriphery.sol:830 — The routing in _receiveWithAuthorization switched from signature-length to extcodesize check, which is the right call. The previous signature.length == 65 approach (Codex P1) was broken for contract wallets that produce standard 65-byte ECDSA signatures. Routing by address type is correct in principle.

if (!_isContract(from)) {
    // (v,r,s) path for EOAs
} else {
    // bytes path for contract wallets (ERC-1271)
}

Issue: Token compatibility assumption (Codex P2 — partially valid)

The routing assumes any token whose from is a contract supports the bytes overload of receiveWithAuthorization. If a token only implements the (v,r,s) overload, calling it with a contract wallet from will revert with a missing-selector error.

In practice this is limited to USDC pre-v2.2 (Circle only added the bytes overload in v2.2). For those tokens, contract wallets couldn't use ERC-3009 authorization anyway (the (v,r,s) overload does ecrecover, not ERC-1271), so this isn't a regression from a security perspective. However, it's worth documenting this limitation in the function NatDoc.

Recommendation: Add a note to _receiveWithAuthorization's NatDoc that tokens must support the bytes overload of receiveWithAuthorization (e.g. USDC v2.2+) when from is a contract wallet. Fix this →

EIP-7702 detection (_isContract, line 872–891)

The 7702 detection is correct and well-implemented:

• The 0xef prefix is reserved by EIP-3541 and cannot appear as the first byte of valid deployed bytecode, so there's no collision risk between a regular 23-byte contract and a 7702 designation.
• Loading only the first 3 bytes via extcodecopy then checking shr(232, ...) is gas-efficient and correct.

One minor concern: the size == 23 guard is load-bearing. A deployed contract that happens to be exactly 23 bytes and starts with 0xef0100 (practically impossible under EVM rules) would be misclassified as an EOA. This is safe given EVM invariants but worth noting.

Missing test: contract wallet with 65-byte signature

The smart wallet tests (testDepositWithAuthorizationSmartWalletSignature, testSwapAndBridgeWithAuthorizationSmartWalletSignature) use an abi.encode-wrapped signature (>65 bytes) to simulate Coinbase Wallet format. There's no test for a smart wallet that returns a raw 65-byte ECDSA signature from isValidSignature. This case should work correctly (routing is by extcodesize, not signature length), but an explicit test would make the invariant clear. Fix this →

MockERC20 bytes-overload implementation

contracts/test/MockERC20.sol:85 — The bytes overload uses SignatureChecker.isValidSignatureNow which handles both EOA and ERC-1271 — good. The (v,r,s) overload does the same via bytes.concat(r, s, bytes1(v)) + SignatureChecker. This makes MockERC20 more permissive than production USDC (which uses ecrecover in (v,r,s) overload), which is fine for tests but means the tests don't actually catch the "token-only-has-v,r,s" failure mode described above.

Authorization flow: no separate depositDataSignature required

Unlike the permit flows, depositWithAuthorization and swapAndBridgeWithAuthorization bind deposit parameters through the ERC-3009 witness mechanism rather than a separate EIP-712 signature. This is correct — the ERC-3009 nonce is set to the keccak256 of the deposit data, so any modification to the deposit data invalidates the authorization. This is a clean design.

swapAndBridgeWithAuthorization: modifiedData.nonce overwrite

SpokePoolPeriphery.sol:386–387 — The nonce field of SwapAndDepositData is overwritten with uint256(witness) before being passed to _swapAndBridge. This means any user-provided nonce in the calldata is ignored for auth flows, which is correct (the witness serves as the nonce). This is non-obvious and could benefit from a comment.

---

Summary

Core routing approach	✅ Correct — extcodesize > signature length
EIP-7702 handling	✅ Correct and well-guarded
Witness-based nonce binding	✅ Secure replay protection
Token compatibility assumption	⚠️ Document that bytes overload is required for contract wallets
Test coverage	⚠️ Missing test for 65-byte smart wallet sig (minor)

The approach is sound. The main action items are documentation and an additional edge-case test.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🚀

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".