From 02160b8d408a862dbe43f17829e5985e8985c02f Mon Sep 17 00:00:00 2001 From: Csongor Kiss Date: Fri, 13 Sep 2024 16:20:56 +0100 Subject: [PATCH] solana: add instruction to transfer ownership in one step (#516) --- .../src/instructions/admin.rs | 30 ++++++++++- .../example-native-token-transfers/src/lib.rs | 4 ++ .../tests/governance.rs | 50 +++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/solana/programs/example-native-token-transfers/src/instructions/admin.rs b/solana/programs/example-native-token-transfers/src/instructions/admin.rs index 93db1de34..76e49eaef 100644 --- a/solana/programs/example-native-token-transfers/src/instructions/admin.rs +++ b/solana/programs/example-native-token-transfers/src/instructions/admin.rs @@ -15,13 +15,19 @@ use crate::{ // * Transfer ownership -/// Transferring the ownership is a 2-step process. The first step is to set the +/// For safety reasons, transferring ownership is a 2-step process. The first step is to set the /// new owner, and the second step is for the new owner to claim the ownership. /// This is to prevent a situation where the ownership is transferred to an /// address that is not able to claim the ownership (by mistake). /// /// The transfer can be cancelled by the existing owner invoking the [`claim_ownership`] /// instruction. +/// +/// Alternatively, the ownership can be transferred in a single step by calling the +/// [`transfer_ownership_one_step_unchecked`] instruction. This can be dangerous because if the new owner +/// cannot actually sign transactions (due to setting the wrong address), the program will be +/// permanently locked. If the intention is to transfer ownership to a program using this instruction, +/// take extra care to ensure that the owner is a PDA, not the program address itself. #[derive(Accounts)] pub struct TransferOwnership<'info> { #[account( @@ -73,6 +79,28 @@ pub fn transfer_ownership(ctx: Context) -> Result<()> { ) } +pub fn transfer_ownership_one_step_unchecked(ctx: Context) -> Result<()> { + ctx.accounts.config.pending_owner = None; + ctx.accounts.config.owner = ctx.accounts.new_owner.key(); + + // NOTE: unlike in `transfer_ownership`, we use the unchecked version of the + // `set_upgrade_authority` instruction here. The checked version requires + // the new owner to be a signer, which is what we want to avoid here. + bpf_loader_upgradeable::set_upgrade_authority( + CpiContext::new( + ctx.accounts + .bpf_loader_upgradeable_program + .to_account_info(), + bpf_loader_upgradeable::SetUpgradeAuthority { + program_data: ctx.accounts.program_data.to_account_info(), + current_authority: ctx.accounts.owner.to_account_info(), + new_authority: Some(ctx.accounts.new_owner.to_account_info()), + }, + ), + &crate::ID, + ) +} + // * Claim ownership #[derive(Accounts)] diff --git a/solana/programs/example-native-token-transfers/src/lib.rs b/solana/programs/example-native-token-transfers/src/lib.rs index 693a890be..a9a4e514c 100644 --- a/solana/programs/example-native-token-transfers/src/lib.rs +++ b/solana/programs/example-native-token-transfers/src/lib.rs @@ -117,6 +117,10 @@ pub mod example_native_token_transfers { instructions::transfer_ownership(ctx) } + pub fn transfer_ownership_one_step_unchecked(ctx: Context) -> Result<()> { + instructions::transfer_ownership_one_step_unchecked(ctx) + } + pub fn claim_ownership(ctx: Context) -> Result<()> { instructions::claim_ownership(ctx) } diff --git a/solana/programs/example-native-token-transfers/tests/governance.rs b/solana/programs/example-native-token-transfers/tests/governance.rs index 141b6f390..cc744c4de 100644 --- a/solana/programs/example-native-token-transfers/tests/governance.rs +++ b/solana/programs/example-native-token-transfers/tests/governance.rs @@ -102,6 +102,9 @@ async fn test_governance() { data: inner_ix_data.data(), }; + let config_account: Config = ctx.get_account_data_anchor(test_data.ntt.config()).await; + assert!(!config_account.paused); // make sure not paused before + wrap_governance( &mut ctx, &test_data.governance, @@ -131,6 +134,53 @@ async fn test_governance() { assert!(config_account.paused); } +#[tokio::test] +async fn test_governance_one_step_transfer() { + let (mut ctx, test_data) = setup(Mode::Locking).await; + + let governance_pda = test_data.governance.governance(); + + // step 1. transfer ownership to governance (1 step) + let ix = example_native_token_transfers::instruction::TransferOwnershipOneStepUnchecked; + + let accs = example_native_token_transfers::accounts::TransferOwnership { + config: test_data.ntt.config(), + owner: test_data.program_owner.pubkey(), + new_owner: governance_pda, + upgrade_lock: test_data.ntt.upgrade_lock(), + program_data: test_data.ntt.program_data(), + bpf_loader_upgradeable_program: bpf_loader_upgradeable::id(), + }; + + Instruction { + program_id: test_data.ntt.program, + accounts: accs.to_account_metas(None), + data: ix.data(), + } + .submit_with_signers(&[&test_data.program_owner], &mut ctx) + .await + .unwrap(); + + let config_account: Config = ctx.get_account_data_anchor(test_data.ntt.config()).await; + assert!(!config_account.paused); // make sure not paused before + + // step 2. set paused + wrap_governance( + &mut ctx, + &test_data.governance, + &test_data.ntt.wormhole, + set_paused(&test_data.ntt, SetPaused { owner: OWNER }, true), + None, + None, + None, + ) + .await + .unwrap(); + + let config_account: Config = ctx.get_account_data_anchor(test_data.ntt.config()).await; + assert!(config_account.paused); +} + #[tokio::test] async fn test_governance_bad_emitter() { let (mut ctx, test_data) = setup(Mode::Locking).await;