add pinocchio nft-minter example - #611
Conversation
Greptile SummaryThis PR adds a Pinocchio port of the
Confidence Score: 5/5The program logic is correct and the Metaplex CPI account orderings align with the current on-chain IDL. Safe to merge. The hand-rolled Metaplex CPIs are wired correctly (account order, discriminators, Borsh layout), Borsh string parsing guards against out-of-bounds reads, and the tests exercise the full Create→Mint flow end-to-end. The only item worth noting is that No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Program as NFT Minter (Pinocchio)
participant SystemProg as System Program
participant TokenProg as SPL Token Program
participant ATAProg as Associated Token Program
participant MetaProg as Token Metadata Program
Note over Client,MetaProg: Instruction 0 – Create
Client->>Program: Create(name, symbol, uri)
Program->>SystemProg: CreateAccount (mint, MINT_SIZE, token_program owner)
SystemProg-->>Program: ok
Program->>TokenProg: "InitializeMint2 (0 decimals, mint_authority=payer)"
TokenProg-->>Program: ok
Program->>MetaProg: CreateMetadataAccountV3 (discriminator 33)
MetaProg-->>Program: metadata PDA created
Program-->>Client: Ok
Note over Client,MetaProg: Instruction 1 – Mint
Client->>Program: Mint
Program->>ATAProg: CreateIdempotent (payer ATA)
ATAProg-->>Program: ok
Program->>TokenProg: "MintTo (amount=1)"
TokenProg-->>Program: ok
Program->>MetaProg: "CreateMasterEditionV3 (discriminator 17, max_supply=Some(1))"
Note right of MetaProg: Transfers mint & freeze authorities to edition PDA
MetaProg-->>Program: edition PDA created
Program-->>Client: Ok
Reviews (7): Last reviewed commit: "nft-minter pinocchio: use official packa..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ); | ||
| const client = context.banksClient; | ||
| const payer = context.payer; | ||
|
|
There was a problem hiding this comment.
readTokenAmount uses Number for u64 arithmetic
buffer.readUInt32LE(68) * 4294967296 multiplies a 32-bit unsigned integer by 2³², which produces values up to ~18.4 quintillion. JavaScript's Number can only represent integers exactly up to Number.MAX_SAFE_INTEGER (2⁵³ − 1 ≈ 9 quadrillion), so any token amount whose high word is non-zero will silently lose precision. For an NFT the amount is always 1 so this is benign here, but readers learning from this example may copy the pattern for fungible tokens and get incorrect assertion results. Using buffer.readBigUInt64LE(64) and comparing against 1n avoids the issue entirely.
There was a problem hiding this comment.
Good catch — fixed in 0c168c4. readTokenAmount now uses Buffer.readBigUInt64LE(64) and the assertion compares against 1n, so the full u64 range is represented exactly.
| invoke( | ||
| &instruction, | ||
| &[ | ||
| metadata_account, |
There was a problem hiding this comment.
read_borsh_string safely guards against reading beyond the input buffer, but name, symbol, and uri are forwarded to the Metaplex CreateMetadataAccountV3 CPI without checking Metaplex's field-length limits (name ≤ 32 chars, symbol ≤ 10 chars, uri ≤ 200 chars). An oversized value will silently pass on-chain parsing and only fail inside the CPI, producing a low-signal error. For a teaching example, an explicit length check before the CPI would make the constraint visible to learners.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Leaving this as-is for consistency. This example is a direct Pinocchio port of the existing tokens/nft-minter/native (and anchor) variants, and neither of those validates name/symbol/uri lengths before the CPI — Metaplex itself enforces the limits (name ≤ 32, symbol ≤ 10, uri ≤ 200) and returns an error. Adding a length guard here would diverge from the canonical examples this one is meant to mirror, so I am keeping the behavior identical across the three variants.
Use Buffer.readBigUInt64LE so the full u64 range is represented exactly, avoiding the silent precision loss of Number arithmetic above 2^53. Addresses review feedback on PR solana-foundation#611.
Use Buffer.readBigUInt64LE so the full u64 range is represented exactly, avoiding the silent precision loss of Number arithmetic above 2^53. Addresses review feedback on PR solana-foundation#611.
0c168c4 to
4581d7c
Compare
Use Buffer.readBigUInt64LE so the full u64 range is represented exactly, avoiding the silent precision loss of Number arithmetic above 2^53. Addresses review feedback on PR solana-foundation#611.
4581d7c to
1f10648
Compare
|
@Perelyn-sama @dev-jodee — rebased onto latest main (picks up the ASM sbpf/Solana pin from #625), CI is now fully green. Ready for review whenever you have a chance 🙏 |
Move the async bankrun setup out of the `describe` callback and into a
`before` hook so Mocha collects the `it` blocks (an async `describe` body
registers tests after the suite is already collected, so nothing ran).
With the test now executing, replace `Rent::try_minimum_balance` with the
integer rent formula: its floating-point exemption-threshold path emits an
opcode the bankrun VM rejects ("unsupported BPF instruction"). Matches the
create-token example.
Apply the kit + litesvm template from the mint-close-authority example: build and sign transactions with @solana/kit and run them on litesvm, dropping @solana/web3.js and solana-bankrun entirely. PDAs (metadata, master edition, ATA) are derived with getProgramDerivedAddress; the Metaplex Token Metadata program is still dumped from mainnet by prepare.mjs and loaded into LiteSVM via addProgramFromFile (litesvm bundles only the SPL programs). - deps: drop @solana/web3.js + solana-bankrun, add litesvm; pin @solana/kit to ^6.10.0 (litesvm's kit major) so there is a single kit in the tree - tsconfig: bump typescript to ^5 and lib to es2022+dom (kit's types), add @types/node; the suite is type-clean under tsc --noEmit
|
Applied the |
Applies dev-jodee's solana-foundation#624 review refinements on top of the kit + litesvm test: - program: compute rent with Rent::get()?.try_minimum_balance(MINT_SIZE)? instead of the integer-math workaround (only needed to dodge the f64 opcode the old bankrun VM rejected; litesvm runs the real syscall). - test: source the token, associated-token and system program ids from the official @solana-program/token and @solana-program/system packages, and read the minted amount with getTokenDecoder().decode(...).amount instead of a raw byte offset. Token Metadata has no official @solana-program client, so its id stays hand-rolled. - tsconfig: moduleResolution bundler for the packages' subpath exports. Verified locally: cargo build-sbf + ts-mocha -> 2 passing, tsc --noEmit and biome clean, frozen-lockfile OK.
|
Applied the same refinements @dev-jodee asked for on #624 (now merged):
|
2 participants
Adds a Pinocchio port of the
tokens/nft-minterexample, alongside the existinganchorandnativeversions.What it does
Two instructions, dispatched by a leading discriminator byte (matching the native
NftMinterInstructionenum):0) — creates a 0-decimal SPL mint and attaches a Metaplex metadata account via a hand-rolledCreateMetadataAccountV3CPI (name, symbol, URI; immutable, no royalties).1) — creates the payer's associated token account (idempotent), mints the single token, then creates the master edition via a hand-rolledCreateMasterEditionV3CPI (max_supply = Some(1)). Creating the master edition hands the mint/freeze authorities to the edition PDA, making it a true non-fungible token.Since there is no typed Pinocchio crate for
mpl-token-metadata, both Metaplex instructions are built by hand (discriminators 33 and 17) and invoked throughpinocchio::cpi::invoke. The mint authority is aliased to the payer (who signs the transaction) to satisfy the CPIs' signer requirements, mirroring the native example.Tests
tests/test.tsruns undersolana-bankrun, loading the program plus the Token Metadata program (dumped from mainnet intotests/fixturesbyprepare.mjs). Two cases:CreateMasterEditionV3CPI succeeded).