Skip to content

feat(token-fundraiser): add pinocchio example - #708

Open
MarkFeder wants to merge 4 commits into
solana-foundation:mainfrom
MarkFeder:tokens-token-fundraiser-pinocchio-revive
Open

feat(token-fundraiser): add pinocchio example#708
MarkFeder wants to merge 4 commits into
solana-foundation:mainfrom
MarkFeder:tokens-token-fundraiser-pinocchio-revive

Conversation

@MarkFeder

Copy link
Copy Markdown
Contributor

What

Adds a Pinocchio implementation of the token-fundraiser example (the anchor example has no native sibling). A maker starts a fundraiser with a token target and a duration; contributors deposit into a PDA-owned vault up to a per-contributor cap while it runs. If the target is met the maker releases the funds; if the fundraiser ends without meeting it, contributors can refund.

This revives a previously-deferred port — its only blocker was that bankrun couldn't execute Clock::get(). The example now uses litesvm, which runs the Clock, so the time-based logic works and is testable.

Instructions

Four instructions over two PDA state accounts (fundraiser, contributor):

  1. initialize — create the fundraiser PDA + its vault ATA; record the target, duration, and start time.
  2. contribute — validate the per-contributor cap and that the fundraiser is still running; create the contributor record on first contribution; transfer tokens into the vault.
  3. check_contributions — once the target is met, release the vault to the maker (PDA-signed) and close the vault + fundraiser.
  4. refund — after the fundraiser ends without meeting the target, return a contributor's deposit (PDA-signed) and close their record.

Uses pinocchio-token / pinocchio-associated-token-account for the vault and transfers, invoke_signed for the PDA-authorized transfers, and the Clock sysvar for the time logic. The two inverted time checks in the anchor example are corrected here (contributions only while running; refunds only after the end).

Test

litesvm + @solana/kit, driving the full lifecycle by controlling the clock:

  • Refund path — contribute, warp the clock past the deadline, refund; assert the contributor is repaid and their account closed.
  • Release path — ten contributors reach the target (the 10% cap requires ten), then the maker releases the funds; assert the maker receives them and the fundraiser is closed.
Token Fundraiser (Pinocchio)
  ✔ Refunds a contributor after the fundraiser ends without meeting its target
  ✔ Releases the funds to the maker once the target is met
2 passing

Verified locally: cargo build-sbf, the litesvm tests, tsc --noEmit, Prettier, cargo fmt --check, Clippy, and pnpm install --frozen-lockfile all clean. (Deploy uses the *.so glob per #702.)


AI use: I directed the design (the instruction set, PDA/vault layout, the corrected time logic, the multi-scenario clock-driven test) and verified the token/PDA CPI patterns against the merged escrow pinocchio example; implementation and tests were written with Claude Code and reviewed by me.

Ports the token-fundraiser example to Pinocchio (the anchor example has no
native sibling). A maker starts a fundraiser with a token target and a
duration; contributors deposit tokens into a PDA-owned vault up to a
per-contributor cap while the fundraiser runs. Once the target is met the
maker releases the funds; if the fundraiser ends without meeting the target,
contributors can refund their deposits.

Four instructions (initialize, contribute, check_contributions, refund) over
two PDA state accounts, using pinocchio-token / pinocchio-associated-token-
account for the vault and transfers, PDA-signed CPIs to move funds out of the
vault, and the Clock sysvar for the time-based logic. The two inverted time
checks in the anchor example are corrected here (contributions are only
accepted while running; refunds only after the fundraiser ends).

The litesvm test drives the full lifecycle: the refund path (contribute, warp
the clock past the deadline, refund) and the release path (ten contributors
reach the target, then the maker releases the funds), controlling the clock to
exercise the time branches.
@MarkFeder
MarkFeder requested a review from dev-jodee as a code owner August 30, 2026 09:40
@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The follow-up changes complete the requested fundraiser hardening without leaving a blocking failure from the prior review.

  • Derives and validates the canonical signer-scoped contributor PDA before creating or loading its record.
  • Uses recorded campaign contributions rather than the externally mutable vault balance to determine release and refund eligibility.
  • Adds regression coverage for substituted contributor records, noncanonical records, and direct vault transfers.

Confidence Score: 5/5

The PR appears safe to merge because the previously reported blocking failures are fixed and no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename Overview
tokens/token-fundraiser/pinocchio/program/src/instructions/contribute.rs Canonical contributor PDA validation now precedes both existing-account and account-creation paths, closing the previously reported identity and bump bypasses.
tokens/token-fundraiser/pinocchio/program/src/instructions/check_contributions.rs Successful settlement now requires recorded contributions to reach the target while still sweeping the complete vault balance.
tokens/token-fundraiser/pinocchio/program/src/instructions/refund.rs Refund eligibility now depends on recorded campaign contributions, so unrecorded vault transfers cannot block contributors.
tokens/token-fundraiser/pinocchio/tests/test.ts Regression tests cover substituted and noncanonical contributor records plus direct vault deposits across release and refund behavior.

Reviews (4): Last reviewed commit: "token-fundraiser: apply prettier formatt..." | Re-trigger Greptile

Comment on lines +77 to +79
// Create the per-contributor record on first contribution; otherwise load it.
let already_contributed = if contributor_account.owner() == program_id {
Contributor::deserialize(&contributor_account.try_borrow()?)?.amount

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 Contributor PDA binding is skipped

When a contributor supplies another participant's existing program-owned record, this branch trusts that record without deriving it from the signing contributor. The signer’s tokens are then credited to the substituted record, allowing the per-contributor cap to be bypassed and the record’s actual owner to claim those tokens through the refund path.

How this was verified: The existing-record branch skips the signer-scoped PDA comparison while the transfer debits the signer and serialization credits the supplied record.

Knowledge Base Used:

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 c76d7161. The contributor PDA derivation now runs before the exists/create branch, so an existing record is bound to the signer too. Added a test where an attacker contributes against another contributor's record (amounts kept under the cap so only the PDA check can reject it) — it is now rejected with InvalidSeeds, and fails against the previous program.

Comment on lines +62 to +65
let vault_amount = TokenAccount::from_account_view(vault)?.amount();
if vault_amount < state.amount_to_raise {
return Err(FundraiserError::TargetNotMet.into());
}

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 Vault balance bypasses contribution accounting

When any token holder transfers directly into the public vault ATA, vault_amount increases without updating current_amount or a contributor record. That unrecorded deposit can make this check release the fundraiser and make the refund path reject legitimate contributors even though accepted contributions remain below the target.

How this was verified: Direct SPL transfers can increase the standard vault ATA independently, while both release and refund eligibility read its balance instead of the recorded campaign total.

Suggested change
let vault_amount = TokenAccount::from_account_view(vault)?.amount();
if vault_amount < state.amount_to_raise {
return Err(FundraiserError::TargetNotMet.into());
}
let vault_amount = TokenAccount::from_account_view(vault)?.amount();
if state.current_amount < state.amount_to_raise {
return Err(FundraiserError::TargetNotMet.into());
}

Knowledge Base Used:

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 c76d7161. Both check_contributions and refund now gate on state.current_amount instead of the vault balance, so an unrecorded direct transfer into the vault can neither release the fundraiser nor block a legitimate refund. The release still drains the full vault balance, so stray deposits are not stranded. Added a test that transfers straight into the vault to reach the target: release is rejected and the contributor can still refund. It fails against the previous program.

…ded totals

contribute skipped the contributor PDA derivation whenever the supplied
record was already program-owned, so a signer could credit their transfer
to another participant's record: it bypassed the per-contributor cap and
left the tokens claimable by that record's owner through refund. The
derivation now runs before the branch, for existing and new records alike.

check_contributions and refund gated on the vault's token balance. The
vault is a standard ATA, so any holder could transfer straight into it and
push that balance past the target, releasing the fundraiser and blocking
legitimate refunds without any recorded contribution. Both now read
current_amount; the release still drains the full vault balance.

Adds LiteSVM coverage for both: a contribution into a substituted record is
rejected, and a direct vault transfer neither releases the fundraiser nor
prevents a refund. Both tests fail against the previous program.
Comment on lines +84 to +93
Seed::from(&bump_bytes),
];
let contributor_pda = Address::create_program_address(
&[Contributor::SEED_PREFIX, fundraiser.address().as_ref(), contributor.address().as_ref(), &bump_bytes],
program_id,
)
.map_err(|_| ProgramError::InvalidSeeds)?;
if contributor_account.address() != &contributor_pda {
return Err(ProgramError::InvalidSeeds);
}

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 Noncanonical PDAs bypass contributor cap

When a contributor supplies a noncanonical bump that still produces a valid PDA, this check accepts a separate record and enforces the cap only against that record. The signer can therefore exceed the per-contributor cap across multiple bump-derived records, and the canonical-only refund path rejects those records, leaving their contributions unclaimable.

How this was verified: The caller controls the accepted bump, while refund derives and accepts only the canonical contributor PDA.

Knowledge Base Used:

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 f90ef59a. The bump is now derived on-chain with find_program_address, and the supplied byte is gone from the instruction data entirely, so there is no caller-controlled input to the derivation left to attack. That also brings it in line with the Anchor version, whose seeds = [b"contributor", ...], bump constraint derives canonically rather than accepting a bump from the client — passing one was my divergence from the reference.

Added a test that a contributor_account which is not the canonical PDA is refused before any record is created. 5 tests passing.

Comment on lines +65 to +67
if state.current_amount < state.amount_to_raise {
return Err(FundraiserError::TargetNotMet.into());
}

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 Direct deposits become permanently stranded

When tokens are transferred directly into the vault and an expired campaign refunds all recorded contributions, this recorded-total gate prevents release while refund returns only recorded amounts and closes only contributor records. The remaining tokens stay locked in the vault, and neither the vault nor fundraiser rent can be recovered because no failed-campaign cleanup instruction remains reachable.

Knowledge Base Used:

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.

Accurate, but I am leaving this one as-is, and I want to be explicit about the trade rather than quietly not fixing it.

The stranding is real only on the failed-campaign path. On the successful path check_contributions still transfers vault_amount — the full balance, not current_amount — and then closes the vault, so stray deposits are swept to the maker rather than stranded.

On the failed path, the gap you describe is inherited from the example, not introduced here. The Anchor version this ports has the same four instructions (initialize, contribute, checker, refund) and no teardown for a campaign that expires under target: even with zero stray deposits, once every contributor refunds, the vault and fundraiser accounts are left behind with their rent unrecovered. Adding a fifth instruction to close them would make this no longer a faithful port of the reference.

The alternative — going back to gating on the vault balance — is exactly the P1 you flagged in the sibling thread: anyone can transfer into a standard ATA, so it would let a stranger force-release a campaign that never met its target, and block legitimate refunds. Recovering rent for stray deposits is not worth reopening that.

Worth noting the Anchor example gates on self.vault.amount >= amount_to_raise today and so still has the original issue. If maintainers want failed-campaign teardown, that seems like a follow-up applied to both implementations together rather than a divergence introduced here — happy to open an issue for it.

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.

That tradeoff is clear, and I agree this should not block the PR. The successful path sweeps the full vault balance before closing it; the failed-path cleanup gap is inherited from the Anchor reference, while gating on vault balance would reintroduce the more serious unauthorized-release/refund issue. I’ll treat this as an acknowledged follow-up for both implementations rather than a required change here.

contribute took the contributor bump from instruction data and fed it to
create_program_address. Several bumps can yield a valid address for the same
seeds, so a contributor could open extra, non-canonical records for
themselves and be metered against the per-contributor cap separately on each.
Those records were also unrefundable, since refund only ever derives the
canonical address.

The bump is now derived on-chain with find_program_address and the supplied
byte is gone from the instruction data entirely, so there is no longer a
caller-controlled input to the derivation. This matches the Anchor version,
whose `seeds = [...], bump` constraint likewise derives canonically rather
than accepting a bump from the client.

Adds a test that a contributor_account which is not the canonical PDA is
refused before any record is created.
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.

1 participant