solana-labs / solana

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://solanalabs.com
Apache License 2.0
13.21k stars 4.29k forks source link

docs: Add warning in exchange integration guide about possibility of inaccessible tokens #19156

Closed jstarry closed 1 year ago

jstarry commented 3 years ago

Problem

When an associated token account (1) is created for an account (2) which is also an associated token account, there is no way to withdraw tokens held by the account (1) because the owner account (2) cannot sign to approve a transfer because it is a PDA for the ATA program. Tokens can only be transferred if the ATA program supports transfers itself, which it currently does not.

Proposed Solution

Add warning to exchange guide and the SPL wallet guide to check whether a recipient address is already a token account before attempting to create a new ATA for the recipient.

mvines commented 3 years ago

What if ATA required the provided wallet address be a system account: https://github.com/solana-labs/solana-program-library/blob/7c89855f0fe473466a6fdc0a2274d40db647b517/associated-token-account/program/src/lib.rs#L62

It was never the intent that programs could own an ATA account anyway, and I think this simple change could remove this footgun entirely

jstarry commented 3 years ago

Very much in favour of the system account requirement. Protocols can always create a system PDA to manage tokens if needed. Of course, there could be some programs which rely on creating ATA accounts for non system accounts, so I'm in favour of a new instruction for ATA creation which has this check and is also idempotent

jstarry commented 3 years ago

To prevent this issue from occurring in the future here is a proposal to add a new instruction with fewer foot guns: https://github.com/solana-labs/solana-program-library/issues/2249

Arrowana commented 3 years ago

What if ATA required the provided wallet address be a system account: https://github.com/solana-labs/solana-program-library/blob/7c89855f0fe473466a6fdc0a2274d40db647b517/associated-token-account/program/src/lib.rs#L62

It was never the intent that programs could own an ATA account anyway, and I think this simple change could remove this footgun entirely

This doesn't remove the footgun entirely for other scenarios, i saw at least 2 times user who closed their AUX then sent to the AUX address they had saved in their exchange.

What exchange have to do is: Only accept a system program address from the start, never an ATA, never an AUX token account, to set the culture of working with a wallet address (system program address) rather than token accounts directly. There is no point doing so. On top of that flush all those addresses that can potentially be AUX that could be closed.