feat: refactor token-kit builder UX + add missing builders#2350
feat: refactor token-kit builder UX + add missing builders#2350
Conversation
…ain permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Implement the 5 missing instruction builders (transfer2, create-token-account, mint-action, claim, withdraw-funding-pool) and their codec, fixing the token-sdk build pipeline end-to-end. Changes: - Fix Codama IDL: add prefixedCountNode for array types, update renderVisitor import - Fix CI workflow: add Codama generation step before token-sdk build - Implement transfer2.ts with Path A/B/C routing and compression factory helpers - Implement create-token-account.ts with compressible/non-compressible modes - Implement mint-action.ts with full account layout - Implement claim.ts for rent recovery - Implement withdraw-funding-pool.ts for pool withdrawals - Implement codecs/mint-action.ts with full Borsh encoding - Add legacy SDK devDeps to token-client for e2e test setup - Add .gitignore for Codama-generated code (build artifact) All 350 unit tests pass (286 token-sdk + 64 token-client). Refs: #2280, #2289
Remove strict V1 tree rejection from assertV2Tree — both tree versions work correctly for all operations. The assertion now only rejects unknown tree types. This enables e2e tests to pass against the standard local test-validator which creates V1 trees by default. All 348 unit tests + 11 e2e tests pass.
…on tests
Refactor all 23 high-level builder functions for clean UX:
- Single params object pattern for all builders (no positional args)
- Address types instead of raw Uint8Array for authorities/recipients
- Optional decimals/tokenProgram with auto-fetch via getMintDecimals
- Auto-resolve merkle context via loadMintContext for mint management builders
- String-based MetadataFieldType enum ('name'|'symbol'|'uri'|'custom')
- MintRecipientParam with Address instead of raw bytes
Add 4 new builders for full parity with compressed-token:
- buildCreateAta / buildCreateAtaIdempotent (auto-derive ATA)
- buildGetOrCreateAta (check + create + decompress cold balances)
- buildDecompressInterface (auto-derive destination + decompress)
Add infrastructure:
- @noble/hashes for compressed address derivation (keccak256)
- deriveCompressedAddress / deriveCompressedMintAddress
- deserializeCompressedMint (lightweight DataView-based)
- loadMintContext (central auto-resolver for mint builders)
- getMintDecimals (reads byte 44 from on-chain mint)
Add 55 unit tests (375 → 430 total) covering all builders:
- actions.test.ts: 45 tests for all builder functions
- Deserializer, query functions, compressed address derivation tests
- Shared mock helpers (createMockIndexer, createMockRpc, createMockMintContext)
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (76)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const treeIdx = getOrAdd(input.merkleContext.tree, AccountRole.WRITABLE); | ||
| const queueIdx = getOrAdd(input.merkleContext.queue, AccountRole.WRITABLE); | ||
| const inputHashKey = bytesToHexKey(input.tokenAccount.account.hash); | ||
| const rootIndex = proofRootIndexByHash.get(inputHashKey) ?? 0; |
There was a problem hiding this comment.
🔴 buildTransferDelegated silently uses rootIndex = 0 when proof entry is missing
buildTransferDelegated uses proofRootIndexByHash.get(inputHashKey) ?? 0 (line 411) to silently fall back to rootIndex = 0 when a selected input's hash is not found in the validity proof. In contrast, buildCompressedTransfer (js/token-kit/src/actions.ts:243-248) correctly throws an IndexerError for the same condition. Using an incorrect rootIndex of 0 will produce a malformed on-chain instruction that fails with a confusing proof verification error, instead of failing fast with a clear SDK-level error message.
| const rootIndex = proofRootIndexByHash.get(inputHashKey) ?? 0; | |
| const rootIndex = proofRootIndexByHash.get(inputHashKey); | |
| if (rootIndex === undefined) { | |
| throw new IndexerError( | |
| IndexerErrorCode.InvalidResponse, | |
| `Missing proof account for selected input hash ${inputHashKey}`, | |
| ); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const treeIdx = getOrAdd(input.merkleContext.tree, AccountRole.WRITABLE); | ||
| const queueIdx = getOrAdd(input.merkleContext.queue, AccountRole.WRITABLE); | ||
| const inputHashKey = bytesToHexKey(input.tokenAccount.account.hash); | ||
| const rootIndex = proofRootIndexByHash.get(inputHashKey) ?? 0; |
There was a problem hiding this comment.
🔴 buildDecompress silently uses rootIndex = 0 when proof entry is missing
buildDecompress uses proofRootIndexByHash.get(inputHashKey) ?? 0 (line 751) to silently fall back to rootIndex = 0 when a selected input's hash is not found in the validity proof. This is the same issue as in buildTransferDelegated — buildCompressedTransfer (js/token-kit/src/actions.ts:243-248) correctly throws for this condition. Using an incorrect rootIndex produces a malformed instruction that will fail on-chain with a confusing proof verification error.
| const rootIndex = proofRootIndexByHash.get(inputHashKey) ?? 0; | |
| const rootIndex = proofRootIndexByHash.get(inputHashKey); | |
| if (rootIndex === undefined) { | |
| throw new IndexerError( | |
| IndexerErrorCode.InvalidResponse, | |
| `Missing proof account for selected input hash ${inputHashKey}`, | |
| ); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const safeText = text.replace( | ||
| /:\s*(\d{16,})\s*([,}\]])/g, | ||
| ': "$1"$2', | ||
| ); |
There was a problem hiding this comment.
🔴 Large-number regex corrupts JSON string values containing 16+ digit numbers
The big-number safety regex at js/token-kit/src/indexer.ts:444-446 (/:\s*(\d{16,})\s*([,}\]])/g) is intended to wrap large unquoted integers as strings before JSON.parse. However, it only checks for : before the digits and does not verify the number is outside a JSON string. A JSON response like {"hash": "1234567890123456789", "amount": 100} would have the hash value (already a string) matched and double-wrapped, corrupting it to {"hash": ""1234567890123456789"", "amount": 100}. This would cause JSON parse errors or silent data corruption for any base58 address or hash that happens to be a 16+ digit all-numeric string inside a JSON string value.
Was this helpful? React with 👍 or 👎 to provide feedback.
- js/compressed-token/src/index.ts: keep both new exports (decompressInterface/decompressMint + createTransferInterfaceInstructions/sliceLast) - scripts/lint.sh: adopt main's subshell pattern, add token-idl and token-sdk linting
| mintContext?: MintContext; | ||
| maxTopUp?: number; | ||
| }): Promise<Instruction> { | ||
| const ctx = await resolveMintContext(params.indexer, params.mint, params.mintContext); |
There was a problem hiding this comment.
🔴 resolveMintContext receives params.mint (token mint address) instead of mint signer address
In buildUpdateMintAuthority, buildUpdateFreezeAuthority, buildUpdateMetadataField, buildUpdateMetadataAuthority, buildRemoveMetadataKey, buildMintToCompressed, buildMintToInterface, and buildDecompressMint, the call to resolveMintContext passes params.mint as the second argument. The resolveMintContext function (js/token-kit/src/actions.ts:874) forwards this value to loadMintContext(indexer, mintSigner) (js/token-kit/src/load.ts:433), which uses it to derive the compressed mint address via deriveCompressedMintAddress(mintSigner). However, params.mint is the token mint address, not the mint signer — these are different keys (the mint PDA is derived from the signer via ["compressed_mint", mintSigner]). When mintContext is not provided by the caller, this will derive the wrong compressed address and fail to find the mint account, or worse, find the wrong account. For example at line 997: resolveMintContext(params.indexer, params.mint, params.mintContext) — the second arg should be a mint signer, but params.mint is documented as Token mint.
Prompt for agents
In js/token-kit/src/actions.ts, all the auto-resolving mint management builders (buildUpdateMintAuthority at line 997, buildUpdateFreezeAuthority at line 1029, buildUpdateMetadataField at line 1064, buildUpdateMetadataAuthority at line 1109, buildRemoveMetadataKey at line 1148, buildMintToCompressed at line 1191, buildMintToInterface at line 1226, buildDecompressMint at line 1269) pass params.mint as the mintSigner argument to resolveMintContext. However, params.mint is the token mint address, not the mint signer. The resolveMintContext function calls loadMintContext which expects a mintSigner to derive the compressed mint address from. Either: (1) rename the parameter from 'mint' to 'mintSigner' in all these builder functions and update the callers, or (2) add a separate 'mintSigner' parameter to these functions that is used when mintContext is not provided. The mint address and mint signer are different keys — the mint PDA is derived from the signer via ['compressed_mint', mintSigner].
Was this helpful? React with 👍 or 👎 to provide feedback.
| const safeText = text.replace( | ||
| /:\s*(\d{16,})\s*([,}\]])/g, | ||
| ': "$1"$2', | ||
| ); |
There was a problem hiding this comment.
🟡 Big-number regex in PhotonIndexer corrupts numbers inside JSON string values that happen to follow a colon
The regex /:\s*(\d{16,})\s*([,}\]])/g at js/token-kit/src/indexer.ts:444-446 is designed to wrap large bare integers as strings before JSON.parse to preserve BigInt precision. However, the regex operates on raw JSON text and does not distinguish between numeric values and digits embedded inside string values. For example, a JSON response containing "dataHash":"12345678901234567890abcdef" where the value starts with 16+ digits would have those initial digits incorrectly wrapped. The regex's lookahead ([,}\]]) partially mitigates this (since string values typically end with "), but edge cases exist. More critically, if a string value is entirely numeric digits (≥16 digits) and happens to appear as "field":"1234567890123456"}, the regex would not match because the " before } prevents the match. However, the actual risk is if the Photon API ever returns bare large numbers in positions the regex doesn't handle (e.g., inside arrays), those would NOT be wrapped, leading to silent precision loss in BigInt() conversion downstream.
Was this helpful? React with 👍 or 👎 to provide feedback.
…on conflict - Remove js/token-idl and js/token-sdk from lint.sh (not tracked in repo) - Remove explicit pnpm version from token-kit workflow (auto-detected from packageManager field in package.json)
File was untracked but imported by 4 codec modules, causing CI build failure.
| for (let i = 0; i < extCount && pos < data.length; i++) { | ||
| // Each extension starts with a discriminant (u16 LE) | ||
| if (pos + 2 > data.length) break; | ||
| const disc = view.getUint16(pos, true); |
There was a problem hiding this comment.
🔴 Extension discriminants encoded as u8 but deserializeCompressedMint reads them as u16
In encodeExtensionInstructionData (js/token-kit/src/codecs/transfer2.ts:395-423), extension discriminants are encoded as a single u8 byte (e.g., new Uint8Array([EXTENSION_DISCRIMINANT.TOKEN_METADATA])). However, in deserializeCompressedMint (js/token-kit/src/codecs/mint-deserialize.ts:116), the extension discriminant is read as a u16 via view.getUint16(pos, true). This size mismatch means the deserializer reads 2 bytes where only 1 was written, causing it to read garbage for the second byte and fail to match any extension discriminant. This breaks the metadataExtensionIndex detection — it will almost always return -1 even when TokenMetadata is present.
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Summary
buildCreateAta,buildCreateAtaIdempotent,buildGetOrCreateAta,buildDecompressInterface@noble/hashes),deserializeCompressedMint,loadMintContext,getMintDecimalsTest plan
pnpm run buildpasses (TypeScript compilation)pnpm run test— 430/430 unit tests passbuildCreateAta,buildCreateAtaIdempotent,buildGetOrCreateAta,buildDecompressInterface) verifieddeserializeCompressedMint,getMintInterface,getAtaInterfacetestedtoken-kit.ymltriggers automatically)