fix: withdraw spl without receiver ata #65
Conversation
skosito
requested review from
brewmaster012,
lumtis and
ws4charlie
as code owners
skosito
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65 +/- ##
========================================
+ Coverage 6.53% 7.32% +0.79%
========================================
Files 1 1
Lines 398 355 -43
========================================
Hits 26 26
+ Misses 372 329 -43 ☔ View full report in Codecov by Sentry. |
| @@ -367,73 +364,30 @@ pub mod gateway { | |||
| Errors::SPLAtaAndMintAddressMismatch, | |||
| ); | |||
|
|
|||
| // test whether the recipient_ata is created or not; if not, create it | |||
| // test whether the recipient_ata is created or not; if not log and exit successfully | |||
There was a problem hiding this comment.
Can we explain why we log and exit successfully instead of making the tx fails
There was a problem hiding this comment.
@lumtis the core issue here is the possibility of a "legitimate authorized failed withdraw tx" (for a lack of better word) which means the withdraw cannot be accomplished and we tried, as opposed to other "illegitimate unauthorized failed withdraw" which may include: wrong/fake TSS signature, replay using a wrong nonce).
For "authorized failed withdraw tx", zetaclient should report as "revert" like in Ethereum case so the solana nonce on zetacore will increment and we move on. However if the Solana tx actually failed/reverted, the program nonce will not increment. In this case we need to allow the solana tx to succeed, but without transfering SPL tokens and indicate in logs that it "reverted".
For "unauthorized failed withdraw", the correct behavior is that zetaclients would ignore such tx as they are not authorized. The solana program simply fail them.
There was a problem hiding this comment.
I see, I think still make sense to precise this in the comment
|
|
||
| let rentPayerPdaBal1 = await conn.getBalance(rentPayerPda); | ||
| expect(rentPayerPdaBal0-rentPayerPdaBal1).to.be.eq(to_ata_bal); // rentPayer pays rent | ||
| expect(to_ata_bal).to.be.eq(0); // still 0 balance |
There was a problem hiding this comment.
perhaps verify the log here
zetaclient will need to the same
| msg!( | ||
| "Refunding the rent paid by the signer {:?}", | ||
| ctx.accounts.signer.to_account_info().key | ||
| msg!("recipient ATA account does not exist"); |
There was a problem hiding this comment.
thinking about how zetaclient should parse and validate this--
grep the log?
can it be tricked? (for example, the user included two intructions in the same tx, one is from their attack program which emits the same log above, and the second instruction is the withdraw spl that actually points to a valid ata address so transfer actually succeeds)
|
solved in: #67 |
Remove rent payer logic with withdraw spl and add log but still succeed tx so it can be parsed on zetaclient