Skip to content

feat(nft-staking): add NFT staking example with checkpointed rewards - #727

Open
Harsh-H-Shah wants to merge 3 commits into
solana-foundation:mainfrom
Harsh-H-Shah:feat/nft-staking-example
Open

feat(nft-staking): add NFT staking example with checkpointed rewards#727
Harsh-H-Shah wants to merge 3 commits into
solana-foundation:mainfrom
Harsh-H-Shah:feat/nft-staking-example

Conversation

@Harsh-H-Shah

Copy link
Copy Markdown
Contributor

Closes #726.

Approach was raised in #726 and confirmed with a maintainer before building.

Approach

tokens/nft-staking/anchor — stake an NFT to earn reward tokens over time, and claim them without unstaking.

The NFT never leaves the owner's wallet. Rather than moving it into a vault (which tokens/escrow already teaches), the program takes delegate authority over the owner's token account with approve and then freezes it via Metaplex FreezeDelegatedAccount, reversing with ThawDelegatedAccount + revoke on unstake.

Three accounts (StakeConfig, UserAccount, StakeAccount) and five instructions (initialize_config, initialize_user, stake, claim, unstake). StakeAccount doubles as the SPL delegate for the staked NFT.

What this adds that the repo didn't have

Each verified by grep before writing:

  • Delegate-and-freeze custody — nowhere else in the repo. The only other "delegate" matches are Token-2022's permanent-delegate, a mint-level admin override rather than a revocable per-account approval.
  • Checkpointed accrual — every existing Clock use (token-fundraiser, games/gacha, games/world-cup) is a one-shot deadline check. None settle a value repeatedly over an account's life.
  • emit! events — not used in any example today.

Account validation, authorities, and value movement

  • stake requires nft_token_account.amount == 1, that the caller is under max_stake, and that the NFT's metadata carries a verified collection matching StakeConfig.collection.
  • claim and unstake require stake_account.owner == user.key(); unstake additionally requires the freeze period to have elapsed.
  • Reward mint authority is the config PDA, so rewards can only be minted from claim/unstake — never by the admin directly. The freeze and thaw CPIs are signed by the stake_account PDA, which is also the token account's delegate.
  • Value moves in exactly two places: reward tokens minted to the staker, and the StakeAccount rent returned to the staker on close. The NFT itself never changes owner.

Threat model notes

Two traps this example exists partly to teach, both documented in its README:

  1. Paying for the same time twice. Rewards are a function of elapsed time, so a payout that does not record having paid lets the next call read the same span again. There is no lock to forget on Solana; the defence is that settling and checkpointing happen in one instruction on one account. A subtler variant: only whole days pay out, so snapping the checkpoint to now rather than advancing it by the days actually settled would swallow the part-day remainder, and someone claiming every 23 hours would earn nothing forever.

  2. Freezing an NFT you don't own. approve and the Metaplex freeze both succeed on a zero-balance token account, and anyone can open an ATA for any mint — so without a balance check a caller could open an empty account for a collection NFT and farm rewards from an NFT they never held. stake checks amount == 1 for this reason; deleting that line makes the stake succeed, which the test suite catches.

Differences from the earlier attempts

  • No direct mpl-token-metadata dependency — the freeze/thaw CPIs go through anchor-spl 1.0.2's metadata feature, avoiding the Borsh conflict that broke Added nft_staking example on Anchor (both for points and for tokens) #58.
  • One example, not three near-duplicate variants (Added nft_staking example on Anchor (both for points and for tokens) #58 shipped staking-for-points, staking-for-tokens and pnft-staking; the duplicated Metaplex pins are a plausible cause of that build failure).
  • Rewards are claimable while still staked. In both prior PRs points only settled inside unstake, so a mid-stake claim paid nothing — which is why neither needed a checkpoint.
  • Collection is pinned in StakeConfig at init. NFT Staking Example with Token Metadata program #118 compared metadata against a caller-supplied collection_mint, which is self-consistent for any verified NFT (its PR body notes this was deliberate simplification).
  • ok_or(MissingCollection) instead of .unwrap() on optional collection metadata, split into distinct MissingCollection / UnverifiedCollection / InvalidCollection errors.
  • Checked arithmetic throughout.
  • The large Metaplex accounts in unstake are Boxed — without it the instruction overruns its stack frame at runtime.

Testing

17 LiteSVM cases (anchor test), covering the full lifecycle against the real Token Metadata program loaded from the prepare.mjs fixture, with a real collection and real NFTs rather than mocks. Clock warping drives accrual.

Negative paths covered: not-held, missing/unverified/wrong collection, stake cap, claiming another user's position, claiming before a day elapses, double claim, and unstaking before the freeze period.

Per CONTRIBUTING's "break an assertion once to confirm the test can actually fail", I mutated the program three ways and confirmed each is caught, and that the tests catch distinct bugs:

Mutation Result
Remove the checkpoint advance 4 fail, incl. "Refuses to pay the same day twice"
Snap the checkpoint to now Only the remainder test fails — double-claim still passes
Remove the amount == 1 check Only "Rejects staking an NFT the signer does not actually hold" fails

Also run locally: anchor build --ignore-keys, pnpm exec tsc --noEmit, anchor test --validator legacy, cargo fmt --check, cargo clippy -- -D warnings (0 findings), and prettier --check . repo-wide.

Notes

  • No new dependencies: the crate uses the same anchor-lang/anchor-spl 1.0.2 pins as every other token example, and the test stack matches AGENTS.md (@anchor-lang/core + anchor-litesvm + litesvm 0.8 with pnpm.overrides). The test helper hand-builds the three Metaplex instructions it needs from their wire format, mirroring mpl_util.rs in the native examples, so no mpl dependency is introduced.
  • The crate is registered in .github/.workspace-ignore (it carries its own workspace, like the other anchor token examples).
  • Native and Pinocchio flavors could follow separately if wanted.

AI use

Implementation and tests written with Claude; design, review and verification mine.

Adds tokens/nft-staking/anchor: stake an NFT to earn reward tokens over
time, and claim those rewards without unstaking.

The NFT stays in the owner's wallet. The program takes delegate authority
over the token account via `approve` and freezes it in place through
Metaplex's FreezeDelegatedAccount, reversing it with ThawDelegatedAccount
and `revoke` on unstake. That custody model, and `emit!` events, are not
demonstrated anywhere else in the repo.

Rewards accrue against a `last_claimed_at` checkpoint that each payout
advances by exactly the span it paid for, so the same seconds can never
be claimed twice and a part-day remainder is never forfeited. Both
mistakes have dedicated tests that fail if reintroduced.

Builds on the approach in solana-foundation#58 and solana-foundation#118, which stalled on an
mpl-token-metadata/Borsh build failure and on inactivity. This version
goes through anchor-spl 1.0.2's `metadata` feature with no direct mpl
dependency, ships one example rather than three near-duplicate variants,
and additionally:

- rejects staking an NFT the signer does not hold. Both `approve` and the
  Metaplex freeze succeed on a zero-balance token account, so without
  this anyone could open an empty ATA for a collection NFT and farm
  rewards from an NFT they never owned
- replaces an `.unwrap()` on optional collection metadata with distinct
  MissingCollection / UnverifiedCollection / InvalidCollection errors
- uses checked arithmetic throughout
- boxes the large Metaplex accounts in `unstake`, which otherwise
  overruns the instruction stack frame

Tested with 17 LiteSVM cases covering the full lifecycle against the real
Token Metadata program, using clock warping for accrual. Verified the
suite fails when the checkpoint advance, the remainder handling, or the
balance check is removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The PR is not yet safe to merge because a sufficiently long-lived stake can still overflow reward settlement and leave its NFT frozen.

Findings

  1. P1 Security Anyone Can Seize Configuration
  2. P1 Invalid Rewards Can Lock NFTs
  3. P1 Install Switches Global Cluster
  4. P1 Pools Share Stake Limits
  5. P2 Fixture Failures Are Hidden

Summary

  • Introduces independently addressed staking pools and per-pool user totals.
  • Supports reward claims without unstaking while preserving partial-day accrual.
  • Adds lifecycle and negative-path LiteSVM coverage using the Token Metadata program.
  • The latest changes correctly scope each UserAccount to its pool, resolving cross-pool stake-limit interference.

Reviews (3) · Last reviewed commit: "fix(nft-staking): scope UserAccount to i..."

Comment on lines +10 to +21
pub admin: Signer<'info>,

/// The collection every staked NFT must belong to.
pub collection_mint: Account<'info, Mint>,

#[account(
init,
payer = admin,
space = ANCHOR_DISCRIMINATOR + StakeConfig::INIT_SPACE,
seeds = [b"config"],
bump,
)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Anyone Can Seize Configuration

The sole config PDA can be initialized by any signer. An unrelated caller can initialize it first with their own collection and reward settings, permanently preventing the intended operator from configuring the pool because there is no update, reset, or close path. Restrict initialization to an expected authority or make it atomic with deployment.

How this was verified: The fixed [b"config"] address uses one-time init, while admin is an unrestricted signer and is never checked by another instruction.

Knowledge Base Used: Program-derived address design

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 937fdd7.

StakeConfig is now seeded by its admin ([b"config", admin.key().as_ref()]) rather than living at a single [b"config"] address, so pools are per-operator and there is no slot to grab. This also matches how token-fundraiser scopes its state by maker. The other instructions derive it as [b"config", config.admin.as_ref()], and shared.rs signs with the same seeds as the reward mint authority.

Added a test ("Lets a second admin run their own pool") that initializes a second pool under a different admin and asserts it lands.

Comment on lines +48 to +56
self.config.set_inner(StakeConfig {
admin: self.admin.key(),
collection: self.collection_mint.key(),
points_per_day,
max_stake,
freeze_period_days,
rewards_bump: bumps.rewards_mint,
bump: bumps.config,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Invalid Rewards Can Lock NFTs

These configuration values are stored without checking that payouts fit in u64. For example, a nonzero rate with reward_decimals >= 20, or a sufficiently large points_per_day, makes reward calculation or decimal scaling overflow after a whole day. Both claim and unstake then revert before the NFT is thawed, and the immutable configuration leaves the position permanently stuck. Validate that the scaled daily payout is representable and reject unusable settings such as max_stake == 0.

Knowledge Base Used: Token program workflows

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and this one was the most serious of the four — unstake settles rewards before it thaws, so an overflow makes both claim and unstake revert and the NFT is stranded frozen with no recovery path. Fixed in 937fdd7.

initialize_config now rejects:

  • max_stake == 0 (a pool nobody can stake in)
  • reward_decimals > 9 (the SPL convention)
  • any rate where points_per_day * 10^decimals * MAX_ACCRUAL_DAYS overflows u64, with MAX_ACCRUAL_DAYS = 365 * 100

Bounding a century of accrual rather than a single day covers both multiplications, since amount = days * points_per_day * 10^decimals — a per-day check alone would still let a long-lived position overflow later.

Covered by "Rejects pool settings that would strand a staked NFT", which exercises all three cases; I verified it fails when the checks are removed.

Comment thread tokens/nft-staking/anchor/prepare.mjs Outdated
for (const program of programs) {
const { id, name } = program;
const outputFile = join(outputDir, name);
await $`solana config set -um`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Install Switches Global Cluster

pnpm install runs this script and persistently changes the user's global Solana CLI endpoint to mainnet. Because the previous endpoint is never restored, a later unqualified solana command can unexpectedly operate against mainnet. Pass the mainnet RPC URL directly to the dump command or save and restore the prior configuration.

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!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 937fdd7 — the cluster is passed per command (solana program dump <id> <out> --url <cluster>) instead of mutating global config, so installing this example no longer repoints the machine's default cluster.

Worth flagging for the maintainers: this solana config set -um line is inherited from the existing prepare.mjs files in tokens/nft-operations, tokens/pda-mint-authority and tokens/transfer-tokens, which all have the same side effect. I have only changed it here rather than doing a drive-by sweep across those examples, but it is probably worth the same fix separately if you agree.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that resolves the issue in this PR: the cluster is now scoped to the dump command rather than changing the global Solana CLI configuration. I agree the matching solana config set -um usage in the other three prepare.mjs files should be fixed separately; keeping that cleanup out of this PR avoids unrelated changes.

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

Comment thread tokens/nft-staking/anchor/prepare.mjs Outdated
Comment on lines +23 to +30
try {
await mkdir(outputDir, { recursive: true });
if (overwrite) await rm(outputFile, { force: true });
await $`solana program dump ${id} ${outputFile}`;
console.log(`Program ${id} dumped to ${outputFile}`);
} catch (error) {
console.error(`Error dumping ${id}: ${error.message}`);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Fixture Failures Are Hidden

The script catches solana program dump failures and still exits successfully, although the generated .so is required unconditionally by the tests. If the CLI or RPC is unavailable, installation appears successful and the failure surfaces later when LiteSVM tries to open the missing fixture. Re-throw the error or exit with a nonzero status after logging it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 937fdd7 — the try/catch is gone, so a failed dump now fails the install.

This one actually bit me while preparing the fix for the sibling comment above. The script deletes the fixture before fetching, so when the dump failed the old fixture was already gone, the error was swallowed, pnpm install reported success, and the failure surfaced much later as litesvm refusing to open a missing file — exactly the sequence described. Can confirm the diagnosis first-hand.

CI failure: pnpm-lock.yaml still listed @solana-program/token after the
test switched to @solana/spl-token's AccountLayout, so --frozen-lockfile
rejected the install. Lockfile regenerated.

Review feedback:

- Seed StakeConfig by its admin. A single ["config"] address was a land
  grab: whoever initialized first owned the only pool the program could
  ever have, with no update or close path. Pools are now per-operator,
  which also matches how token-fundraiser scopes its state.

- Validate pool settings at creation. unstake settles rewards before it
  thaws the NFT, so a rate whose scaled payout can overflow u64 would
  make both claim and unstake revert and strand the NFT frozen with no
  recovery. initialize_config now rejects max_stake == 0, reward
  decimals above the SPL convention of 9, and any rate that cannot
  survive MAX_ACCRUAL_DAYS of accrual.

- prepare.mjs no longer runs `solana config set -um`, which persistently
  repointed the machine's default cluster at mainnet as a side effect of
  installing. The cluster is passed per command instead.

- prepare.mjs no longer swallows dump failures. It deletes the fixture
  before fetching, so a failed dump left no fixture and reported success,
  surfacing later as litesvm failing to open a missing file. Hit this
  during development; it now fails the install.

Tests: 19 passing (two new — a second admin running their own pool, and
rejection of settings that would strand a staked NFT). Verified the new
validation test fails when the checks are removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
init,
payer = admin,
space = ANCHOR_DISCRIMINATOR + StakeConfig::INIT_SPACE,
seeds = [b"config", admin.key().as_ref()],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Pools Share Stake Limits

Configs are now independent per admin, but every pool still checks and updates the same UserAccount.amount_staked counter derived only from the user. If a user reaches max_stake in one pool, they are incorrectly prevented from staking in another pool, so the new pools do not enforce their limits independently.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, and this was a regression from the per-admin config change in 937fdd7 — I scoped the pool but left UserAccount on [b"user", user], so the cap and the points total stayed global.

Fixed in 9199d2b: UserAccount is now seeded [b"user", config, user], and initialize_user takes the pool it belongs to. Each pool tracks its own amount_staked and points_earned.

Added "Enforces stake caps per pool, not globally", which stakes into a second admin's pool while the user is already at max_stake in the first, then asserts each pool's count independently. Confirmed it fails against the old global seeds.

Follow-up to the per-admin config change: UserAccount was still seeded
on ["user", user] alone, so the stake cap and points total were shared
across every pool. A user at max_stake in one pool was wrongly blocked
from staking in another, and pools could not enforce their limits
independently — a regression introduced by making configs per-admin.

UserAccount is now seeded ["user", config, user], and initialize_user
takes the pool it belongs to.

New test "Enforces stake caps per pool, not globally" stakes into a
second admin's pool while already at the cap in the first, and asserts
each pool tracks its own count. Verified it fails with the old global
seeds. 20 passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

tokens/nft-staking: propose an NFT staking example (delegate + freeze, checkpointed rewards)

1 participant