Repository navigation
fix(cct-sdk): Validate EVM deployment receipt contract address - #421
Merged
Merged
Conversation
|
You must have Developer access to commit code to Chainlink Labs on Vercel. If you contact an administrator and receive Developer access, commit again to see your changes. Learn more: https://vercel.com/docs/accounts/team-members-and-roles/access-roles#team-level-roles |
CI Test Report❌ 2 of 3371 tests did not pass (pass 3363, fail 2, cancelled 0, skipped 6) in 6m 40s Failed suites: Failed testsSummaryCoverage report |
apedrob
approved these changes
Sep 15, 2026
Collaborator
Review — PR #421 (DAPP-11480)
|
| Thing | Address / tx |
|---|---|
Deployer (sender) |
0x9d087fC03ae39b088326b67fA3C788236645b717 |
Real deploy tx (through the patched execute) |
0xe52d379cb196af53b523deac278fad54c84a809cb999eaf057a327c2e90d8f9f |
Deployed ERC20LockBox (SDK-returned == derived == receipt) |
0xBB20C959E940aB92a5A57A36532223eAF950F8F3 |
token it escrows (CrossChainToken 2.0.0) |
0x227f0db66e9bf42a28e1e38438c5f19d0adb2df0 |
4.1 Happy path — SDK-returned address == getCreateAddress(on-chain tx.from, tx.nonce) == receipt (live)
A real deployLockbox through the patched execute, then the address re-derived independently from the mined
transaction's own from/nonce:
=== 11480 verify-receipt-address — live Ethereum Sepolia deploy through the patched execute ===
deploy tx: 0xe52d379cb196af53b523deac278fad54c84a809cb999eaf057a327c2e90d8f9f
SDK-returned address: 0xBB20C959E940aB92a5A57A36532223eAF950F8F3
on-chain tx.from: 0x9d087fC03ae39b088326b67fA3C788236645b717
on-chain tx.nonce: 6208
getCreateAddress(tx.from, tx.nonce) = 0xBB20C959E940aB92a5A57A36532223eAF950F8F3
receipt.contractAddress = 0xBB20C959E940aB92a5A57A36532223eAF950F8F3
ALL THREE AGREE: true (SDK == getCreateAddress == receipt)
4.2 Threat — a substituted receipt address is caught, anchored to the same real (sender, nonce) (live)
The exact comparison execute runs (contractAddress !== expectedAddress), against the genuine locally-derived address:
=== threat: a substituted receipt address is caught by the SDK check (anchored to real values) ===
expectedAddress (real, locally derived): 0xBB20C959E940aB92a5A57A36532223eAF950F8F3
substituted RPC address: 0x0BF3dE8c5D3e8A2B34D2BEeB17ABfCeBaf363A59
SDK check "contractAddress !== expectedAddress" -> true (true = throws "unexpected contract address")
So on a genuine deploy all three sources agree and the returned address is real and openable on-chain; had the RPC
returned any other address, the derived expectedAddress (independent of the RPC) would reject it.
4.3 Verification summary
| Claim | Evidence | Live |
|---|---|---|
| Deploy returns the deterministic CREATE address, RPC-independent | §4.1 all-three-agree, tx 0xe52d…8f9f |
✅ |
| The returned address is the real on-chain contract | §4.1 receipt == returned; §4 Etherscan | ✅ |
A substituted receipt address is rejected (!== expectedAddress) |
§4.2 anchored to real (sender, nonce=6208) |
✅ |
| Every deploy op is EOA CREATE (no factory/CREATE2) → check valid | §2 chainlink-ccip v2.0.0 constructors |
✅ (grounded) |
null / malformed receipt address rejected with context.{expected,returned} |
operation.test.ts (3 cases) |
mocked |
5. MAJOR — public @throws on the deploy methods understate the new behavior
cct/evm/index.ts — deployToken (:1033), deployTokenPool (:1480), deployLockbox (:1527) all still read
@throws {@link CCTTxFailedError} if the tx reverts, fails, or mines without an address. The diff updated the internal
execute JSDoc ("without the expected contract address") but not these three public forwarders, so the documented public
surface omits the two genuinely-new observable failures: an invalid/un-checksummable receipt address and a
mismatch vs getCreateAddress. Fix: update all three to e.g. "…or mines at an unexpected/invalid contract address".
6. Recommended additions (minor — implemented + verified green in the review worktree)
The committed head ecb616ed carries the executable fix and 3 tests but under-documents and under-tests the new
behavior. All three below are implemented, tsc/oxfmt clean, op suite 12/12 with them (patch saved — §8):
- Document the plain-EOA-CREATE assumption (the load-bearing precondition, §2) — add an
@remarkson
EVMDeployOperation.execute: "Assumes a plain EOA CREATE deployment: the expected address is
getCreateAddress(signer, nonce). Correct for every deploy op here (each builds a no-tocreation tx), but would not
hold for a factory/CREATE2 or a smart-account signer whose address is not the CREATE-deriving EOA — such a deployment
would be rejected below." - Cover the two new paths the PR adds but does not test: a null
receipt.contractAddress, and thesubmitno-nonce
guard (asserting the reason is not double-wrapped). The 3 committed tests cover only lowercase-accepted /
substituted-rejected / malformed-rejected. - Public
@throws— the §5 MAJOR.
Two optional NITs (not applied):
- Missing next-action clause. Messages distinguish the cases and carry
context.{expectedAddress,returnedAddress}
(good), but every siblingCCTTxFailedErrorends with a remedy (validate.ts:365,contracts.ts:471/476); a short
"the RPC may be misconfigured" hint would match house style. Internal invariant, no caller param. - Redundant
getAddress.operation.tswrapsgetCreateAddress(...)ingetAddress(...); it already returns
checksummed. Harmless/defensive.
7. Verified consistent (no action)
- Invariant correct for every deploy op: all three subclasses inherit the base
buildUnsigned→deployTx(no-to),
no factory/CREATE2 anywhere; the address is fully determined by(from, nonce). Architecture/type/DX lenses: correct on
all counts. noncesource: captured astx.nonceafterpopulateTransaction— the exact noncesubmitTransactionuses, so
it equals the nonce the mined CREATE consumes; thenextNonce/nonceConsumed/rollbackNonceinteraction stays correct
and the new guard rethrows without double-wrapping.- Threat closed: the returned address derives from
wallet.getAddress()+ the locally-populated nonce, RPC-independent;
a hostile RPC cannot launder a fakecontractAddress. The non-deployEVMOperation.executereads nothing spoofable
from the receipt (returns onlyhash) — no residual gap. - Error type/shape: bare
CCTTxFailedErrormatches every sibling deploy/tx failure (no "mismatch" class exists to
reuse);context: { txHash, expectedAddress, returnedAddress }rides the standard...options.contextspread. - Returning
expectedAddress: asserted equal to the receipt, so identical — and the locally-derived one is the more
trustworthy of the two.
8. Types / build / tests (verified in isolation)
Note on the worktree: the review worktree also held uncommitted edits (my §6 recommendations + §5 fix). To avoid
conflating them with the PR, I stashed them and measured the committed head ecb616ed clean, then restored.
Committed head ecb616ed (PR as submitted), SDK workspace:
tsc --noEmit → EXIT 0
oxfmt --check → EXIT 0
oxlint → EXIT 0
operation.test.ts + submit.test.ts → tests 10, pass 10, fail 0 (operation 3 + submit 7)
npm run check returns EXIT 1, but not from this diff: oxfmt/oxlint/tsc all pass; the non-zero is the
generate:lint stage reporting drift in generated files (ccip-sdk/src/selectors.ts, ccip-sdk/src/api/index.ts) —
network-regenerated, not in the diff, and the same drift the repo clears with its routine chore: selectors commits.
The diff itself introduces no any/unsafe as; the sync nonce capture and submit return-widening typecheck cleanly.
Committed head + my §5/§6 additions: tsc EXIT 0, oxfmt EXIT 0, op suite tests 12, pass 12, fail 0 (adds the
null-receipt and no-nonce tests).
9. Recommendation
Approve. The receipt-integrity check is correct, grounded against every v2.0.0 CCT deploy constructor (§2), and proven
live: on a real Sepolia deploy the SDK-returned address is the deterministic CREATE address, equal to both the derived
value and the on-chain receipt, and any substituted address is provably rejected (§4). Recommended before merge: the
§5 MAJOR (stale public @throws) and the §6 additions (document the EOA-CREATE assumption; add null-receipt +
no-nonce tests) — all implemented in the review worktree and verified green (SDK tsc/lint clean, op suite 12/12). None
blocks the current diff.
Collaborator
Author
@aelmanaa Addressed §5 MAJOR and recommended additions. |
aelmanaa
approved these changes
Sep 28, 2026
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
3 participants
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.
What
CREATEaddress derived from the submitting wallet and nonceWhy