Skip to content

fix(cct-sdk): Update and refactor solana lifecycle and tests - #321

Open
mervin-link wants to merge 82 commits into
cct-sdkfrom
fix/refactor-solana-operation
Open

fix(cct-sdk): Update and refactor solana lifecycle and tests#321
mervin-link wants to merge 82 commits into
cct-sdkfrom
fix/refactor-solana-operation

Conversation

@mervin-link

@mervin-link mervin-link commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

What

  • Include CCT sources and builds in the ccip-sdk package and remove the dev build
  • Add Solana prepare lifecycle: prepare (validate → parse) → build unsigned tx → submit
  • Migrate acceptAdmin to parse validated public-key strings into PublicKeys
  • Add validateOptionalPublicKey and replace truthy optional-key validation checks
  • Group Solana operation tests by generate, validation, and execute

Why

  • Enable incremental Solana input normalization without changing existing operation APIs or refactoring all operations at once
  • Make operation test coverage easier to navigate

@mervin-link
mervin-link requested a review from apedrob August 3, 2026 11:04
@mervin-link
mervin-link requested review from a team, PabloMansanet and aelmanaa as code owners August 3, 2026 11:04
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

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

Base automatically changed from feat/DAPP-10812-get-supported-tokens to cct-sdk August 4, 2026 13:29
Comment thread ccip-sdk/src/cct/solana/validate.ts Outdated
Comment thread ccip-sdk/src/cct/solana/validate.test.ts
Comment thread ccip-sdk/src/cct/solana/operation.ts
Comment thread ccip-sdk/src/cct/solana/validate.ts
Comment thread ccip-sdk/src/cct/solana/operation.ts
@mervin-link
mervin-link requested a review from aelmanaa August 5, 2026 11:54

@aelmanaa aelmanaa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

§A — validateOptionalPublicKey + test refactor. Centralizes 8 duplicated if (params.x) validatePublicKey(...) guards into one validateOptionalPublicKey, swapping truthiness (if (x)) for !== undefined. The only new runtime divergence is the empty string: old code silently rewrote a caller's ''/null authority into "use payer" (masking a bug); new code rejects it. ''/null were never valid base58 pubkeys → pure hardening, no program-accepted input newly refused. Aligns to an existing in-file standard (resolvePoolProgram, validateInteger already use !== undefined); validate.ts isn't re-exported, so no CLI↔SDK boundary moves. The deploy-token.ts freezeAuthority?: string | null keeping its explicit !== null guard is correctnull is its documented "disable freezing" sentinel.

§B — parse/prepare lifecycle. SolanaOperation<P,Tx,Parsed=SolanaGenerateParams<P>> gains parse(params): Parsed (no-mutation normalize/default/convert), prepare() = validate()+parse(), and buildUnsigned consumes typed Parsed. The Parsed default makes the 9 non-adopters textually identical to before; adopters (accept-admin, and now configure/remove-allowlist) get typed PublicKeys with one fewer re-parse. Contained (base not re-exported), cross-family Operation<> untouched.

Live devnet proofs (carry forward — the add68cb delta is validation/doc/test only, no instruction/account change):

  • §A createLookupTable on devnet, wallet 2SGSoyjD…C3V3: (a) authority explicit → tx 5Xcj2tj…Hwjvp; (b) authority omitted (→payer) → tx 8NCoUz8…utRt9; both ALTs read back authority = 2SGSoyjD…C3V3. (c) negative authority: '' → rejected pre-flight CCTParamsInvalidError.
  • §B real acceptAdmin LANDED through the new prepare path — tx 5gLXmjt…5ZUzJu, on-chain ix data 6af010ad89d5a3f6, accounts {config, token_admin_registry, mint, authority} byte-identical to the generated instruction; TAR read-back administrator = 2SGSoyjD…C3V3, pendingAdministrator = 1111…1111. Generate-path authority explicit vs omitted → identical instruction (no-mutation confirmed). Grounded to ccip-router/src/token_context.rs:179-196 (authority = Signer with address = pending_administrator @ Unauthorized).

Verified at add68cb: npm run check exit 0 (lint+typecheck+generate), 27/27 targeted tests pass (validate + deploy-token-pool + operation), including the new null/''-rejected and op-level authority:''-throws cases.

Resolved since the last pass (e27c959 → add68cb): §A TSDoc (validateOptionalPublicKey now documents "only undefined is absent; null/'' rejected" + @throws), §A coverage (null-throws + valid-passes + op-level authority:''), §A quoted error message, §B class/parse TSDoc (validate=cross-field / override parse when Parsed differs), and the B4 append-to-lookup-table regression fix. Thanks for these.

Not blockers / correct-by-design: the truthiness→!== undefined shift is pure hardening (''/null never valid pubkeys); freezeAuthority's null sentinel correctly keeps its explicit guard; parse() no-mutation is on-chain-proven byte-identical; the EVM side having no optional-address validator is a pre-existing gap, not this branch's responsibility. The B4 append-to-lookup-table.ts:94 fix (?. restored) is correct.

Comment thread ccip-sdk/src/cct/solana/operation.ts
Comment thread ccip-sdk/src/cct/solana/operation.test.ts
* validation, defaults, or conversion; it must be overridden whenever `Parsed` differs from
* `SolanaGenerateParams<P>`.
*/
export abstract class SolanaOperation<

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[§B nit — base no-op validate] Parse-only ops must still write an empty validate() {} stub. Consider a default no-op validate on the base so those ops can omit the boilerplate (removes the empty stub in configure/remove/accept-style ops).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this, but it would require override across every existing validator under noImplicitOverride. I’ll add a default no-op validation on the base once every op uses prepare.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants