fix(ata): lock down ATA::Transfer recipient contract#103
Conversation
Enforce at the ATA layer that the recipient token holding is already initialized, owned by the same token program as the sender ATA, decodes to a valid `TokenHolding`, and points at the same token definition as the sender. Align the core instruction doc and guest wrapper doc with that contract, and cover the boundary with unit tests (default, foreign-owned, malformed, mismatched-definition recipients, plus the missing-owner-auth and happy paths) and end-to-end integration tests (default and mismatched-definition recipients). Without this, the downstream `token::Transfer` default-recipient `Claim::Authorized` path was reachable through ATA, so integrators had to reverse-engineer recipient semantics from token/runtime internals.
There was a problem hiding this comment.
Pull request overview
This PR tightens the ATA::Transfer recipient contract to require an already-initialized recipient token holding that is (1) owned by the same token program as the sender ATA and (2) references the same token definition, preventing the ATA path from implicitly relying on downstream token::Transfer default-recipient/claim behavior after the LEZ update.
Changes:
- Enforce recipient invariants in
ata_program::transferbefore constructing the chainedtoken::Transfercall. - Align and expand recipient contract documentation in ATA core instruction docs and the guest wrapper docs.
- Add unit + integration coverage for rejected recipient shapes (default/uninitialized, foreign-owned, malformed data, mismatched definition).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
ata/src/transfer.rs |
Adds ATA-layer assertions to require initialized recipient holding, same token program owner, and matching token definition before chaining into token transfer. |
ata/src/tests.rs |
Adds unit tests validating the new recipient contract, including distinct failure modes and the happy-path chained call. |
integration_tests/tests/ata.rs |
Adds end-to-end tests ensuring runtime rejects default recipient and mismatched-definition recipient, and preserves state on failure. |
ata/core/src/lib.rs |
Updates Instruction::Transfer docs to explicitly list recipient invariants. |
ata/methods/guest/src/bin/ata.rs |
Updates guest wrapper doc-comment to match the core/implementation recipient contract. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| recipient.account, | ||
| Account::default(), | ||
| "Recipient token holding must be initialized" | ||
| ); |
There was a problem hiding this comment.
I think this one here is not necessary. If the account isn't initialized, token problem would try to claim and fail because of no signature being available.
Closes #53
Summary
After the LEZ update,
ATA::Transferaccepts a generic recipient token holding account, but the existing contract was only documented as "must be initialized" and not enforced at the ATA layer. The downstreamtoken::Transferstill has default-recipient claim handling viaClaim::Authorized, so callers had to reverse-engineer the boundary from token/runtime semantics.This PR pins the recipient contract at the ATA layer and locks it down with explicit tests.
Decision: enforce "initialized recipient" at the ATA layer
ata_program::transfernow requires the recipient holding to be:Account::default()),TokenHolding,Each boundary fails with a distinct ATA-level assertion message rather than being silently materialized by the downstream
token::Transfer(e.g. via the default-recipientClaim::Authorizedpath) or surfacing as a "Mismatch Token Definition" panic from the token layer.The downstream
token::Transferdefault-recipient logic is left in place — it remains usable from the direct token program surface — but it is no longer reachable via the ATA transfer path, which is the surface this issue is about.Changes
ata/src/transfer.rs: enforce the recipient contract on the ATA-side before constructing the chainedToken::Transfercall.ata/core/src/lib.rs: expand theInstruction::Transferdoc-comment to describe all three recipient invariants (initialized, same token program, same definition).ata/methods/guest/src/bin/ata.rs: align the guest wrapper doc-comment with the core contract.ata/src/tests.rs: add 5 unit tests covering the contract:Token::Transfer,integration_tests/tests/ata.rs: add 2 integration tests covering the end-to-end runtime rejection of:The wire IDL (
artifacts/ata-idl.json) is unchanged — this PR clarifies and enforces the existing instruction surface rather than reshaping it.Why not the broader contract
The other option was to keep ATA delegating to the downstream token layer and let the default-recipient
Claim::Authorizedpath materialize a fresh recipient holding when applicable. That would have required the ATA-level docs to describe a multi-modal recipient behavior driven by token/runtime claim semantics, which is exactly the reverse-engineering hazard #53 flags. The single "initialized recipient only" rule is narrower and makes the failure model explicit at the surface clients interact with.Validation
All commands pass; the ATA IDL is unchanged.