feat(solana): IFT #873
feat(solana): IFT #873
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #873 +/- ##
==========================================
+ Coverage 70.82% 71.51% +0.69%
==========================================
Files 83 125 +42
Lines 14176 16219 +2043
==========================================
+ Hits 10040 11599 +1559
- Misses 4136 4620 +484
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ); | ||
|
|
||
| let discriminator = solana_sha256_hasher::hash(discriminator_name); | ||
| let mut ix_data = discriminator.to_bytes()[..8].to_vec(); |
There was a problem hiding this comment.
nit: you can use const for discriminator len
| &[bump], | ||
| ], | ||
| gmp_program, | ||
| ) |
There was a problem hiding this comment.
What about using GMPAccount::pda() from solana-ibc-types/src/ics27.rs?
| /// Whether bridge is active | ||
| pub active: bool, | ||
|
|
||
| pub _reserved: [u8; 64], |
There was a problem hiding this comment.
In all apps we reserve 256 bytes. Is there any particular reason to reduce it for this case?
| /// Transfer initiation timestamp | ||
| pub timestamp: i64, | ||
|
|
||
| pub _reserved: [u8; 32], |
| /// GMP program address for sending cross-chain calls | ||
| pub gmp_program: Pubkey, | ||
|
|
||
| pub _reserved: [u8; 128], |
srdtrk
left a comment
srdtrk
left a comment
There was a problem hiding this comment.
Initial review. I mostly looked at the docs and have some major questions
| GMP forwards `on_timeout_packet` and `on_acknowledgement_packet` to the original sender program when `remaining_accounts` are provided by the relayer. | ||
|
|
||
| The sender program ID is extracted from `GMPPacketData.sender`. GMP constructs the callback using Anchor's discriminator convention (`sha256("global:<instruction>")[..8]`) and invokes the sender program. |
There was a problem hiding this comment.
I thought these callbacks would be done in a separate instruction by the relayer and not in an extension an already existing instruction.
For example, IFT would have an instruction called on_ack_packet callback, and the relayer would be able to call it directly (access controlled so that only a relayer can call it). IFT would verify the validity of the callback via the state stored in GMP?
| Programs sending GMP packets that need callbacks must implement: | ||
|
|
||
| ```rust | ||
| pub fn on_timeout_packet(ctx: Context<...>, msg: OnTimeoutPacketMsg) -> Result<()> | ||
| pub fn on_acknowledgement_packet(ctx: Context<...>, msg: OnAcknowledgementPacketMsg) -> Result<()> | ||
| ``` |
There was a problem hiding this comment.
How does the relayer know these instructions are implemented in general? (a non-IFT sender implementing this?)
| | **Set Authority** | `initialize.rs` | Transfer mint authority to IFT PDA | | ||
| | **Create ATA** | `ift_mint.rs` | Create receiver's token account if needed | | ||
|
|
||
| ### Operations Intentionally Not Used |
There was a problem hiding this comment.
Not sure why this section is needed?
| ## SPL Token Operations | ||
|
|
||
| ### Operations Used | ||
|
|
||
| | Operation | Instruction | Usage | | ||
| |-----------|-------------|-------| | ||
| | **Burn** | `ift_transfer.rs` | Burn tokens when initiating cross-chain transfer | | ||
| | **Mint** | `ift_mint.rs` | Mint tokens to receiver on incoming transfer | | ||
| | **Mint** | `on_ack_packet.rs` | Refund on failed transfer (mint back to sender) | | ||
| | **Mint** | `on_timeout_packet.rs` | Refund on timeout (mint back to sender) | | ||
| | **Set Authority** | `initialize.rs` | Transfer mint authority to IFT PDA | | ||
| | **Create ATA** | `ift_mint.rs` | Create receiver's token account if needed | |
There was a problem hiding this comment.
So a couple questions:
- This contract is not an SPL token, but is meant to work with an existing one, right?
- This contract only supports a single SPL token, right?
| pub mint: Pubkey, | ||
| pub client_id: String, // IBC client (max 64) | ||
| pub counterparty_ift_address: String,// IFT contract on destination (max 128) | ||
| pub counterparty_chain_type: CounterpartyChainType, |
There was a problem hiding this comment.
I think it would be better for this to follow the solidity pattern, where this is a program ID. But I don't feel strongly about this.
|
|
||
| ### Key Security Properties | ||
|
|
||
| 1. **Burn Authorization**: Only token owner can burn (standard SPL token semantics) |
There was a problem hiding this comment.
Anyone calling iftTransfer can burn right?
Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com>
…ty-ibc-eureka into vaporif/solana-ift
Description
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.