Skip to content

refactor(token-2022/default-account-state): use kit codecs for instruction data - #675

Open
amilz wants to merge 1 commit into
mainfrom
fix/token-2022-default-account-state-kit-codec
Open

refactor(token-2022/default-account-state): use kit codecs for instruction data#675
amilz wants to merge 1 commit into
mainfrom
fix/token-2022-default-account-state-kit-codec

Conversation

@amilz

@amilz amilz commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What

The Pinocchio default-account-state test added in #652 serialized its instruction data with the borsh package. This repo's TS examples standardize on @solana/kit codecs, so swap it out and drop the extra dependency.

Changes

  • tests/test.ts: borsh.serialize(CreateTokenArgsSchema, ...)getStructEncoder([['tokenDecimals', getU8Encoder()]])
  • package.json / pnpm-lock.yaml: remove borsh

Mint account decoding was already using the official @solana-program/token-2022 codecs and is unchanged.

Test

pnpm build-and-test → 1 passing. tsc --noEmit clean.

Note

tokens/token-2022/non-transferable/pinocchio (from #630) has the same borsh usage — left out of this PR, tracked separately.

…ith kit codecs

Replace the borsh serializer in the Pinocchio example's test with a
@solana/kit struct encoder and drop the borsh dependency.
@amilz
amilz requested a review from dev-jodee as a code owner August 6, 2026 16:55
@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR replaces Borsh serialization of a one-byte instruction argument with an equivalent @solana/kit struct encoder and removes the now-unused dependency.

  • Encodes tokenDecimals using getStructEncoder and getU8Encoder.
  • Removes borsh from the package manifest and lockfile.
  • Refreshes LiteSVM platform package metadata in the lockfile.

Confidence Score: 3/5

The code change appears behaviorally sound, but the PR must not merge until its commit is signed and verified.

The new encoder preserves the program's one-byte instruction layout, but the sole commit lacks the repository-required verified signature.

Files Needing Attention: Commit aceb167

Security Review

The sole PR commit is unsigned and therefore does not meet the repository's commit-provenance requirement. How this was verified: Git reports signature status N for commit aceb167e8eaaad29b4ee11ac58a8e8c15392c516.

Important Files Changed

Filename Overview
tokens/token-2022/default-account-state/pinocchio/tests/test.ts Replaces Borsh serialization with an equivalent fixed-width Kit encoder for the program's single-byte decimals argument.
tokens/token-2022/default-account-state/pinocchio/package.json Removes the unused Borsh dependency; the change is carried by an unsigned commit that violates the repository requirement.
tokens/token-2022/default-account-state/pinocchio/pnpm-lock.yaml Removes Borsh lock entries and records LiteSVM platform libc metadata without changing resolved LiteSVM versions.

Reviews (1): Last reviewed commit: "refactor(token-2022/default-account-stat..." | Re-trigger Greptile

Comment thread tokens/token-2022/default-account-state/pinocchio/package.json
@linear

linear Bot commented Aug 6, 2026

Copy link
Copy Markdown

DEV-837

@amilz

amilz commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

@greptileai The unsigned-commit finding is incorrect. Commit `aceb167e8eaaad29b4ee11ac58a8e8c15392c516` is SSH-signed and verified by GitHub:

```
$ gh api repos/solana-foundation/program-examples/commits/aceb167e --jq .commit.verification
{"verified": true, "reason": "valid", ...}
```

The `N` status comes from running `git log --format=%G?` in a checkout with no `gpg.ssh.allowedSignersFile` configured — git errors out on SSH signature verification and falls back to `N`. That reproduces on every commit in this repo, including `main` at `6fe04fee`, which GitHub likewise reports as verified. It's a property of the verifying environment, not of the commit.

On the lockfile: the `libc: [glibc]/[musl]` lines are not incidental noise. They're what pnpm 10.33.0 (this repo's pinned `packageManager`) emits, and they match every other lockfile in the repo — the version added in #652 was generated by an older pnpm and had drifted. Regenerating with `pnpm@10.33.0 install --lockfile-only` produces exactly the committed file.

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Both corrections are valid — acknowledged on both counts.

On the commit signature: you're right that git log --format=%G? returning N is an artifact of the verification environment lacking an allowedSignersFile, not a property of the commit itself. GitHub's verification ("verified": true, "reason": "valid") is the authoritative signal. I'll retract that finding.

On the lockfile: the libc: [glibc]/[musl] lines being pnpm 10.33.0's standard output makes sense — regenerating with the pinned package manager to fix drift from #652 is exactly the right approach. That's correct behavior, not noise.

Both findings were false positives. The PR looks good.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

MarkFeder added a commit to MarkFeder/program-examples that referenced this pull request Aug 6, 2026
- create/mint: derive the mint-authority PDA directly with
  create_program_address (the canonical bump is already known and stored)
  instead of searching with find_program_address
- test: build instruction data with @solana/kit codecs instead of the borsh
  package, and derive the ATA with findAssociatedTokenPda instead of
  hand-rolling the seeds (follows solana-foundation#675)
- test stack: switch to mocha + tsx and @solana/kit ^7 per AGENTS.md / solana-foundation#656
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants