Repository navigation
[2.x] Port the dynamic client to Codama v2 - #1209
Conversation
|
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Ports @codama/dynamic-client to v2 IDLs on top of the already-ported @codama/dynamic-instructions / @codama/dynamic-address-resolution. The runtime surface shrinks accordingly: MethodsBuilder now takes a NodePath<InstructionNode> and forwards { accounts, data, signers } to createInstructionsBuilder(path); .resolvers() is gone; collectPdaNodes becomes collectPdaPaths so resolveStandalonePda({ path, seedsInput }) gets a real path; codegen emits ${Name}InstructionDataArgs / ${Pda}Seeds and drops ${Name}Resolvers. Deps are trimmed (superstruct out, @codama/dynamic-codecs + @solana/codecs to dev) — I checked src/ and nothing there imports either, so that's correct. Test IDLs are committed as upgrade() output and the tests were rewritten to assert on CodamaError codes/context instead of message regexes, which is a nice robustness win.
The source changes are small and read correctly against the downstream packages' current signatures (getResolutionRefs(ix, definedTypes), resolveStandalonePda({ path, seedsInput }), InstructionInput). Approving — the points below are doc/edge-case notes rather than blockers.
Things to watch
- Inline PDAs with a
pdaValueNode.programId(inline comment oncollect-pdas.ts):client.pdas.<name>()for a PDA collected from an account default will derive against the enclosing program, sinceresolveStandalonePdahas no context to resolvepdaValueNode.programId. v1pdaValueNodedidn't carryprogramId, so this is a v2-specific gap. Probably worth either skipping those or documenting. - Two PDA collectors:
collectPdaPathshere andcollectPdaNodesFromIdlin@codama/dynamic-address-resolution/codegenimplement the same scan independently. The generated${Program}Pdastype comes from the latter, the runtime proxy from the former — if they ever diverge (e.g. one starts skippingprogramIdinline PDAs), types and runtime will disagree. Not new to this PR, just flagging since both were touched by the port. - README "Resolved inputs" example (inline comment): I believe
masterEditionis an optional account without a default oncreate, so the snippet as written would throwACCOUNT_MISSINGunder the new "optional-without-default requires explicitnull" rule. - Changeset: I don't see a
.changeset/*.mdin the file list. This removes.resolvers(), renames every generated type, and changes dependencies — all user-facing for@codama/dynamic-client. If the seeded 2.x major changeset is meant to cover the port for every package that's fine, but if per-package changelog entries are expected it'd need one (npx changeset add --empty).
Notes for subsequent reviewers
- The semantics change for optional accounts that used to be auto-resolved by a resolver (e.g.
splTokenProgramon MPLcreate): omitting them now throwsACCOUNT_MISSING;nullapplies theoptionalAccountStrategy. That's enforced byresolveInstructionAccountAddressdownstream and the README documents it — just be aware when reading the rewrittencreate-with-resolvers.test.ts. generateClientTypesresolvesdefinedTypeLinkNodedata only againstroot.program.definedTypes; links intoadditionalProgramsfall back tounknown. Fine for now, but if anyone hits that case it's where to look.- Error-assertion tests were mostly fetched in full (
nested-example-ix.test.tstail is truncated in the diff view) — the expectations line up with the new codec error shapes.
105952b to
88b6bfc
Compare
a1a4ae3 to
7c1d78c
Compare
trevor-cortex
left a comment
There was a problem hiding this comment.
Re-review
Re-fetched everything that changed since my last pass. The update adds an explicit programId escape hatch for standalone PDA derivation, which addresses the pdaValueNode.programId gap I raised, and fixes the README snippet (masterEdition: null). Still approving.
What changed
resolveStandalonePdagains an optionalprogramId?: AddressInputwith precedence givenprogramId→pdaNode.programId→ defining program. The implementation is a one-liner on top of the existinggetPdaProgramAddress(path)/derivePdasplit, andtoAddressmeans it acceptsAddress, base58 string, or legacyPublicKey-like. Tests cover all three fallback levels plus the string form.client.pdas.x(seeds, { programId })threads it through;PdaOptionsis exported from the package root. Thepdas programId optionunit test is a good one: it builds the inline PDA via the instruction (programId: accountValueNode('otherProgram')) and viaclient.pdas.vault(..., { programId: otherProgram })and asserts both agree. That's exactly the equivalence the feature promises.- Codegen:
${Program}Pdasfunctions now takeoptions?: { programId?: Address }.Addressis already in the generated header imports (getTypesFileHeader), so no missing-import risk. The generated type is narrower than the runtime (AddressvsAddressInput), which matches how seeds are typed — fine, just noting it's deliberate. - New
IDL versionstests pin theVERSION_MISMATCHbehaviour for1.5.0and3.0.0, and thecreateProgramClienterror tests now assert on error codes/context rather than messages.
Verified
- Both PDA collectors (
collectPdaPathshere,collectPdaNodesFromIdlin codegen) still produce the same key set — the type/runtime sync concern from my previous review isn't affected by this change since neither started filtering onprogramId. pdaOptions: PdaOptions = {}default meansclient.pdas.x(seeds)andclient.pdas.x(seeds, undefined)behave identically.resolveStandalonePdatest setup (programIdValueNodeconstant seed) confirms the override also flows intoprogramIdValueNodeseeds viaderivePda'sprogramAddressparameter, so a PDA seeded with its own program id derives correctly under the override.
Remaining notes (non-blocking, unchanged from last time)
- No
.changeset/*.mdin the diff.@codama/dynamic-address-resolutionnow has a public API addition on top of thedynamic-clientbreaking changes, so if per-package changelog entries are expected on the 2.x branch this PR would want one for both. If the branch-level major changeset covers it, ignore. generateClientTypesstill resolvesdefinedTypeLinkNodeonly againstroot.program.definedTypes— same as before, just flagging for anyone hittingunknownin generated data types.
7c1d78c to
6c69a42
Compare
297dd25 to
aa957c7
Compare
400bc63 to
bb3f67c
Compare
aa957c7 to
a98f6b8
Compare
a98f6b8 to
8dff65d
Compare
4ab3131 to
d543524
Compare
8dff65d to
3ed69d8
Compare
d543524 to
b784060
Compare
3ed69d8 to
c89f907
Compare
c89f907 to
d41ee16
Compare

This PR ports
@codama/dynamic-clientto Codama v2 IDLs.VERSION_MISMATCH. Upgrade them first withcreateProgramClient(upgrade(v1Idl)), which keeps the client decoupled from@codama/upgrade..resolvers()removed. Inputs carrying acodama.resolverplugin have no default, so they must be provided (or set tonullfor optional accounts).data, remaining accounts are passed asAddress[]in.accounts(), and PDAs are derived from their node paths.client.pdas.x(seeds, { programId })derives a PDA from the given program, like an instruction'spdaValueNode.programId. It's backed by a new optionalprogramIdonresolveStandalonePdain@codama/dynamic-address-resolution.${Name}InstructionDataArgsand${Pda}Seeds, and no longer emits${Name}Resolvers.superstructis dropped, and@codama/dynamic-codecsand@solana/codecsmove to devDependencies, sincesrchasn't used them since the instruction builder was extracted.