[2.x] Adapt dynamic-address-resolution to Codama v2 - #1192
Conversation
|
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Adapts @codama/dynamic-address-resolution to the v2 node model. The big structural shift is that callers now pass a NodePath ([root, program, instruction, account] / [root, program, pda]) instead of root + ixNode + ixAccountNode, which lets the package reuse LinkableDictionary, ProvidedScope and resolveTypePath from core rather than hand-rolling link resolution. All the bespoke encoding machinery (codec-input-transformer, default-value-encoder, value-node-value, bytes-encoding, codecs) is deleted in favour of getNodeValueCodecVisitor / getValueNodeVisitor from @codama/dynamic-codecs, and resolverValueNode support goes away with the node. Net −3.3k lines, and the new context.ts + slimmed-down derivePda are much easier to follow than what they replace.
I read every source file and the main test files. The logic checks out: getDataValue correctly gets a structFieldTypeNode back from resolveTypePath (it returns the last node "as addressed"), the ProvidedScope frame ordering in getProvidedScope is parents-first as the class expects, and the WeakMap linkables cache keyed on root is a nice touch. Tests cover the new behaviours well (cross-program PDA links, pdaValueNode.programId from data/accounts, nested data paths, field defaults, injected values, enum/integer condition comparison).
Things to watch
- Constant
programIdValueNodeseeds now encode the resolved program address (derivePdaL92–94), i.e. thepdaValueNode.programIdoverride when present, not the PDA's defining program. This matches what renderers-js does with itsprogramAddressparameter, so I believe it's the right call — just flagging it as a deliberate semantic since v1 always used the PDA's own program. - Condition without
valuenow uses!== undefined && !== nullrather than truthiness, so0,falseand''pass. The description calls this out; just noting it's a behaviour change from v1. @codama/dynamic-clientstill imports removed symbols (ArgumentsInput,CODAMA_ERROR__DYNAMIC_CLIENT__ARGUMENT_MISSING) and the oldresolveInstructionAccountAddressshape. It was already broken by the errors rename before this PR, so I assume it's excluded from CI on this branch and adapted in the next PR of the stack — worth confirming.is-account-auto-resolvable.tsis untouched.accountDataValueNodeis now inACCOUNT_DEFAULT_VALUE_SUPPORTED_NODE_KINDSbut throwsUNSUPPORTED_NODE, andinjectedValueNodeonly resolves when a parent provides the key — yet codegen still marks both as?(omittable). Pre-existing foraccountBumpValueNode, so fine as a follow-up, but the generated types will over-promise for those three kinds.- No changeset. CONTRIBUTING says user-facing changes need one; if the seeded v2 major changeset is meant to cover all adaptation PRs on this branch, ignore this — otherwise the README-level API rename (
argumentsInput→dataInput,path, removed exports) probably deserves a line.
Inline comments below are all non-blocking suggestions.
For subsequent reviewers
getDataValueinsrc/resolvers/context.tsis the linchpin fordataValueNodedefaults, seeds and conditions — worth a close read of theomitted/ default-value fallback ordering.isValueEqualinsrc/shared/util.tsdefines cross-representation equality for conditions; check the enum handling matches whatever shapesgetValueNodeVisitoractually emits in@codama/dynamic-codecs.
bbdd5f5 to
803123b
Compare
trevor-cortex
left a comment
There was a problem hiding this comment.
Re-review
Re-fetched every changed source file and the affected tests. All six inline points from my first pass have been addressed, and the fixes are clean:
nulloption seeds —derivePdanow only treatsundefinedas missing and, forOPTIONAL_NODE_KINDS, encodesnull(i.e.None) instead of throwingPDA_SEED_MISSING. Tests added foroptionTypeNode,zeroableOptionTypeNodeandremainderOptionTypeNode, matching theT | nullthe generated${Pda}Seedstype already advertised.as stringcast forpdaValueNode.programId— replaced with a sharedtoAddressOrThrowhelper inshared/address.tsthat runsisAddressConvertibleand raisesUNEXPECTED_ADDRESS_TYPE, andaccount-default-value.tsnow uses the same helper. Nice consolidation.- Nested default lookup —
getDataValuenow walks the path withgetStructFieldValue, applying each field'sdefaultValuealong the way (soconfig.ownerreadsownerout ofconfig's struct default whenconfigis absent), and respectsomittedfields. Docblock explains the behaviour. Test added for the parent-default case. Uint8ArrayinisValueEqual— explicit branch comparing byte-wise, with tests.getResolutionRefson unresolvable links — falls back toix.dataitself sohasData/hasRequiredDataare still correct; the unresolvable-link test now assertsdataRefis set.- Value-less condition on unprovided account —
visitAccountValueincondition-node-value.tsnow returnsundefinedwhen the account is neither provided nor has a default, soifFalseis reachable; tests cover both branches.
Also new since last pass: isAccountAutoResolvable was updated so accountBumpValueNode, accountDataValueNode and injectedValueNode are no longer treated as auto-resolvable (with tests), so the generated types stop over-promising for those kinds. That closes the follow-up I flagged in the review body.
Nothing new to flag. The remaining open items from the first review (the dynamic-client compile break, changeset coverage) are stack-level and unchanged — still assuming they land in a later PR.
Approving.
803123b to
f4b10ab
Compare
9c91444 to
81bd5ed
Compare
f4b10ab to
ef341d9
Compare
cb8dd0b to
45c081b
Compare
5acfda4 to
1e5a11f
Compare
45c081b to
e3bfabb
Compare
1e5a11f to
94bf7c5
Compare
45c081b to
e3bfabb
Compare
1e5a11f to
94bf7c5
Compare
e3bfabb to
97d1f4c
Compare
94bf7c5 to
59d18a6
Compare
97d1f4c to
9812253
Compare
59d18a6 to
3210487
Compare
9812253 to
fcd8c62
Compare
9b81b4a to
517ca2f
Compare
fcd8c62 to
375cb5e
Compare
517ca2f to
4043eca
Compare
36c98ff to
af66e5f
Compare
4043eca to
e540f66
Compare
af66e5f to
608b4bd
Compare
e540f66 to
2fe5e55
Compare
608b4bd to
1ef2655
Compare
83e5377 to
6f075e0
Compare
1ef2655 to
9c01248
Compare
9c01248 to
4d1c3d7
Compare

This PR adapts
@codama/dynamic-address-resolutionto Codama v2.resolveInstructionAccountAddresstakes thepathof the instruction account andresolveStandalonePdathepathof the PDA, from which links and injected values are resolved.argumentsInputbecomesdataInput, using the value format of@codama/dynamic-codecs.dataValueNodedefaults and seeds resolve nested paths such asconfig.owner, using the default value of each field along the path, including parent fields, the same way their codec encodes them.resolverValueNode. Accounts without default values must be provided.@codama/dynamic-codecs, which now accept these inputs. PDA seeds are encoded with their declared type, and missing option seeds encode asNone.injectedValueNode,programLinkNodeandpdaValueNode.programIddefaults, as well as PDAs of other programs.2andintegerValueNode('2'), and pass when their condition exists when they have novalue.${Name}InstructionDataArgs,${Pda}Seeds, and remaining accounts as named account lists in${Name}Accounts.DYNAMIC_CLIENT__ARGUMENT_MISSINGerror toDYNAMIC_CLIENT__DATA_MISSING, addsDYNAMIC_CLIENT__PDA_SEED_MISSING, and deprecates the resolver errors.