Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

solana: Fix logic error in Inbox release process #336

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

solana: Fix logic error in Inbox release process #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e5004eb
solana: Fix logic error in Inbox release process
johnsaigle Mar 22, 2024
86f5a74
solana: change comment to revert_on_delay
johnsaigle Mar 22, 2024
5eea9b3
solana: Let redeem code return error instead of try_release
johnsaigle Mar 28, 2024
ef961ad
solana: update arg name in tests
johnsaigle Apr 2, 2024
ed933ac
solana: update IDL
johnsaigle May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 1 addition & 25 deletions solana/idl/json/dummy_transfer_hook.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@
"isMut": false,
"isSigner": false
},
{
"name": "counter",
"isMut": true,
"isSigner": false
},
{
"name": "systemProgram",
"isMut": false,
Expand Down Expand Up @@ -78,11 +73,6 @@
"docs": [
"computes and the on-chain code correctly passes on the PDA."
]
},
{
"name": "counter",
"isMut": true,
"isSigner": false
}
],
"args": [
Expand All @@ -93,21 +83,7 @@
]
}
],
"accounts": [
{
"name": "Counter",
"type": {
"kind": "struct",
"fields": [
{
"name": "count",
"type": "u64"
}
]
}
}
],
"metadata": {
"address": "BgabMDLaxsyB7eGMBt9L22MSk9KMrL4zY2iNe14kyFP5"
"address": "7R368vaW4Ztt8ShPuBRaaCqSTumLCVMbc1na4ajR5y4G"
}
}
2 changes: 1 addition & 1 deletion solana/idl/json/example_native_token_transfers.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@
"kind": "struct",
"fields": [
{
"name": "revertOnDelay",
"name": "revertWhenNotReady",
"type": "bool"
}
]
Expand Down
48 changes: 0 additions & 48 deletions solana/idl/ts/dummy_transfer_hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ export type DummyTransferHook = {
"isMut": false,
"isSigner": false
},
{
"name": "counter",
"isMut": true,
"isSigner": false
},
{
"name": "systemProgram",
"isMut": false,
Expand Down Expand Up @@ -78,11 +73,6 @@ export type DummyTransferHook = {
"docs": [
"computes and the on-chain code correctly passes on the PDA."
]
},
{
"name": "counter",
"isMut": true,
"isSigner": false
}
],
"args": [
Expand All @@ -92,20 +82,6 @@ export type DummyTransferHook = {
}
]
}
],
"accounts": [
{
"name": "counter",
"type": {
"kind": "struct",
"fields": [
{
"name": "count",
"type": "u64"
}
]
}
}
]
};

Expand Down Expand Up @@ -141,11 +117,6 @@ export const IDL: DummyTransferHook = {
"isMut": false,
"isSigner": false
},
{
"name": "counter",
"isMut": true,
"isSigner": false
},
{
"name": "systemProgram",
"isMut": false,
Expand Down Expand Up @@ -189,11 +160,6 @@ export const IDL: DummyTransferHook = {
"docs": [
"computes and the on-chain code correctly passes on the PDA."
]
},
{
"name": "counter",
"isMut": true,
"isSigner": false
}
],
"args": [
Expand All @@ -203,19 +169,5 @@ export const IDL: DummyTransferHook = {
}
]
}
],
"accounts": [
{
"name": "counter",
"type": {
"kind": "struct",
"fields": [
{
"name": "count",
"type": "u64"
}
]
}
}
]
};
4 changes: 2 additions & 2 deletions solana/idl/ts/example_native_token_transfers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1471,7 +1471,7 @@ export type ExampleNativeTokenTransfers = {
"kind": "struct",
"fields": [
{
"name": "revertOnDelay",
"name": "revertWhenNotReady",
"type": "bool"
}
]
Expand Down Expand Up @@ -3353,7 +3353,7 @@ export const IDL: ExampleNativeTokenTransfers = {
"kind": "struct",
"fields": [
{
"name": "revertOnDelay",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure it's worth introducing a breaking change at this point

"name": "revertWhenNotReady",
"type": "bool"
}
]
Expand Down
30 changes: 21 additions & 9 deletions solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub struct ReleaseInbound<'info> {

#[derive(AnchorDeserialize, AnchorSerialize)]
pub struct ReleaseInboundArgs {
pub revert_on_delay: bool,
pub revert_when_not_ready: bool,
}

// Burn/mint
Expand All @@ -55,8 +55,8 @@ pub struct ReleaseInboundMint<'info> {
}

/// Release an inbound transfer and mint the tokens to the recipient.
/// When `revert_on_error` is true, the transaction will revert if the
/// release timestamp has not been reached. When `revert_on_error` is false, the
/// When `revert_when_not_ready` is true, the transaction will revert if the
/// release timestamp has not been reached. When `revert_when_not_ready` is false, the
/// transaction succeeds, but the minting is not performed.
/// Setting this flag to `false` is useful when bundling this instruction
/// together with [`crate::instructions::redeem`] in a transaction, so that the minting
Expand All @@ -70,8 +70,14 @@ pub fn release_inbound_mint(
let released = inbox_item.try_release()?;

if !released {
if args.revert_on_delay {
return Err(NTTError::CantReleaseYet.into());
if args.revert_when_not_ready {
match inbox_item.release_status {
ReleaseStatus::NotApproved => return Err(NTTError::TransferNotApproved.into()),
ReleaseStatus::ReleaseAfter(_) => return Err(NTTError::CantReleaseYet.into()),
// Unreachable: if released, [`InboxItem::try_release`] will return an Error immediately
// rather than Ok(bool).
ReleaseStatus::Released => return Err(NTTError::TransferAlreadyRedeemed.into()),
}
} else {
return Ok(());
}
Expand Down Expand Up @@ -113,8 +119,8 @@ pub struct ReleaseInboundUnlock<'info> {
}

/// Release an inbound transfer and unlock the tokens to the recipient.
/// When `revert_on_error` is true, the transaction will revert if the
/// release timestamp has not been reached. When `revert_on_error` is false, the
/// When `revert_when_not_ready` is true, the transaction will revert if the
/// release timestamp has not been reached. When `revert_when_not_ready` is false, the
/// transaction succeeds, but the unlocking is not performed.
/// Setting this flag to `false` is useful when bundling this instruction
/// together with [`crate::instructions::redeem`], so that the unlocking
Expand All @@ -128,8 +134,14 @@ pub fn release_inbound_unlock(
let released = inbox_item.try_release()?;

if !released {
if args.revert_on_delay {
return Err(NTTError::CantReleaseYet.into());
if args.revert_when_not_ready {
match inbox_item.release_status {
ReleaseStatus::NotApproved => return Err(NTTError::TransferNotApproved.into()),
ReleaseStatus::ReleaseAfter(_) => return Err(NTTError::CantReleaseYet.into()),
// Unreachable: if released, [`InboxItem::try_release`] will return an Error immediately
// rather than Ok(bool).
ReleaseStatus::Released => return Err(NTTError::TransferAlreadyRedeemed.into()),
}
} else {
return Ok(());
}
Expand Down
13 changes: 12 additions & 1 deletion solana/programs/example-native-token-transfers/src/queue/inbox.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,18 @@ impl InboxItem {
pub const SEED_PREFIX: &'static [u8] = b"inbox_item";

/// Attempt to release the transfer.
/// Returns true if the transfer was released, false if it was not yet time to release it.
/// If the inbox item status is [`ReleaseStatus::ReleaseAfter`], this function returns true if the current timestamp
/// is newer than the one stored in the release status. If the timestamp is in the future,
/// returns false.
///
/// # Errors
///
/// Returns errors when the item cannot be released, i.e. when its status is not
/// `ReleaseAfter`:
/// - returns [`NTTError::TransferNotApproved`] if [`ReleaseStatus::NotApproved`]
/// - returns [`NTTError::TransferAlreadyRedeemed`] if [`ReleaseStatus::Released`]. This is
/// important to prevent a single transfer from being redeemed multiple times, which would
/// result in minting arbitrary amounts of the token.
pub fn try_release(&mut self) -> Result<bool> {
let now = current_timestamp();

Expand Down
19 changes: 9 additions & 10 deletions solana/ts/sdk/ntt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ export class NTT {
payer: PublicKey
chain: ChainName | ChainId
nttMessage: NttMessage
revertOnDelay: boolean
revertWhenNotReady: boolean
recipient?: PublicKey
config?: Config
}): Promise<TransactionInstruction> {
Expand All @@ -494,7 +494,7 @@ export class NTT {

return await this.program.methods
.releaseInboundMint({
revertOnDelay: args.revertOnDelay
revertWhenNotReady: args.revertWhenNotReady
})
.accounts({
common: {
Expand All @@ -513,7 +513,7 @@ export class NTT {
payer: Keypair
chain: ChainName | ChainId
nttMessage: NttMessage
revertOnDelay: boolean
revertWhenNotReady: boolean
config?: Config
}): Promise<void> {
if (await this.isPaused()) {
Expand All @@ -536,7 +536,7 @@ export class NTT {
payer: PublicKey
chain: ChainName | ChainId
nttMessage: NttMessage
revertOnDelay: boolean
revertWhenNotReady: boolean
recipient?: PublicKey
config?: Config
}): Promise<TransactionInstruction> {
Expand All @@ -553,7 +553,7 @@ export class NTT {

return await this.program.methods
.releaseInboundUnlock({
revertOnDelay: args.revertOnDelay
revertWhenNotReady: args.revertWhenNotReady
})
.accounts({
common: {
Expand All @@ -573,7 +573,7 @@ export class NTT {
payer: Keypair
chain: ChainName | ChainId
nttMessage: NttMessage
revertOnDelay: boolean
revertWhenNotReady: boolean
config?: Config
}): Promise<void> {
if (await this.isPaused()) {
Expand Down Expand Up @@ -742,7 +742,7 @@ export class NTT {

return await this.program.methods.receiveWormholeMessage().accounts({
payer: args.payer,
config: { config: this.configAccountAddress() },
config: { config: this.configAccountAddress() },
peer: transceiverPeer,
vaa: derivePostedVaaKey(this.wormholeId, Buffer.from(wormholeNTT.hash)),
transceiverMessage: this.transceiverMessageAccountAddress(
Expand Down Expand Up @@ -825,7 +825,7 @@ export class NTT {
// In case the redeemed amount exceeds the remaining inbound rate limit capacity,
// the transaction gets delayed. If this happens, the second instruction will not actually
// be able to release the transfer yet.
// To make sure the transaction still succeeds, we set revertOnDelay to false, which will
// To make sure the transaction still succeeds, we set revertWhenNotReady to false, which will
// just make the second instruction a no-op in case the transfer is delayed.

const tx = new Transaction()
Expand All @@ -838,8 +838,7 @@ export class NTT {
nttMessage,
recipient: new PublicKey(nttMessage.payload.recipientAddress.toUint8Array()),
chain: chainId,
revertOnDelay: false,
config: config
revertWhenNotReady: false
}

if (config.mode.locking != null) {
Expand Down
Loading