fix(ata): namespace accounts by token program#108
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request hardens the ATA program’s trust boundary by making the downstream token program an explicit caller-provided input and by namespacing ATA derivation with (token_program_id, owner, token_definition), preventing account-metadata-driven dispatch.
Changes:
- Updated ATA instruction/IDL surface to include
token_program_idforCreate,Transfer, andBurn, and incorporated it into ATA seed derivation. - Added ownership/consistency validations in
create,transfer, andburnbefore emitting chained calls to the token program (including stricter idempotent-create occupant checks). - Expanded unit + integration tests to cover foreign owners, malformed/mismatched occupants, and token-program-specific derivation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| integration_tests/tests/ata.rs | Threads token_program_id through integration flows and adds negative-path coverage for foreign-owned definitions/occupants and mismatches. |
| ata/src/create.rs | Accepts explicit token_program_id and validates token definition + idempotent occupant ownership/definition match before returning success. |
| ata/src/transfer.rs | Accepts explicit token_program_id and validates sender ATA ownership before verifying derivation and dispatching chained transfer. |
| ata/src/burn.rs | Accepts explicit token_program_id and validates holder ATA ownership plus holder/definition consistency before dispatching chained burn. |
| ata/core/src/lib.rs | Updates instruction enum, ATA seed derivation to include token_program_id, and extends ATA verification helper accordingly. |
| ata/methods/guest/src/bin/ata.rs | Wires new token_program_id argument through guest entrypoints for create/transfer/burn. |
| ata/src/tests.rs | Updates unit tests for new seed derivation + new ownership/definition mismatch validations; adds “differs by token program” test. |
| artifacts/ata-idl.json | Regenerates IDL to reflect new token_program_id arguments on all three instructions. |
Comments suppressed due to low confidence (1)
ata/methods/guest/src/bin/ata.rs:67
- The
burnguest instruction now takestoken_program_id, but the doc comment above doesn’t mention this new required argument / dispatch constraint. Please update the comment accordingly.
owner: AccountWithMetadata,
holder_ata: AccountWithMetadata,
token_definition: AccountWithMetadata,
token_program_id: ProgramId,
amount: u128,
) -> SpelResult {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ATA accounts are now namespaced by token program, so callers must explicitly pass the token_program_id when invoking ATA::Transfer. BREAKING CHANGE: `Instruction::Transfer`, `Instruction::Burn`, `Instruction::Create` now requires a `token_program_id` field. Any existing call site that omits it will fail to compile. Closes #83
0c39644 to
893775a
Compare
|
This does actually something different than #84 So they will have to know the program IDs they are interested in anyway (either to derive the ATA and verify, or to ask for the ata's program_owner). Anyway, I think adding the token program id to the ATA is a good move. |
Summary
Closes #83.
Replaces #84
This updates ATA instructions to take an explicit
token_program_idand namespaces ATA derivation by(token_program_id, owner, token_definition).The change preserves support for multiple token programs while removing the unsafe behavior where ATA inferred the downstream token program from caller-supplied account metadata.
What Changed
token_program_idtoCreate,Transfer, andBurnATA instructions.token_program_idincompute_ata_seed, so the same owner and token definition can have distinct ATAs for distinct token programs.Createtoken definitions against the expected token program before dispatch.Createso an existing ATA occupant must be a valid token holding owned by the expected token program and matching the requested token definition.Transfersender ATA ownership before dispatching to the token program.Burnholder ATA ownership, token definition ownership, and holder-definition matching before dispatch.Why This Solves #83
Issue #83 is about ATA crossing a trust boundary incorrectly: ATA accepted account metadata as the source of truth for which token program should receive a chained call. That let a malicious token-like program occupy the same ATA-shaped flow and have ATA forward operations to code the caller did not intend to use.
This PR makes token-program selection caller intent, not account state. A caller or integrating program that expects a specific token program now passes that
token_program_idinto ATA. ATA then checks that the relevant token definition and holdings are owned by that expected program before authorizing any chained token call.The PDA namespace also includes the token program. This means multi-token-program support remains possible without collisions:
(TokenProgramA, owner, definition)and(TokenProgramB, owner, definition)derive different associated token accounts.For programs that expect a specific token program, auto-selection becomes deterministic: derive or look up the ATA using the token program configured by that program, then pass the same
token_program_idinto ATA. ATA no longer guesses from attacker-controlled account metadata.Validation
cargo +nightly fmt --all -- --checkRISC0_SKIP_BUILD=1 cargo +1.94.0 clippy -p ata_program --all-targets -- -D warningsRISC0_SKIP_BUILD=1 cargo +1.94.0 clippy -p integration_tests --all-targets -- -D warningsRISC0_DEV_MODE=1 cargo +1.94.0 test -p ata_program --libRISC0_DEV_MODE=1 cargo +1.94.0 test -p integration_tests --test ata./target/debug/idl-gen ata/methods/guest/src/bin/ata.rs > /tmp/ata-idl.json && diff artifacts/ata-idl.json /tmp/ata-idl.jsongit diff --check