[2.x] Reject values of the wrong type when encoding dynamic codecs

[lorisleiva]

This PR makes the @codama/dynamic-codecs encoders reject values of the wrong type. Kit encoders silently encode many of them, e.g. 'abc' or undefined as a u16 zero, 1.5 as 1 or 'no' as true, which could produce unexpected instruction data or PDAs.

• Every type checks the values it encodes and throws DYNAMIC_CLIENT__UNEXPECTED_VALUE_TYPE otherwise: integers accept integer numbers or bigints, public keys base58 addresses, bytes a Uint8Array or an [encoding, data] tuple, structs and maps plain objects (so a Map or a Date is rejected), arrays, sets and tuples arrays, enums an identifier or a { __kind } object, and standalone enum variants a { __kind } object of that variant.
• Missing options encode as None (previously Some of a zero value) and missing structs as structs with missing fields, so their default values apply. Other missing values throw.
• The error context gains the nodePath of the node that rejected the value, e.g. [root, program, instruction, data, amountField, amountType].
• Renames DYNAMIC_CLIENT__UNEXPECTED_ARGUMENT_TYPE to DYNAMIC_CLIENT__UNEXPECTED_VALUE_TYPE, keeping its code.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 7f7c898

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[lorisleiva]

• [2.x] Adapt the dynamic-instructions builder to Codama v2 #1194
• [2.x] Reject values of the wrong type when encoding dynamic codecs #1193 👈 (View in Graphite)
• [2.x] Adapt dynamic-address-resolution to Codama v2 #1192
• [2.x] Accept bytes, enum shorthands and struct defaults when encoding dynamic codecs #1191
• [2.x] Add resolveTypePath helper to visitors-core #1190
• [2.x] Adapt dynamic-parsers to Codama v2 #1187
• [2.x] Add withPath and visitPath helpers to NodeStack #1186
• [2.x] Adapt dynamic-codecs to Codama v2 #1185
• [2.x] Add relations, nested argument seeds and legacy PDAs to nodes-from-anchor #1181
• [2.x] Adapt nodes-from-anchor to Codama v2 #1180
• [2.x] Adapt instruction visitors to Codama v2 #1179
• [2.x] Adapt update visitors to Codama v2 #1178
• [2.x] Adapt structure visitors to Codama v2 #1175
• [2.x] Move casing helpers to @codama/fragments #1174
• [2.x] Adapt validators to Codama v2 #1173
• [2.x] Adapt visitors-core to Codama v2 #1172
• [2.x] Adapt nodes to Codama v2 #1171
• [2.x] Adapt errors to Codama v2 #1167
• [2.x] Rename node name attribute to identifier #1162
• [2.x] Regenerate the node types from the v2 spec #1158
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[lorisleiva]

@trevor-cortex

[trevor-cortex]

Summary

Adds encode-side type validation to @codama/dynamic-codecs so that values Kit would silently coerce ('abc' → u16 zero, 1.5 → 1, short base58 strings padded into a 32-byte pubkey, etc.) now throw DYNAMIC_CLIENT__UNEXPECTED_VALUE_TYPE instead. The mechanism is a single assertValueType helper that wraps each leaf/container codec in an encode-only transformCodec, capturing stack.getPath() at codec-creation time so the error carries the exact nodePath even after the stack unwinds. Options gain undefined → None, structs gain undefined → {} so field defaults apply, and the error constant is renamed (same code).

The design is sound. A few things I checked and that hold up:

• Layering order is right. Validation is applied inside next(node), so the interceptVisitor transform layer (fixedSize, sizePrefix, offsets…) sits outside the check and the check still sees the raw user value. For structs the undefined → {} transform is correctly outside assertValueType, so encode(undefined) doesn't trip the isObjectRecord check.
• stack.getPath() returns a copy ([...this.currentPath]), so the captured nodePath closures are stable. The node-path tests confirm this through links, enums, options and maps.
• transformCodec preserves fixedSize, so wrapping option/struct items doesn't break the assertIsFixedSize calls in visitOptionType / visitZeroableOptionType.
• nodePath: readonly Node[] in the error context has precedent (CANNOT_RESOLVE_PATH, LINKED_NODE_NOT_FOUND), and encodeContextObject stringifies objects as [object Object], so production error messages won't balloon with the full IDL.
• Bigint range checks remain Kit's responsibility (NUMBER_OUT_OF_RANGE), which is the right split — isInteger only guards the shape.

Things to watch

• Map instances slip through visitMapType — see inline. isObjectRecord(new Map(...)) is true, and Object.entries(map) is [], so a Map silently encodes as an empty map. That's the same class of footgun this PR is eliminating; sets already reject Set instances via Array.isArray, so maps should be symmetric.
• No changeset in the diff. CONTRIBUTING.md says any user-facing change needs one, and this touches three published packages (@codama/errors in the fixed group, @codama/dynamic-codecs, @codama/dynamic-client) with a behaviour change and a renamed export. If the 2.x seeded major changeset is meant to cover this, fine — just confirming it's deliberate.
• Renamed export is a hard break. CODAMA_ERROR__DYNAMIC_CLIENT__UNEXPECTED_ARGUMENT_TYPE disappears from @codama/errors and @codama/dynamic-client. The repo kept FAILED_TO_EXECUTE_RESOLVER as a @deprecated constant rather than removing it — see inline for an optional alias in the same spirit.
• Standalone enumVariantTypeNode without data still accepts anything. In visitEnumVariantType, if (node.data === undefined) return undefined; runs before any shape check, so getNodeValueCodec([enumVariantTypeNode('quit')]).encode(42) encodes to empty bytes. Inside an enum the union's __kind lookup already guards this, so it only affects the standalone case — low priority, but it's the one remaining node kind that doesn't validate.

For subsequent reviewers

• The @solana/addresses runtime dependency is new to dynamic-codecs (for isAddress). Same ^8.4.0 range as @solana/codecs, and the sibling dynamic packages already depend on it, so no version skew — but worth knowing it's there.
• nodeKind in the error context is now redundant with nodePath[nodePath.length - 1].kind; it's kept because $nodeKind is interpolated into the message. Reasonable.
• formatValueType deliberately includes the value for numbers only (number (1.5)), not for strings — presumably to avoid leaking long/sensitive strings into error messages. Consistent with the tests.

[trevor-cortex]

Re-review

Re-fetched the diff. Everything raised in the first pass has been addressed, and the fixes are cleaner than what I suggested:

• Map / non-plain objects — isObjectRecord now checks the prototype (Object.prototype or null) rather than special-casing Map, so Map, Set, Date and class instances are all rejected for structs and maps. The Object.create(null) acceptance test covers the JSON-parse-with-null-proto case too.
• BYTES_ENCODINGS exhaustiveness — now derived from a satisfies Record<BytesEncoding, true> record, so a new encoding in the spec is a compile error here.
• Standalone enumVariantTypeNode — now rejects anything that isn't a plain object with the matching __kind, with the expectedType string adapting to whether the variant carries data. Nice touch that actualType reports variant 'move' when the shape is right but the kind is wrong, and falls back to formatValueType otherwise. The comment noting the check is redundant inside an enum is accurate: the enum's own transform already guarantees isObjectRecord(value) && typeof __kind === 'string', and the union index lookup guarantees the kind matches.
• visitEnumType non-string/non-object values — now throw string | { __kind: string } instead of reaching the union with a garbage __kind. Test added.
• The @deprecated alias was optional; a clean break on 2.x is a reasonable call.

One small note, not blocking: the prototype check in isObjectRecord is realm-sensitive — an object created in another realm (Node vm context, iframe) has a different Object.prototype and will be rejected. That's an unusual scenario for instruction inputs, and the alternative (Object.prototype.toString.call(value) === '[object Object]') has its own holes with Symbol.toStringTag, so I'd leave it as is — just flagging in case a bug report ever shows up with that shape.

The changeset point from the first review still stands if it wasn't deliberate, but I assume the 2.x stack handles it collectively.

LGTM.