[2.x] Adapt the dynamic-instructions builder to Codama v2 - #1194
Conversation
|
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Adapts the @codama/dynamic-instructions builder to Codama v2: every entry point now takes the instruction's NodePath (so additional programs work via findProgramNodeFromPath), data encoding is delegated to getNodeValueCodec from @codama/dynamic-codecs (dropping superstruct and the hand-rolled validators), remaining accounts move from data/argument values to identifier-keyed address lists under accounts, and custom resolvers are removed. AccountsInput in dynamic-address-resolution gains readonly AddressInput[], with a new getAccountInput guard so the resolvers reject lists for named accounts. Error constants are renamed keeping their codes.
The shape is a clear improvement — ~1.3k lines of validation removed in exchange for the codec's own type checks, a single { accounts, data, signers } input object, and much better test coverage (the old package had no builder-level test at all). The optionalAccountStrategy ?? 'programId' default in the readonly downgrade also fixes a v1 gap where the strict === 'programId' check missed IDLs that omit the strategy.
Things to watch out for
FAILED_TO_ENCODE_DATAwraps all non-UNEXPECTED_VALUE_TYPEerrors, including otherCodamaErrors. SincegetNodeValueCodecVisitorevaluates struct field defaults lazily at encode time (seegetDefaultValueGetter), aninjectedValueNodedefault with no provider, or a link failing to resolve inside a default, throws its own Codama error duringcodec.encode(...)— and gets hidden behind a genericFAILED_TO_ENCODE_DATAwith the real code only incause. Suggested inline: rethrow anyisCodamaError(error)as-is and only wrap foreign errors (KitSolanaErrors like out-of-range). Non-blocking.- Codec creation is now eager in
createInstructionsBuilder.getNodeValueCodec(path)runs at builder creation, so an instruction with a brokendefinedTypeLinkNodethrows synchronously fromcreateInstructionsBuilderrather than rejecting onbuild(). Fail-fast is arguably better, but whoever adaptsdynamic-client(which creates a builder per instruction) should be aware a single bad instruction will now take downcreateProgramClientunless it catches per-instruction. - Empty list for a required remaining-accounts tail is accepted. The spec docblock says
isOptionalis "whether the remaining-accounts tail may be empty".getRemainingAccountMetasonly rejectsundefined, sosigners: []passes for a required tail (and the test explicitly asserts this). Same as v1 behaviour, so fine to defer, but worth a deliberate decision — see inline. - Two ways to fail on the same input. A named account given a list throws
INVALID_ACCOUNT_ADDRESSviaassertValidAccountAddressesincreateAccountMetas, butUNEXPECTED_ADDRESS_TYPEviagetAccountInputif you callresolveInstructionAccountAddressdirectly. Minor, see inline.
Notes for subsequent reviewers
test/test-utils.tsis untouched and still uses the v1 node API (name,arguments) — it's the display layer's helper file, being adapted separately. Codegen/CLI tests were moved to the new_setup.ts, so there are now twomakeRoot/generateAddresshelpers side by side. Consolidate when the display PR lands.dynamic-client/src/index.tsstill re-exportsArgumentsInputfromdynamic-address-resolution, which no longer exists. Pre-existing on this branch and out of scope here — only the error constant renames were touched — but it confirmsdynamic-clientwon't type-check until its own adaptation.- No changeset in the diff.
@codama/errors(renamed exports),@codama/dynamic-address-resolution(AccountsInputwidened) and@codama/dynamic-instructionsall have user-facing changes. Per CONTRIBUTING the major cut seeds a changeset covering all packages, so this may be intentional on the2.xbranch — just verify that's the convention being followed for the other[2.x]PRs. - The codegen map still only iterates
idl.program.instructions, so builder types aren't generated for additional programs even though the runtime now supports them. Pre-existing scope, not a regression.
2a7865f to
2f32ece
Compare
trevor-cortex
left a comment
There was a problem hiding this comment.
Re-review
Re-fetched the diff. All four points from the previous pass have been addressed:
- Codama errors pass through unchanged —
createInstructionDataEncodernow rethrowsisCodamaError(error)and only wraps foreign errors inFAILED_TO_ENCODE_DATA. Covered by the newit throws other Codama errors as istest (INJECTED_VALUE_NOT_PROVIDEDsurfacing from a lazily-evaluated default), and the docblock/README paragraph were updated to match. - Empty required remaining-accounts tail — now a documented decision in the
createAccountMetasdocblock ("must be provided, possibly as an empty list"). Fine by me. - One error code for a list given to a named account —
assertValidAccountAddresseslets arrays fall through to the resolver, so bothcreateAccountMetasandresolveInstructionAccountAddressthrowUNEXPECTED_ADDRESS_TYPE. Test updated accordingly. toAddressOrThrowreuse — exported fromdynamic-address-resolution(signature widened tostringsosigners[1]-style names work) and used ingetRemainingAccountMetas; the redundantas Addressis gone.
Nothing new to flag. The remaining (addresses as unknown[]) cast is a pragmatic workaround for Array.isArray narrowing poorly on readonly unions — not worth fighting.
The notes for follow-ups from the previous review still stand (eager codec creation in createInstructionsBuilder when adapting dynamic-client, the duplicated test-utils.ts/_setup.ts helpers until the display PR lands, the stale ArgumentsInput re-export in dynamic-client, and confirming the 2.x changeset convention).
2f32ece to
7e26f27
Compare
2e559ef to
1655168
Compare
43372f4 to
b6d6105
Compare
b79b1c0 to
50f84a6
Compare
b6d6105 to
86623ec
Compare
0b55a54 to
a72493e
Compare
86623ec to
126d297
Compare
a72493e to
a5ba85a
Compare
126d297 to
570230c
Compare
a5ba85a to
6845cc7
Compare
4bfb4f6 to
6c6aeb0
Compare
6845cc7 to
b57efa1
Compare
6c6aeb0 to
b74e525
Compare
8a311bc to
a773e2e
Compare
6d153a0 to
db46f59
Compare
add2ee0 to
cac2333
Compare
db46f59 to
45e5f64
Compare
cac2333 to
eadafbd
Compare
2e6b2d4 to
5a4f4c0
Compare
eadafbd to
7f7c898
Compare
5a4f4c0 to
d8e6a86
Compare

This PR adapts the instruction builder of
@codama/dynamic-instructionsto Codama v2. Its display layer is adapted separately.createInstructionsBuilder(path)takes theNodePathof the instruction, e.g.[root, program, instruction], and returnsbuild({ accounts, data, signers }). The program address now comes from the program defining the instruction, so instructions of additional programs are supported.encodeInstructionArgumentsbecomesencodeInstructionData(path, data), alongsidecreateInstructionDataEncoder(path). Data is encoded with the codec of@codama/dynamic-codecs, which rejects values of the wrong type, so the superstruct validator and dependency are removed. Codama errors raised while encoding pass through, and other encoding errors throwDYNAMIC_CLIENT__FAILED_TO_ENCODE_DATAwith the original error ascause.createAccountMetabecomescreateAccountMetas({ path, accounts, data, signers }), resolving accounts throughresolveInstructionAccountAddress. Remaining accounts are provided as lists of addresses under their identifier, e.g.{ signers: [a, b] }, whichAccountsInputnow accepts.resolverValueNode, andArgumentsInputbecomesDataInput.InstructionsBuilderFn<${Name}InstructionDataArgs, ${Name}Accounts, ${Name}Signers>.DYNAMIC_CLIENT__FAILED_TO_ENCODE_ARGUMENTandDYNAMIC_CLIENT__INVALID_ARGUMENT_INPUTerrors toDYNAMIC_CLIENT__FAILED_TO_ENCODE_DATAandDYNAMIC_CLIENT__INVALID_ACCOUNT_INPUT, keeping their codes, and deprecatesDYNAMIC_CLIENT__DEFAULT_VALUE_MISSING.