feat(cct-sdk): Add deploy verification + unify ops - #316
Conversation
…10418-handle-pool-versioning
…10418-handle-pool-versioning
…10418-handle-pool-versioning
…10418-handle-pool-versioning
* feat(cct-sdk): Source token + pool abi/bytecode from contracts-ccip * Remove outdated bytecodes
* feat(cct-sdk): Source token + pool abi/bytecode from contracts-ccip * feat(cct-sdk): Add versioned Deploy Token (CCT + ERC20) * Remove outdated bytecodes * Deploy only CrossChainToken * Recover previous type changes
Vendor ERC20LockBox v2.0.0 artifacts; add the cached lockbox Interface + DeployLockbox op, wire generateUnsignedDeployLockbox/deployLockbox into EVMTokenManager, and document the LockRelease deploy sequence.
Add AuthorizeLockboxCallers op (wraps ERC20LockBox applyAuthorizedCallerUpdates) + tests, and wire generateUnsignedAuthorizeLockboxCallers/authorizeLockboxCallers into EVMTokenManager to complete the LockRelease flow.
|
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 |
aelmanaa
left a comment
There was a problem hiding this comment.
Approve with fixes — the unification is real and non-regressive, and the verification handle is live-proven. I re-ran the whole thing on Sepolia at this head: deployLockbox through the new EVMDeployOperation deployed 0xC645befF…55B1 (tx 0xa98116b2…), and the returned verification.encodedConstructorArgs matched both an independently computed abi.encode([token]) and the on-chain tx.data tail after the vendored bytecode — the slice-recovery math holds against real chain data, and the zero-token rejection still fires client-side with the exact pre-refactor message (nonce unchanged). I also confirmed the PR-body claim: your Basescan pool 0x71ba2bfD… is verified (BurnFromMintTokenPool), i.e. the handle drives a real explorer verification.
What I like: the three copy-pasted deploy executes collapse into one audited path; the contracts.ts layer is now symmetric across token/token-pool/lockbox; every public surface approved on #307/#311/#312 keeps its exact shape, validators and messages (I checked param-by-param and live); and the per-type verification tests assert against the hand-encoded golden vectors rather than the SDK encoder.
Three asks before merge, in order: (①) the authorizeLockboxCallers lockbox zero-check — this PR built the exact helper and applied it everywhere except the one target I live-proved produces a status-1 tx to 0x0; it's now a one-word diff. (⑤) merge the updated base so the head stops failing npm run check on inherited Solana staleness. (②) make the new verification field visible in the docs/examples — it currently ships invisible. One framing note: this is explorer-verification support (valuable!), not deployability verification — execute does no post-deploy reads; with the shared deploy base you now have, a single typeAndVersion read-back would give every deploy op the on-chain self-check my reviews keep doing manually (③, follow-up welcome).
Landing order note: this stacks 10545 → 10738 → 10788 → 10572 (#307 → #311 → #312 → #316); #311's // generate:end artifact-gate fix and #312's items travel with their own PRs — nothing here supersedes them, and #312's zero-lockbox comment is restated (not mooted) by this PR.
Addendum (multi-explorer follow-up): I subsequently exercised the handle against all three EVM explorer verification APIs (Etherscan v2, Blockscout, Sourcify) — with only SDK-provided data, none of the three requests can be formed (details + live transcripts in ⑥). The handle is a fine constructor-args companion, but "deploy verification" currently rides on Etherscan SimilarMatch; completing it needs the standard-json/compiler side, which the @chainlink/contracts-ccip npm package already ships (live-proven end-to-end on Sepolia against all three explorers). A complete reference implementation of the missing pieces (Etherscan v2 / Sourcify / Blockscout clients, per-chain verifier routing, solc long-version resolver, bundled std-json fixtures + manifest, and a CLI verify command with CREATE2 recovery) already exists on the org branch smartcontractkit/feat/propose-admin-role @ 1cecb26 (PR #204) and can be lifted directly — see ⑥ for the file-by-file map and the two ways to close this. And ⑦: the ctor-args handle it does build is dropped on the generateUnsigned* path (the async/multisig deploy flow), which is a one-accessor fix independent of ⑥.
Coverage caveat (⑧): I ran a full completeness audit of the deployable set. Two of this PR's six deployables — BurnFromMintTokenPool and BurnWithFromMintTokenPool — have no std-json fixture on either this PR or the reference branch, and since each burn-* variant has distinct bytecode (contracts.ts:162), BurnMintTokenPool's fixture can't verify them. So even adopting the branch, those two stay unverifiable until fixtures are generated (details + settings-pin note in ⑧). The audit otherwise confirmed ⑥/⑦ are the only design gaps and that the init-code slice is correct for every #316 deploy (all are direct CREATE — there's no factory/CREATE2 path in this PR's scope, so no silent-slice risk).
On direction (DX): verification is inherently a post-deploy, address-first, retryable operation, so my recommendation is to make the address-based verifier the primary path (derive ctor args from the deployed address / creation tx, incl. CREATE2, submit full std-json across all four providers — already built on feat/propose-admin-role, PR #204) and keep this PR's inline handle as an optional zero-RPC fast-path input into it when you deploy directly. That closes the two verification asks together and avoids two half-verification stories. Honest cost of the address model: a debug_traceTransaction dependency for factory/CREATE2 (gated on some RPCs) — worth weighing, not disqualifying.
| deployData: string, | ||
| bytecode: string, | ||
| ): DeployVerification { | ||
| return { contract, encodedConstructorArgs: `0x${deployData.slice(bytecode.length)}` } |
There was a problem hiding this comment.
[major — scope/completeness] First, the structural point: once you have the handle, the SDK offers no way to actually verify. There is no submission surface at all — no Etherscan/Sourcify/Blockscout client, no verifyContract, no verify/ module (git grep for any of these across ccip-sdk/src + ccip-cli/src at this head returns nothing; the verify/ directory has 0 files). DeployResult.verification answers "what are the constructor args?", not "is my contract verified?" — the developer is left to source std-json + compiler version + FQN elsewhere and POST to each explorer by hand.
I live-tested exactly how far the handle gets you, using only what the SDK provides (bare contract name + encodedConstructorArgs; the SDK vendors ABI/bytecode but no sources, no compiler version, no settings, no qualified path/File.sol:Name):
- Etherscan v2
verifysourcecode→NOTOK: Missing sourceCode(the request can't even be formed); - Blockscout standard-input API →
Bad request(files[]mandatory); - Sourcify v2 →
must have required property 'stdJsonInput'.
So today the handle only serves the manual Etherscan form — and even there the user must source the std-json, compiler version and qualified name elsewhere. (Both my Sepolia deploys from this head showed as "verified" on Etherscan purely via SimilarMatch before any submission — Blockscout and Sourcify give nothing for free.) The missing inputs are entirely available: @chainlink/contracts-ccip@2.0.0 on npm ships all 118 .sol sources + remappings.txt + foundry.toml — I built a standard-json from the npm package contents alone and it fully verified my LockReleaseTokenPool on Sourcify (creationMatch: match, which then auto-propagated to Etherscan/Routescan/Blockscout), and a pre-built per-contract standard-input took my ERC20LockBox to Pass - Verified on Etherscan and verified on Blockscout.
You don't have to design this from scratch — it's already built on the org branch smartcontractkit/feat/propose-admin-role (PR #204, @ 1cecb26), and its ERC20LockBox bytecode is byte-identical to this PR's, so the same fixtures verify #316's deploys exactly. Concretely, either:
-
grow the handle into the full input set the three APIs need — every piece below is already implemented on that branch and can be lifted directly:
- standard-json-input bundling (sources + settings that produced the bytecode) — the vendored per-contract fixtures + a manifest keyed by contract name:
fixtures/manifest.json, a samplefixtures/ERC20LockBox.standard-input.json, loaded bygetVerificationArtifact(registry.ts); - long
compilerVersionresolver (short0.8.26→v0.8.26+commit.8a97fa7a, offline pin map + solc-list fallback) —solc-version.ts; - fully-qualified
contract(contracts/pools/ERC20LockBox.sol:ERC20LockBox) — thecontractNamefield infixtures/manifest.json; - constructor-args encoder (ABI-encode, no
0x, no selector — Etherscan's exact form) —constructor-args.ts; - Etherscan v2 submission client (submit → GUID poll → already-verified short-circuit → slow-explorer
getsourcecodeconfirm) —etherscan.ts; - Sourcify submission (v2 JSON API,
verificationIdpoll, creation-tx match) —sourcify.ts; - Blockscout support + per-chain verifier routing (67-chain map to etherscan-v2/blockscout/sourcify) —
registry.tsoverfixtures/verifiers.json; - high-level orchestrator that ties it together (
verifyContract/verifyDeployedContract) —verify.ts; - CLI
verifycommand +--verifyon deploy (verify an already-deployed contract, deriving ctor args from on-chain creation code, incl. factory CREATE2 recovery viadebug_traceTransaction) —ccip-cli/.../verify.ts+verify-utils.ts.
The
{ standardJsonInput, compilerVersion, contract(FQN) }tuple is the complete input set for all three APIs; the clients above then submit it. Or - standard-json-input bundling (sources + settings that produced the bytecode) — the vendored per-contract fixtures + a manifest keyed by contract name:
-
add one
@remarksonDeployVerificationstating the scope explicitly: "constructor-args companion for explorer verification; sources/compiler settings must come from@chainlink/contracts-ccip@2.0.0(npm ships them)" — so consumers don't discover the gap the way I did.
Severity is "major" against a multi-explorer support goal; if the intended scope is Etherscan-manual-only, option 2 downgrades this to docs.
There was a problem hiding this comment.
Took option 2. I renamed it to ExplorerVerificationInput since execute never does a
post-deploy check, and added an @remarks saying it's just the constructor args, with
sources and compiler settings coming from @chainlink/contracts-ccip. The op docstrings
and facades now mention verification, so by your own framing this is docs now.
On the rest, I've also created my own verification that I shared with you, so we agree
it's wanted. Shouldn't this be handled at the CLI level though? @chainlink/contracts-ccip
is only a root devDependency, not a dependency of ccip-sdk, which vendors the generated
ABI/bytecode instead. Putting std-json in the SDK means vendoring all .sol files or
adding heavy runtime dep. And even if we want to use the Etherscan API, an API key is
needed, so credentials and polling fit better in the CLI. Your reference implementation
already puts the command there.
Can you create a Jira ticket with the requirements for what the SDK needs to expose to
feed a CLI verify command, so we can plan it?
What
Why
Testing
0x71ba2bfDBD3564D3C8434aA98EA70b07A5bf0fD6) using the SDK verification output