Skip to content

feat(cct-sdk): Add accept admin solana op - #317

Merged
mervin-link merged 59 commits into
cct-sdkfrom
feat/DAPP-10605-accept-admin
Jul 31, 2026
Merged

feat(cct-sdk): Add accept admin solana op#317
mervin-link merged 59 commits into
cct-sdkfrom
feat/DAPP-10605-accept-admin

Conversation

@mervin-link

@mervin-link mervin-link commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

What

  • DAPP-10605
  • Add Solana CCT acceptAdmin and generateUnsignedAcceptAdmin TokenAdminRegistry operations
  • Validate current pending authority before building acceptAdminRoleTokenAdminRegistry

Why

  • Enable pending TokenAdminRegistry administrators to accept the role through the SDK

@mervin-link
mervin-link requested a review from apedrob July 28, 2026 15:55
@mervin-link
mervin-link requested review from a team, PabloMansanet and aelmanaa as code owners July 28, 2026 15:55
@vercel

vercel Bot commented Jul 28, 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

@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.

I ran this op live on Solana devnet from an isolated build:

  • deployToken 3NSLt4D1…c77KW→ deployTokenPool → createLookupTable → registerToken(owner) 2EaqH4xV…rCrs (registry: administrator = zero default, pending = wallet) → acceptAdmin 2Uendc85…bQiXsetPool succeeds 2UqWZfap…VS3U — the exact call that reverted Unauthorized (7000) in the register-token review before this op existed.
  • Wire-exact: the landed instruction data is exactly 6af010ad89d5a3f6 = sha256(global:accept_admin_role_token_admin_registry)[0..8], no args; program log Instruction: AcceptAdminRoleTokenAdminRegistry; accounts (config RO, registry writable, mint RO, authority signer+writable) match the canonical AcceptAdminRoleTokenAdminRegistry context (token_context.rs:179-196) and what ccip-solana-bs58-generator's accept-admin-role builds. Read the registry PDA back two independent ways (SDK decode + raw borsh-offset decode): administrator == wallet, pending zeroed — the handler's exact promotion semantics (token_admin_registry.rs:115-132).
  • Both accept paths proven: accept-after-register (admin = zeros) and accept-after-transfer (admin non-zero, via a self-proposeAdmin then a second accept, tx oEExtfeJ…Vb8n) — the preflight gates on pending == authority only, exactly like the chain (never stricter, never looser), and the registry's lookup_table survives the accept untouched, as the handler promises.
  • Isolation semantics are right and typed, client-side before any tx: unregistered mint → CCIPTokenNotConfiguredError; no pending / wrong wallet → CCTParamsInvalidError. Notably better than our bs58 generator, which has no preflight at all for this op.
  • Naming: 👍. acceptAdmin is the one op of the trio that already matches the agreed cross-family scheme (registerAdmin / transferAdmin / acceptAdmin) — nothing to rename here.

Two asks before merge, both raised inline with suggestions: A1 (the "no pending admin" test rejects via the wrong branch — proven with a sentinel-message experiment at this head) and A3 (one message for two different rejection states; the idempotent-retry case reads wrong — hit it live). On stacking: since #313 will rename proposeAdmintransferAdmin on the base branch, and this PR's added lines never reference the propose op, the rename only shuffles adjacent import/barrel lines — landing order #313-rename → #317 keeps both merges trivial. 5/5 new tests pass at the merged form on my machine.

Comment thread ccip-sdk/src/cct/solana/token-admin-registry/operations/accept-admin.test.ts Outdated
Comment thread ccip-sdk/src/cct/solana/index.ts

@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.

Re-reviewed at 0429741 (was 0a53a3c). 6 of 7 prior comments are fixed — thank you. One trivial blocker remains: the "no pending admin" test assertion substring doesn't match the message text, so accept-admin.test.ts fails 1/5.

Fixed at 0429741:

  • A3 (conflated message) ✅ — two distinct messages at accept-admin.ts:69-82, both live-proven on devnet
  • Address docstring ✅ — trimmed to "Router or OffRamp"
  • Test typing ✅ — Partial<GenerateAcceptAdminParams>
  • Discriminator + account-metas assert ✅ — 6af010ad89d5a3f6 + full key flags
  • Execute-path reason assert ✅ — includes('requires authority to be the executing wallet')
  • Journey docs ✅ — @remarks/@see to registerAdmin/transferAdmin/setPool on both facade methods

Remaining (inline): the A1 stub fix is structurally correct (now reaches the !pendingAdministrator branch), but the assertion checks reason.includes('no pending administrator') while the message reads "no administrator is pending for this token…" — word order is swapped, substring doesn't match, test is RED. One-word fix: align the assertion to 'no administrator is pending' or 'nothing to accept'.

Comment thread ccip-sdk/src/cct/solana/token-admin-registry/operations/accept-admin.test.ts Outdated
Base automatically changed from feat/DAPP-10604-propose-admin to cct-sdk July 30, 2026 15:36
@mervin-link
mervin-link merged commit 1030c86 into cct-sdk Jul 31, 2026
2 of 4 checks passed
@mervin-link
mervin-link deleted the feat/DAPP-10605-accept-admin branch July 31, 2026 10:59
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