Add Pinocchio version of token-fundraiser example#613
Conversation
Adds a Pinocchio implementation of tokens/token-fundraiser, which previously only had an Anchor version, advancing the repo's goal of having every example available in every framework. - initialize, contribute, check_contributions and refund instructions - mirrors the Anchor reference logic: 10% per-contributor cap, minimum target scaled by the mint's decimals, PDA-signed vault transfers, and account closing on settle / refund - follows the existing escrow and transfer-tokens Pinocchio conventions (manual borsh-compatible (de)serialization, bumps passed in the instruction data, solana-bankrun tests) - registers the program in the workspace Cargo.toml and links it from the root README - solana-bankrun test suite covering the full lifecycle Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR adds a complete Pinocchio implementation of the
Confidence Score: 5/5The implementation is safe to merge; all security-critical checks (canonical vault verification, time gates, per-contributor cap) are correctly in place. The four instruction handlers are correct and well-defended: vault canonicality is enforced on every instruction, time gates are correctly oriented for both contribute (reject when expired) and refund (reject when still active), and the per-contributor 10% cap is applied both per-transaction and cumulatively. The only gaps are non-blocking: contributions are not blocked once the target is already met (same as Anchor), and the bankrun suite is missing a successful end-to-end refund test. The bankrun test file (tests/test.ts) would benefit from a successful refund scenario; contribute.rs silently accepts over-contributions once the target is reached. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Maker
participant Contributor
participant Program
participant TokenProgram
participant VaultATA
participant FundraiserPDA
participant ContributorPDA
Maker->>Program: initialize(amount, duration, bump)
Program->>FundraiserPDA: CreateAccount (system CPI)
Program->>VaultATA: CreateIdempotent (ATA CPI)
Program->>FundraiserPDA: write state (maker, mint, vault, amount_to_raise, duration)
Contributor->>Program: contribute(amount, contributor_bump)
Program->>FundraiserPDA: "verify vault == fundraiser_state.vault"
Program->>Program: "check elapsed_days < duration"
Program->>Program: "check amount <= 10% of target"
Program->>ContributorPDA: CreateAccount if first contribution
Program->>TokenProgram: Transfer(contributor_ata to vault)
Program->>FundraiserPDA: update current_amount
Program->>ContributorPDA: update contributor amount
alt Target met
Maker->>Program: check_contributions()
Program->>Program: "assert vault_amount >= amount_to_raise"
Program->>TokenProgram: Transfer(vault to maker_ata) PDA-signed
Program->>TokenProgram: CloseAccount(vault to maker) PDA-signed
Program->>FundraiserPDA: close lamports to maker
else Campaign expired, target not met
Contributor->>Program: refund(contributor_bump)
Program->>Program: "assert elapsed_days >= duration"
Program->>Program: "assert vault_amount < amount_to_raise"
Program->>TokenProgram: Transfer(vault to contributor_ata) PDA-signed
Program->>FundraiserPDA: update current_amount
Program->>ContributorPDA: close lamports to contributor
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Maker
participant Contributor
participant Program
participant TokenProgram
participant VaultATA
participant FundraiserPDA
participant ContributorPDA
Maker->>Program: initialize(amount, duration, bump)
Program->>FundraiserPDA: CreateAccount (system CPI)
Program->>VaultATA: CreateIdempotent (ATA CPI)
Program->>FundraiserPDA: write state (maker, mint, vault, amount_to_raise, duration)
Contributor->>Program: contribute(amount, contributor_bump)
Program->>FundraiserPDA: "verify vault == fundraiser_state.vault"
Program->>Program: "check elapsed_days < duration"
Program->>Program: "check amount <= 10% of target"
Program->>ContributorPDA: CreateAccount if first contribution
Program->>TokenProgram: Transfer(contributor_ata to vault)
Program->>FundraiserPDA: update current_amount
Program->>ContributorPDA: update contributor amount
alt Target met
Maker->>Program: check_contributions()
Program->>Program: "assert vault_amount >= amount_to_raise"
Program->>TokenProgram: Transfer(vault to maker_ata) PDA-signed
Program->>TokenProgram: CloseAccount(vault to maker) PDA-signed
Program->>FundraiserPDA: close lamports to maker
else Campaign expired, target not met
Contributor->>Program: refund(contributor_bump)
Program->>Program: "assert elapsed_days >= duration"
Program->>Program: "assert vault_amount < amount_to_raise"
Program->>TokenProgram: Transfer(vault to contributor_ata) PDA-signed
Program->>FundraiserPDA: update current_amount
Program->>ContributorPDA: close lamports to contributor
end
Reviews (5): Last reviewed commit: "Verify the vault is the canonical ATA re..." | Re-trigger Greptile |
| export const expectRevert = async (promise: Promise<unknown>) => { | ||
| try { | ||
| await promise; | ||
| throw new Error("Expected a revert"); | ||
| } catch { | ||
| return; | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
expectRevert silently swallows its own sentinel error
The catch {} block uses optional-catch-binding syntax, which catches every thrown value — including the throw new Error("Expected a revert") on line 17 that fires when the promise resolves successfully. If the transaction never reverts, the helper still returns undefined, so both "Rejects a contribution above the per-contributor maximum" and "Rejects settling before the target is met" will always pass regardless of program behavior.
Fix: rethrow the sentinel so a non-reverting transaction actually fails the test:
} catch (e) {
if (e instanceof Error && e.message === "Expected a revert") throw e;
}| space: Contributor::LEN as u64, | ||
| owner: program_id, | ||
| } | ||
| .invoke_signed(&signers)?; | ||
|
|
||
| 0 | ||
| } else { | ||
| Contributor::deserialize(&contributor_account.try_borrow()?)?.amount | ||
| }; | ||
|
|
||
| // The contributor's total contribution cannot exceed the per-contributor max. | ||
| if previous_amount > max || previous_amount + amount > max { | ||
| return Err(FundraiserError::MaximumContributionsReached.into()); | ||
| } | ||
|
|
||
| // Move the tokens into the vault. | ||
| log!("Transferring contribution into vault"); | ||
| Transfer { | ||
| from: contributor_ata, |
There was a problem hiding this comment.
Vault not verified as the canonical ATA — contributions can be redirected
The vault account passed in the instruction is never checked against the expected ATA (derived from fundraiser + mint_to_raise). The Transfer CPI uses authority: contributor, not authority: fundraiser, so the token program will accept any valid token account for the same mint as the destination. A malicious contributor can pass their own ATA as vault, receive their own tokens back, yet have fundraiser_state.current_amount incremented. This inflates the recorded total without actually funding the vault, leaving legitimate contributors unable to refund (the transfer in refund draws from the real vault which is short by the attacker's recorded amount).
Resolves the issues flagged in review: - contribute / check_contributions / refund now verify that the provided vault is a token account owned by the fundraiser PDA for the raised mint (new InvalidVault error). Previously contribute accepted any token account as the vault; since the transfer authority is the contributor, a contributor could redirect tokens to an account they control while still inflating current_amount and later drain the real vault on refund. - fix the expectRevert test helper, which used optional-catch-binding and swallowed its own sentinel error, making the negative-path tests vacuous. - add a regression test asserting a contribution with a substituted vault is rejected. Verified end-to-end on a local solana-test-validator (7/7), including the vault-substitution rejection and a negative control proving expectRevert is no longer vacuous. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the review — both points were valid and are now fixed in e321f19:
Also added a regression test that a contribution with a substituted vault is rejected. Verified end-to-end on a local |
| // Contributions are only accepted within the campaign window. | ||
| let elapsed_days = | ||
| ((Clock::get()?.unix_timestamp - fundraiser_state.time_started) / SECONDS_TO_DAYS) as u16; | ||
| if fundraiser_state.duration > elapsed_days { | ||
| return Err(FundraiserError::FundraiserEnded.into()); | ||
| } |
There was a problem hiding this comment.
The time-gate condition is inverted.
duration > elapsed_days is true while the campaign is still running, so the program rejects contributions during the active window and permits them only after the campaign has expired — the opposite of the stated intent. Because the tests fix duration = 0, every contribution trivially passes the check regardless of the bug, so nothing in the test suite catches this. Any non-zero-duration fundraiser created with this program will reject all contributions.
| // Contributions are only accepted within the campaign window. | |
| let elapsed_days = | |
| ((Clock::get()?.unix_timestamp - fundraiser_state.time_started) / SECONDS_TO_DAYS) as u16; | |
| if fundraiser_state.duration > elapsed_days { | |
| return Err(FundraiserError::FundraiserEnded.into()); | |
| } | |
| // Contributions are only accepted within the campaign window. | |
| let elapsed_days = | |
| ((Clock::get()?.unix_timestamp - fundraiser_state.time_started) / SECONDS_TO_DAYS) as u16; | |
| if fundraiser_state.duration <= elapsed_days { | |
| return Err(FundraiserError::FundraiserEnded.into()); | |
| } |
| // Refunds are only allowed once the campaign has ended. | ||
| let elapsed_days = | ||
| ((Clock::get()?.unix_timestamp - fundraiser_state.time_started) / SECONDS_TO_DAYS) as u16; | ||
| if fundraiser_state.duration < elapsed_days { | ||
| return Err(FundraiserError::FundraiserNotEnded.into()); | ||
| } |
There was a problem hiding this comment.
The time-gate condition is inverted.
duration < elapsed_days is true when the campaign window has already expired, so the program blocks refunds after the fundraiser ends and permits them while it is still active — the opposite of the stated intent. The tests use duration = 0, so this is never exercised and remains undetected.
| // Refunds are only allowed once the campaign has ended. | |
| let elapsed_days = | |
| ((Clock::get()?.unix_timestamp - fundraiser_state.time_started) / SECONDS_TO_DAYS) as u16; | |
| if fundraiser_state.duration < elapsed_days { | |
| return Err(FundraiserError::FundraiserNotEnded.into()); | |
| } | |
| // Refunds are only allowed once the campaign has ended. | |
| let elapsed_days = | |
| ((Clock::get()?.unix_timestamp - fundraiser_state.time_started) / SECONDS_TO_DAYS) as u16; | |
| if fundraiser_state.duration >= elapsed_days { | |
| return Err(FundraiserError::FundraiserNotEnded.into()); | |
| } |
The Anchor reference this example is ported from gates contributions and refunds with inverted comparisons: contributions were only accepted once the campaign window had already closed, and refunds were only allowed while the campaign was still open. The bug is masked in the Anchor tests because they use duration = 0, where both inverted conditions happen to evaluate correctly. This corrects both gates: - contribute now rejects once `elapsed_days >= duration` (campaign ended) - refund now rejects while `elapsed_days < duration` (campaign still open) Tests now use a non-zero (open) campaign for the happy path and a second duration = 0 (already-ended) campaign to exercise both sides of each gate without needing to advance the validator clock. Verified on a local solana-test-validator (8/8): contributions accepted while open, rejected after the campaign ends; refunds rejected while open; plus the vault-ownership, per-contributor-cap and target guards. Note: the Anchor (and native) versions of this example carry the same inverted conditions and would benefit from the same fix in a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Good catch on the time gates — confirmed and fixed in c6e82bb. Both
As you noted, the bug was invisible because every test used Verified end-to-end on a local Heads-up: the Anchor and native versions of |
Addresses the rent-accounting note from review: check_contributions now closes the (drained) vault token account in addition to the fundraiser account, returning both rents to the maker, instead of leaving the vault open with its rent locked. This mirrors the escrow example's take_offer. Adds a happy-path test that funds a campaign to its target with 10 contributors (each at the 10% per-contributor cap), settles it, and asserts the maker receives the full amount and both the vault and the fundraiser account are closed. Verified on a local solana-test-validator (10 contributors -> target -> settle): maker receives 30_000_000 and both accounts are closed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Added the rent-reclaim polish from the review in d812420:
Verified end-to-end on a local On the other note — closing the fundraiser account on |
| if vault_account.owner() != fundraiser.address() | ||
| || vault_account.mint() != mint_to_raise.address() | ||
| { | ||
| return Err(FundraiserError::InvalidVault.into()); | ||
| } | ||
| vault_account.amount() | ||
| }; | ||
|
|
There was a problem hiding this comment.
Vault not verified as canonical ATA — maker can strand contributor tokens
The vault is only checked to be owned by the fundraiser PDA and to hold the correct mint, but is never verified against the canonical ATA derived from (fundraiser, mint_to_raise). A malicious maker can create an arbitrary token account (valid owner, valid mint), pre-fund it to meet the target, and call check_contributions with that account as vault. The program transfers those maker-owned tokens back to the maker, closes the fake vault, then closes the fundraiser account. Contributors' tokens deposited into the real canonical ATA are permanently stranded — the fundraiser state is gone so refund is impossible.
The fix is to derive the expected ATA on-chain and assert that vault.address() equals it, the same way initialize implicitly enforces this via CreateIdempotent.
Review flagged that check_contributions validated the vault only by owner and mint, not that it is the fundraiser's canonical associated token account. A malicious maker could create a token account they own-assign to the fundraiser PDA, pre-fund it to the target, settle against it (recovering their own funding), and close the fundraiser — permanently stranding the real contributors' tokens in the canonical vault. Fix: record the canonical vault address in the fundraiser state at initialization (where the associated token program guarantees the vault is the canonical ATA), and have contribute, check_contributions and refund verify the passed vault matches it. This subsumes the previous owner+mint check. Verified on a local solana-test-validator: settling against a fake fundraiser-owned vault is rejected and the real fundraiser stays open, while a legitimate fully-funded campaign settles and closes both the vault and the fundraiser. Adds a regression test for the fake-vault settle attempt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Good catch — fixed in 05e4109. You're right that an owner+mint check on the vault is insufficient: a maker could create a token account, assign its owner to the fundraiser PDA, pre-fund it to the target, settle against it, and close the fundraiser — stranding the real contributions. Fix: the canonical vault address is now recorded in the fundraiser state at Verified on a local
Added a regression test ( |
Adds a Pinocchio implementation of
tokens/token-fundraiser, which previously only had an Anchor version — advancing the repo's goal of having every example available in every framework (Anchor, Pinocchio, Native).What's included
initialize,contribute,check_contributionsandrefundinstructionsescrow/transfer-tokensPinocchio conventions (manual borsh-compatible (de)serialization, bumps passed in the instruction data,solana-bankruntests)Cargo.tomland links it from the rootREADMETesting
solana-test-validator(initialize → 2 contributions → over-cap rejection → settle-before-target rejection → refund with account close): 5/5 passing.solana-bankrunsuite follows the same structure as the existing Pinocchio token examples.Heads-up for maintainers (not specific to this PR)
With the currently pinned
pinocchio 0.10.2(whose deps require the Solana 4.0 build toolchain) plussolana-bankrun 0.4.0, running the bankrun suites locally traps withunsupported BPF instructiononce a program actually executes. In CI the Pinocchio token suites currently report0 passing(thedescribe(async …)tests don't register before mocha finishes), so this stays hidden. Happy to help with a follow-up (e.g. migrating these suites tolitesvm) if that'd be useful.