feat(amm): route user deposits and LP burns through ATA program#65
feat(amm): route user deposits and LP burns through ATA program#65
Conversation
Add `owner` and `ata_program_id` parameters to `add_liquidity`, `remove_liquidity`, `swap_exact_input`, and `swap_exact_output`. User deposit-side transfers now emit `ATA::Transfer` chained calls instead of `Token::Transfer` directly, and LP burns emit `ATA::Burn` instead of `Token::Burn`. Vault withdrawal chained calls are unchanged. - Add `ata_program_id` field to `AddLiquidity`, `RemoveLiquidity`, `SwapExactInput`, and `SwapExactOutput` instruction variants in `amm_core` - Add `ata_core` dependency to `amm_program` and guest crates - Update guest binary, unit tests, and integration tests to supply the new `owner` account and `ata_program_id` at every call site - Regenerate `artifacts/amm-idl.json` Closes #11
|
Im reviewing this code, I am trying to exploit the lack of trustful relationship between amm and ata transfer. Dont merge it before my final review |
- Introduced a new Cargo.toml for the malicious ATA guest program. - Implemented malicious ATA methods in `malicious_ata.rs`, including create, transfer, and burn functions that simulate malicious behavior. - Updated the integration test suite to include tests for the malicious ATA program, ensuring it is rejected in various value paths. - Enhanced existing tests to accommodate the new malicious ATA program and verify that it does not affect the expected state of the AMM.
|
Thank you @3esmit for laying out the vulnerabilities. Might be worth looking into patterns on how to ensure a program has a program dependency it can trust (maybe via a config account for the program or something like that). |
3esmit
left a comment
3esmit
left a comment
There was a problem hiding this comment.
This can't be merged because it allows an untrusted program to steal from AMM, as AMM trusts the program is doing the actual transfers but can't actually checks they happened due how chainedcalls are designed to be proof compatible.
If this ever would have to be approved, either:
- Force a requirement on verified programids
- Change the chainedcalls to a 2 phase action where AMM verify that transfers acutally occured.
|
Closing this, as we're not moving forward with integrating ATA into AMM. |
2 participants
Add
ownerandata_program_idparameters toadd_liquidity,remove_liquidity,swap_exact_input, andswap_exact_output. User deposit-side transfers now emitATA::Transferchained calls instead ofToken::Transferdirectly, and LP burns emitATA::Burninstead ofToken::Burn. Vault withdrawal chained calls are unchanged.ata_program_idfield toAddLiquidity,RemoveLiquidity,SwapExactInput, andSwapExactOutputinstruction variants inamm_coreata_coredependency toamm_programand guest cratesowneraccount andata_program_idat every call siteartifacts/amm-idl.jsonCloses #11