Unwrap instruction #13
Conversation
| /// 0. `[writeable]` Escrow of unwrapped tokens, must be owned by: | ||
| /// `get_wrapped_mint_authority(wrapped_mint_address)` | ||
| /// 1. `[writeable]` Recipient unwrapped tokens | ||
| /// 2. `[]` Wrapped mint authority, address must be: | ||
| /// `get_wrapped_mint_authority(wrapped_mint)` | ||
| /// 3. `[]` Unwrapped token mint | ||
| /// 4. `[]` SPL Token program for wrapped mint | ||
| /// 5. `[]` SPL Token program for unwrapped mint | ||
| /// 6. `[writeable]` Wrapped token account to unwrap | ||
| /// 7. `[writeable]` Wrapped mint, address must be: | ||
| /// `get_wrapped_mint_address(unwrapped_mint_address, | ||
| /// wrapped_token_program_id)` | ||
| /// 2. `[writeable]` Escrow of unwrapped tokens, must be owned by: | ||
| /// `get_wrapped_mint_authority(wrapped_mint_address)` | ||
| /// 3. `[writeable]` Recipient unwrapped tokens | ||
| /// 4. `[]` Unwrapped token mint | ||
| /// 5. `[]` SPL Token program for wrapped mint | ||
| /// 6. `[]` SPL Token program for unwrapped mint | ||
| /// 7. `[signer]` Transfer authority on wrapped token account | ||
| /// 8. `..8+M` `[signer]` (Optional) M multisig signers on wrapped token | ||
| /// 8. `[signer]` Transfer authority on wrapped token account | ||
| /// 9. `..8+M` `[signer]` (Optional) M multisig signers on wrapped token |
There was a problem hiding this comment.
Re-arranged in order so that accounts can be sub-sliced into the transfer_checked account slice
| let transfer_authority = next_account_info(account_info_iter)?; | ||
| let multisig_signer_accounts = account_info_iter.as_slice(); | ||
|
|
||
| // Validate accounts |
There was a problem hiding this comment.
I was comparing the checks here to the token-upgrade program, which is very similar to this instruction in particular.
I'm not sure if it's actually needed, but that program does a whole bunch of checks on each account, to make sure they're really token accounts and all that.
Here's some owner checks: https://github.com/solana-labs/solana-program-library/blob/63b217336276c1e41163a5ac030d1b5a85741782/token-upgrade/program/src/processor.rs#L96
And here's some account type checks: https://github.com/solana-labs/solana-program-library/blob/63b217336276c1e41163a5ac030d1b5a85741782/token-upgrade/program/src/processor.rs#L126
I could have been overly cautious in that program though. @buffalojoec do you think we need them?
There was a problem hiding this comment.
I would say no, we don't need account state and owner checks, since everything has to go through an SPL-Token instruction (mint_to, burn, transfer_checked) and the respective token program should have those checks. It's better to leave those checks up to the token program anyway, since this program could theoretically be used with alternate token programs.
Verifying that you're unwrapping to the proper token (and wrapping in the previous instruction) is done by the PDA derivations, so I think we're covered.
| let transfer_authority = next_account_info(account_info_iter)?; | ||
| let multisig_signer_accounts = account_info_iter.as_slice(); | ||
|
|
||
| // Validate accounts |
There was a problem hiding this comment.
I would say no, we don't need account state and owner checks, since everything has to go through an SPL-Token instruction (mint_to, burn, transfer_checked) and the respective token program should have those checks. It's better to leave those checks up to the token program anyway, since this program could theoretically be used with alternate token programs.
Verifying that you're unwrapping to the proper token (and wrapping in the previous instruction) is done by the PDA derivations, so I think we're covered.
program/src/processor.rs
Outdated
| { | ||
| let escrow_data = unwrapped_escrow.try_borrow_data()?; | ||
| let escrow_account = PodStateWithExtensions::<PodAccount>::unpack(&escrow_data)?; | ||
| if escrow_account.base.owner != expected_authority { | ||
| Err(TokenWrapError::EscrowOwnerMismatch)? | ||
| } | ||
| } |
There was a problem hiding this comment.
This is another one of those checks that could be left up to the token program, which would also check the base.mint.
program/tests/test_unwrap.rs
Outdated
| .unwrapped_token_program(TokenProgram::SplToken) | ||
| .wrapped_token_program(TokenProgram::SplToken2022) |
program/tests/test_unwrap.rs
Outdated
| &unwrap_result.wrapped_token_account.account.data, | ||
| ) | ||
| .unwrap(); | ||
| assert_eq!(wrapped_token.base.amount, 0.into()); |
There was a problem hiding this comment.
Shouldn't this be source_starting_amount - unwrap_amount?
There was a problem hiding this comment.
Ah, indeed that would be more accurate. At the moment, it's assuming the source starting amount is the same amount as what you want to unwrap. Will update!
The remaining instruction to complete a full user-flow with wrapping:
The unwrap action burns the wrapped tokens and transfers the equivalent amount from the escrow account to the recipient.