From 18ebdc136b1a9263315b59d479ad107e5669ea69 Mon Sep 17 00:00:00 2001 From: Csongor Kiss Date: Thu, 18 Apr 2024 23:24:14 +0100 Subject: [PATCH] solana: add recovery flow --- .../example-native-token-transfers/Cargo.toml | 6 +- .../src/error.rs | 2 + .../src/instructions/mod.rs | 2 + .../src/instructions/recover.rs | 180 ++++++++++++++++++ .../src/instructions/release_inbound.rs | 5 +- .../example-native-token-transfers/src/lib.rs | 26 +++ solana/tests/example-native-token-transfer.ts | 120 +++++++++++- solana/ts/sdk/ntt.ts | 172 ++++++++++++++--- 8 files changed, 469 insertions(+), 44 deletions(-) create mode 100644 solana/programs/example-native-token-transfers/src/instructions/recover.rs diff --git a/solana/programs/example-native-token-transfers/Cargo.toml b/solana/programs/example-native-token-transfers/Cargo.toml index 9a131c741..c14f40a9e 100644 --- a/solana/programs/example-native-token-transfers/Cargo.toml +++ b/solana/programs/example-native-token-transfers/Cargo.toml @@ -9,15 +9,17 @@ crate-type = ["cdylib", "lib"] name = "example_native_token_transfers" [features] -default = ["mainnet"] +default = ["owner-recovery", "mainnet"] no-entrypoint = [] no-idl = [] no-log-ix-name = [] cpi = ["no-entrypoint"] idl-build = [ "anchor-lang/idl-build", - "anchor-spl/idl-build" + "anchor-spl/idl-build", ] +# whether the owner can recover transactions +owner-recovery = [] # cargo-test-sbf will pass this along test-sbf = [] # networks diff --git a/solana/programs/example-native-token-transfers/src/error.rs b/solana/programs/example-native-token-transfers/src/error.rs index 75fcbff04..e244abc89 100644 --- a/solana/programs/example-native-token-transfers/src/error.rs +++ b/solana/programs/example-native-token-transfers/src/error.rs @@ -51,6 +51,8 @@ pub enum NTTError { OverflowScaledAmount, #[msg("BitmapIndexOutOfBounds")] BitmapIndexOutOfBounds, + #[msg("FeatureNotEnabled")] + FeatureNotEnabled, } impl From for NTTError { diff --git a/solana/programs/example-native-token-transfers/src/instructions/mod.rs b/solana/programs/example-native-token-transfers/src/instructions/mod.rs index ed009b00c..7d0a7426d 100644 --- a/solana/programs/example-native-token-transfers/src/instructions/mod.rs +++ b/solana/programs/example-native-token-transfers/src/instructions/mod.rs @@ -1,6 +1,7 @@ pub mod admin; pub mod initialize; pub mod luts; +pub mod recover; pub mod redeem; pub mod release_inbound; pub mod transfer; @@ -8,6 +9,7 @@ pub mod transfer; pub use admin::*; pub use initialize::*; pub use luts::*; +pub use recover::*; pub use redeem::*; pub use release_inbound::*; pub use transfer::*; diff --git a/solana/programs/example-native-token-transfers/src/instructions/recover.rs b/solana/programs/example-native-token-transfers/src/instructions/recover.rs new file mode 100644 index 000000000..645bc8be1 --- /dev/null +++ b/solana/programs/example-native-token-transfers/src/instructions/recover.rs @@ -0,0 +1,180 @@ +//! This module implements instructions to recover transfers. Only the owner can +//! execute these instructions. +//! +//! Recovery means that the tokens are redeemed, but instead of sending them to +//! the recipient, they are sent to a recovery account. The recovery account is +//! a token account of the appropriate mint. +//! +//! This is useful in case the underlying token implements a blocklisting +//! mechanism (such as OFAC sanctions), and the recipient is blocked, meaning +//! the tokens are irredeemable. +//! +//! In such cases, the owner can recover the transfer by sending them to the +//! recovery address (typically controlled by the owner, though we're not +//! prescriptive about access control of that account). +//! Ideally, it would be nice to attempt to make the transfer to the original +//! recipient, and only allow recovery if that fails. However, solana's runtime does +//! not allow recovering from a failed CPI call, so that is not possible. +//! +//! This feature is opt-in, and hidden behind a feature flag ("owner-recovery"). +//! When that flag is set to false, the instructions in this module will revert. + +use anchor_lang::prelude::*; +use anchor_spl::token_interface; + +use crate::instructions::release_inbound::*; + +#[account] +#[derive(InitSpace)] +pub struct RecoveryAccount { + /// The bump seed for the recovery account + pub bump: u8, + /// The token account that will receive the recovered tokens + pub recovery_address: Pubkey, +} + +impl RecoveryAccount { + pub const SEED: &'static [u8] = b"recovery"; +} + +#[derive(Accounts)] +pub struct InitializeRecoveryAccount<'info> { + #[account(mut)] + pub payer: Signer<'info>, + + pub config: Account<'info, crate::config::Config>, + + #[account( + constraint = owner.key() == config.owner + )] + pub owner: Signer<'info>, + + #[account( + init, + payer = payer, + space = 8 + RecoveryAccount::INIT_SPACE, + seeds = [RecoveryAccount::SEED], + bump, + )] + pub recovery: Account<'info, RecoveryAccount>, + + #[account( + token::mint = config.mint, + )] + pub recovery_account: InterfaceAccount<'info, token_interface::TokenAccount>, + + system_program: Program<'info, System>, +} + +pub fn initialize_recovery_account(ctx: Context) -> Result<()> { + // This is the most important instruction to check the feature flag, as the + // other instructions cannot be called if the [`RecoveryAccount`] is not + // initialized anyway. + ensure_feature_enabled()?; + + ctx.accounts.recovery.set_inner(RecoveryAccount { + bump: ctx.bumps.recovery, + recovery_address: ctx.accounts.recovery_account.key(), + }); + Ok(()) +} + +#[derive(Accounts)] +pub struct UpdateRecoveryAddress<'info> { + pub config: Account<'info, crate::config::Config>, + + #[account( + constraint = owner.key() == config.owner + )] + pub owner: Signer<'info>, + + #[account(mut)] + pub recovery: Account<'info, RecoveryAccount>, + + #[account( + token::mint = config.mint, + )] + pub new_recovery_account: InterfaceAccount<'info, token_interface::TokenAccount>, +} + +pub fn update_recovery_address(ctx: Context) -> Result<()> { + ensure_feature_enabled()?; + + ctx.accounts.recovery.recovery_address = ctx.accounts.new_recovery_account.key(); + Ok(()) +} + +#[derive(Accounts)] +pub struct RecoverMint<'info> { + pub release_inbound_mint: ReleaseInboundMint<'info>, + + #[account( + constraint = owner.key() == release_inbound_mint.common.config.owner, + )] + pub owner: Signer<'info>, + + pub recovery: Account<'info, RecoveryAccount>, + + #[account( + mut, + constraint = recovery_account.key() == recovery.recovery_address, + )] + pub recovery_account: InterfaceAccount<'info, token_interface::TokenAccount>, +} + +pub fn recover_mint<'info>( + ctx: Context<'_, '_, '_, 'info, RecoverMint<'info>>, + args: ReleaseInboundArgs, +) -> Result<()> { + ensure_feature_enabled()?; + + let accounts = &mut ctx.accounts.release_inbound_mint; + accounts.common.recipient = ctx.accounts.recovery_account.clone(); + let ctx = Context { + accounts, + bumps: ctx.bumps.release_inbound_mint, + ..ctx + }; + release_inbound_mint(ctx, args) +} + +#[derive(Accounts)] +pub struct RecoverUnlock<'info> { + pub release_inbound_unlock: ReleaseInboundUnlock<'info>, + + #[account( + constraint = owner.key() == release_inbound_unlock.common.config.owner, + )] + pub owner: Signer<'info>, + + pub recovery: Account<'info, RecoveryAccount>, + + #[account( + mut, + constraint = recovery_account.key() == recovery.recovery_address, + )] + pub recovery_account: InterfaceAccount<'info, token_interface::TokenAccount>, +} + +pub fn recover_unlock<'info>( + ctx: Context<'_, '_, '_, 'info, RecoverUnlock<'info>>, + args: ReleaseInboundArgs, +) -> Result<()> { + ensure_feature_enabled()?; + + let accounts = &mut ctx.accounts.release_inbound_unlock; + accounts.common.recipient = ctx.accounts.recovery_account.clone(); + let ctx = Context { + accounts, + bumps: ctx.bumps.release_inbound_unlock, + ..ctx + }; + release_inbound_unlock(ctx, args) +} + +fn ensure_feature_enabled() -> Result<()> { + #[cfg(not(feature = "owner-recovery"))] + return Err(crate::error::NTTError::FeatureNotEnabled.into()); + #[cfg(feature = "owner-recovery")] + return Ok(()); +} diff --git a/solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs b/solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs index 7fe5394e8..fffa9f6c9 100644 --- a/solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs +++ b/solana/programs/example-native-token-transfers/src/instructions/release_inbound.rs @@ -62,7 +62,7 @@ pub struct ReleaseInboundMint<'info> { #[account( constraint = common.config.mode == Mode::Burning @ NTTError::InvalidMode, )] - common: ReleaseInbound<'info>, + pub common: ReleaseInbound<'info>, } /// Release an inbound transfer and mint the tokens to the recipient. @@ -126,10 +126,11 @@ pub fn release_inbound_mint<'info>( #[derive(Accounts)] pub struct ReleaseInboundUnlock<'info> { + /// CHECK: the token program checks if this indeed the right authority for the mint #[account( constraint = common.config.mode == Mode::Locking @ NTTError::InvalidMode, )] - common: ReleaseInbound<'info>, + pub common: ReleaseInbound<'info>, } /// Release an inbound transfer and unlock the tokens to the recipient. diff --git a/solana/programs/example-native-token-transfers/src/lib.rs b/solana/programs/example-native-token-transfers/src/lib.rs index 6521acf1e..dfdfa60d6 100644 --- a/solana/programs/example-native-token-transfers/src/lib.rs +++ b/solana/programs/example-native-token-transfers/src/lib.rs @@ -1,3 +1,5 @@ +#![feature(type_changing_struct_update)] + use anchor_lang::prelude::*; // TODO: is there a more elegant way of checking that these 3 features are mutually exclusive? @@ -77,6 +79,16 @@ pub mod example_native_token_transfers { instructions::initialize_lut(ctx, recent_slot) } + /// Initialize the recovery account. + /// The recovery flow + pub fn initialize_recovery_account(ctx: Context) -> Result<()> { + instructions::initialize_recovery_account(ctx) + } + + pub fn update_recovery_address(ctx: Context) -> Result<()> { + instructions::update_recovery_address(ctx) + } + pub fn version(_ctx: Context) -> Result { Ok(VERSION.to_string()) } @@ -113,6 +125,20 @@ pub mod example_native_token_transfers { instructions::release_inbound_unlock(ctx, args) } + pub fn recover_unlock<'info>( + ctx: Context<'_, '_, '_, 'info, RecoverUnlock<'info>>, + args: ReleaseInboundArgs, + ) -> Result<()> { + instructions::recover_unlock(ctx, args) + } + + pub fn recover_mint<'info>( + ctx: Context<'_, '_, '_, 'info, RecoverMint<'info>>, + args: ReleaseInboundArgs, + ) -> Result<()> { + instructions::recover_mint(ctx, args) + } + pub fn transfer_ownership(ctx: Context) -> Result<()> { instructions::transfer_ownership(ctx) } diff --git a/solana/tests/example-native-token-transfer.ts b/solana/tests/example-native-token-transfer.ts index 4431a5af3..ef4d16768 100644 --- a/solana/tests/example-native-token-transfer.ts +++ b/solana/tests/example-native-token-transfer.ts @@ -50,6 +50,7 @@ describe("example-native-token-transfers", () => { }); const user = anchor.web3.Keypair.generate(); let tokenAccount: anchor.web3.PublicKey; + const recoveryTokenAccount = anchor.web3.Keypair.generate(); const mint = anchor.web3.Keypair.generate(); @@ -113,6 +114,16 @@ describe("example-native-token-transfers", () => { spl.ASSOCIATED_TOKEN_PROGRAM_ID ); + await spl.createAccount( + connection, + payer, + mint.publicKey, + payer.publicKey, + recoveryTokenAccount, + undefined, + spl.TOKEN_2022_PROGRAM_ID + ); + await spl.mintTo( connection, payer, @@ -154,6 +165,14 @@ describe("example-native-token-transfers", () => { }); describe("Burning", () => { + const guardians = new MockGuardians(0, [GUARDIAN_KEY]); + + const emitter = new MockEmitter( + Buffer.from("transceiver".padStart(32, "\0")).toString("hex"), + toChainId("ethereum"), + Number(0) // sequence + ); + before(async () => { await spl.setAuthority( connection, @@ -242,7 +261,7 @@ describe("example-native-token-transfers", () => { messageData.message.payload ); - // assert theat amount is what we expect + // assert that amount is what we expect expect( transceiverMessage.nttManagerPayload.payload.trimmedAmount ).to.deep.equal({ amount: 10000n, decimals: 8 }); @@ -254,14 +273,6 @@ describe("example-native-token-transfers", () => { }); it("Can receive tokens", async () => { - const emitter = new MockEmitter( - Buffer.from("transceiver".padStart(32, "\0")).toString("hex"), - toChainId("ethereum"), - Number(0) // sequence - ); - - const guardians = new MockGuardians(0, [GUARDIAN_KEY]); - const sendingTransceiverMessage: WormholeTransceiverMessage< typeof nttMessageLayout > = { @@ -276,7 +287,7 @@ describe("example-native-token-transfers", () => { sender: new UniversalAddress("FACE".padStart(64, "0")), payload: { trimmedAmount: { - amount: 10000n, + amount: 5000n, decimals: 8, }, sourceToken: new UniversalAddress("FAFA".padStart(64, "0")), @@ -311,5 +322,94 @@ describe("example-native-token-transfers", () => { expect((await counterValue()).toString()).to.be.eq("2") }); + + describe("Recovery", () => { + it("Can initialize recovery account", async () => { + await ntt.initializeRecoveryAccount({ + payer, + owner: payer, + recoveryTokenAccount: tokenAccount, + }); + + const recoveryAccount = await ntt.getRecoveryAccount(); + + expect(recoveryAccount?.toBase58()).to.equal(tokenAccount.toBase58()); + }); + + it("Can update recovery account", async () => { + await ntt.updateRecoveryAddress({ + // payer, + owner: payer, + newRecoveryAccount: recoveryTokenAccount.publicKey, + }); + + const recoveryAccount = await ntt.getRecoveryAccount(); + + expect(recoveryAccount?.toBase58()).to.equal( + recoveryTokenAccount.publicKey.toBase58() + ); + }); + + it("Owner can recover transfers", async () => { + const sendingTransceiverMessage: WormholeTransceiverMessage< + typeof nttMessageLayout + > = { + sourceNttManager: new UniversalAddress( + encoding.bytes.encode("nttManager".padStart(32, "\0")) + ), + recipientNttManager: new UniversalAddress( + ntt.program.programId.toBytes() + ), + nttManagerPayload: { + id: encoding.bytes.encode("sequence2".padEnd(32, "0")), + sender: new UniversalAddress("FACE".padStart(64, "0")), + payload: { + trimmedAmount: { + amount: 5000n, + decimals: 8, + }, + sourceToken: new UniversalAddress("FAFA".padStart(64, "0")), + recipientAddress: new UniversalAddress(user.publicKey.toBytes()), + recipientChain: "Solana", + }, + }, + transceiverPayload: { forSpecializedRelayer: false }, + } as const; + + const serialized = serializePayload( + "Ntt:WormholeTransfer", + sendingTransceiverMessage + ); + + const published = emitter.publishMessage( + 0, // nonce + Buffer.from(serialized), + 0 // consistency level + ); + + const vaaBuf = guardians.addSignatures(published, [0]); + + await postVaa(connection, payer, vaaBuf, ntt.wormholeId); + + const released = await ntt.redeem({ + payer, + vaa: vaaBuf, + recover: payer, + }); + + expect(released).to.equal(true); + + const account = await spl.getAccount( + connection, + recoveryTokenAccount.publicKey, + undefined, + spl.TOKEN_2022_PROGRAM_ID + ); + + expect(account.amount).to.equal(BigInt(50000)); + + expect((await counterValue()).toString()).to.be.eq("3") + }); + }); }); }); diff --git a/solana/ts/sdk/ntt.ts b/solana/ts/sdk/ntt.ts index 9c0a650da..f2d40f50b 100644 --- a/solana/ts/sdk/ntt.ts +++ b/solana/ts/sdk/ntt.ts @@ -118,6 +118,10 @@ export class NTT { return this.derivePda('lut_authority') } + recoveryAccountAddress(): PublicKey { + return this.derivePda('recovery') + } + outboxRateLimitAccountAddress(): PublicKey { return this.derivePda('outbox_rate_limit') } @@ -352,6 +356,48 @@ export class NTT { return this.getAddressLookupTable(false) } + async initializeRecoveryAccount(args: { + payer: Keypair + owner: Keypair + recoveryTokenAccount: PublicKey + }) { + const ix: TransactionInstruction = await this.program.methods + .initializeRecoveryAccount() + .accountsStrict({ + payer: args.payer.publicKey, + config: this.configAccountAddress(), + owner: args.owner.publicKey, + recovery: this.recoveryAccountAddress(), + recoveryAccount: args.recoveryTokenAccount, + systemProgram: SystemProgram.programId, + }) + .instruction(); + + const signers = [args.payer, args.owner]; + + await this.sendAndConfirmTransaction(new Transaction().add(ix), signers); + } + + async updateRecoveryAddress(args: { + owner: Keypair + newRecoveryAccount: PublicKey + }) { + const ix: TransactionInstruction = await this.program.methods + .updateRecoveryAddress() + .accountsStrict({ + // payer: args.payer.publicKey, + config: this.configAccountAddress(), + owner: args.owner.publicKey, + recovery: this.recoveryAccountAddress(), + newRecoveryAccount: args.newRecoveryAccount, + }) + .instruction(); + + const signers = [args.owner]; + + await this.sendAndConfirmTransaction(new Transaction().add(ix), signers); + } + async transfer(args: { payer: Keypair from: PublicKey @@ -481,6 +527,7 @@ export class NTT { payer: args.payer, config: { config: this.configAccountAddress() }, mint, + tokenProgram: await this.tokenProgram(config), from: args.from, tokenProgram: await this.tokenProgram(config), outboxItem: args.outboxItem, @@ -674,6 +721,7 @@ export class NTT { revertOnDelay: boolean recipient?: PublicKey config?: Config + recover?: Keypair }): Promise { const config = await this.getConfig(args.config) @@ -686,23 +734,45 @@ export class NTT { const mint = await this.mintAccountAddress(config) - const transferIx = await this.program.methods - .releaseInboundMint({ + let accounts = { + common: { + payer: args.payer, + config: { config: this.configAccountAddress() }, + inboxItem: this.inboxItemAccountAddress(args.chain, args.nttMessage), + recipient: getAssociatedTokenAddressSync(mint, recipientAddress, true, config.tokenProgram), + mint, + tokenAuthority: this.tokenAuthorityAddress(), + tokenProgram: config.tokenProgram, + custody: await this.custodyAccountAddress(config) + } + } + + var transferIx: TransactionInstruction; + + if (args.recover) { + const recoveryAccount = await this.getRecoveryAccount() + if (!recoveryAccount) { + throw new Error('Recovery account not initialized') + } + transferIx = await this.program.methods + .recoverMint({ revertOnDelay: args.revertOnDelay }) .accountsStrict({ - common: { - payer: args.payer, - config: { config: this.configAccountAddress() }, - inboxItem: this.inboxItemAccountAddress(args.chain, args.nttMessage), - recipient: getAssociatedTokenAddressSync(mint, recipientAddress, true, config.tokenProgram), - mint, - tokenAuthority: this.tokenAuthorityAddress(), - tokenProgram: config.tokenProgram, - custody: await this.custodyAccountAddress(config) - } + releaseInboundMint: accounts, + owner: args.recover.publicKey, + recovery: this.recoveryAccountAddress(), + recoveryAccount }) .instruction() + } else { + transferIx = await this.program.methods + .releaseInboundMint({ + revertOnDelay: args.revertOnDelay + }) + .accountsStrict(accounts) + .instruction() + } const mintInfo = await splToken.getMint(this.program.provider.connection, config.mint, undefined, config.tokenProgram) const transferHook = splToken.getTransferHook(mintInfo) @@ -738,6 +808,7 @@ export class NTT { nttMessage: NttMessage revertOnDelay: boolean config?: Config + recover?: Keypair }): Promise { if (await this.isPaused()) { throw new Error('Contract is paused') @@ -752,6 +823,9 @@ export class NTT { tx.add(await this.createReleaseInboundMintInstruction(txArgs)) const signers = [args.payer] + if (args.recover) { + signers.push(args.recover) + } await this.sendAndConfirmTransaction(tx, signers) } @@ -762,6 +836,7 @@ export class NTT { revertOnDelay: boolean recipient?: PublicKey config?: Config + recover?: Keypair }): Promise { const config = await this.getConfig(args.config) @@ -774,23 +849,47 @@ export class NTT { const mint = await this.mintAccountAddress(config) - const transferIx = await this.program.methods - .releaseInboundUnlock({ - revertOnDelay: args.revertOnDelay - }) - .accountsStrict({ - common: { - payer: args.payer, - config: { config: this.configAccountAddress() }, - inboxItem: this.inboxItemAccountAddress(args.chain, args.nttMessage), - recipient: getAssociatedTokenAddressSync(mint, recipientAddress, true, config.tokenProgram), - mint, - tokenAuthority: this.tokenAuthorityAddress(), - tokenProgram: config.tokenProgram, - custody: await this.custodyAccountAddress(config) - }, - }) - .instruction() + let accounts = { + common: { + payer: args.payer, + config: { config: this.configAccountAddress() }, + inboxItem: this.inboxItemAccountAddress(args.chain, args.nttMessage), + recipient: getAssociatedTokenAddressSync(mint, recipientAddress, true, config.tokenProgram), + mint, + tokenAuthority: this.tokenAuthorityAddress(), + tokenProgram: config.tokenProgram, + custody: await this.custodyAccountAddress(config) + }, + }; + + var transferIx: TransactionInstruction; + + if (args.recover) { + const recoveryAccount = await this.getRecoveryAccount() + if (!recoveryAccount) { + throw new Error('Recovery account not initialized') + } + transferIx = + await this.program.methods + .recoverUnlock({ + revertOnDelay: args.revertOnDelay + }) + .accountsStrict({ + releaseInboundUnlock: accounts, + owner: args.recover.publicKey, + recovery: this.recoveryAccountAddress(), + recoveryAccount + }) + .instruction() + } else { + transferIx = + await this.program.methods + .releaseInboundUnlock({ + revertOnDelay: args.revertOnDelay + }) + .accountsStrict(accounts) + .instruction() + } const mintInfo = await splToken.getMint(this.program.provider.connection, config.mint, undefined, config.tokenProgram) const transferHook = splToken.getTransferHook(mintInfo) @@ -826,6 +925,7 @@ export class NTT { nttMessage: NttMessage revertOnDelay: boolean config?: Config + recover?: Keypair }): Promise { if (await this.isPaused()) { throw new Error('Contract is paused') @@ -840,6 +940,9 @@ export class NTT { tx.add(await this.createReleaseInboundUnlockInstruction(txArgs)) const signers = [args.payer] + if (args.recover) { + signers.push(args.recover) + } await this.sendAndConfirmTransaction(tx, signers) } @@ -1051,6 +1154,7 @@ export class NTT { payer: Keypair vaa: SignedVaa config?: Config + recover?: Keypair // owner keypair if recovering }): Promise { const config = await this.getConfig(args.config) @@ -1090,7 +1194,7 @@ export class NTT { recipient: new PublicKey(nttMessage.payload.recipientAddress.toUint8Array()), chain: chainId, revertOnDelay: false, - config: config + config } if (config.mode.locking != null) { @@ -1100,6 +1204,9 @@ export class NTT { } const signers = [args.payer] + if (args.recover) { + signers.push(args.recover) + } await this.sendAndConfirmTransaction(tx, signers) // Let's check if the transfer was released @@ -1121,6 +1228,11 @@ export class NTT { return config ?? await this.program.account.config.fetch(this.configAccountAddress()) } + async getRecoveryAccount(): Promise { + const account = await this.program.account.recoveryAccount.fetchNullable(this.recoveryAccountAddress()) + return account?.recoveryAddress + } + async isPaused(config?: Config): Promise { return (await this.getConfig(config)).paused }