From c5e56b064a72272d1ba99a5b40c91df6d2e039f4 Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Sun, 16 Oct 2022 23:26:10 -0400 Subject: [PATCH 01/15] SetAuthorityChecked --- programs/bpf_loader/src/lib.rs | 337 ++++++++++++++---- sdk/program/src/bpf_loader_upgradeable.rs | 27 +- .../src/loader_upgradeable_instruction.rs | 13 + transaction-status/src/parse_bpf_loader.rs | 11 + 4 files changed, 325 insertions(+), 63 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 253f260deed3da..bc463af475efd7 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -495,6 +495,82 @@ fn process_instruction_common( } } +fn process_set_authority( + instruction_context: &InstructionContext, + transaction_context: &TransactionContext, + log_collector: &Option>>, + new_authority_must_sign: bool, +) -> Result<(), InstructionError> { + instruction_context.check_number_of_instruction_accounts(2)?; + let mut account = instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let present_authority_key = transaction_context.get_key_of_account_at_index( + instruction_context.get_index_of_instruction_account_in_transaction(1)?, + )?; + let new_authority = instruction_context + .get_index_of_instruction_account_in_transaction(2) + .and_then(|index_in_transaction| { + transaction_context.get_key_of_account_at_index(index_in_transaction) + }) + .ok(); + + match account.get_state()? { + UpgradeableLoaderState::Buffer { authority_address } => { + if new_authority.is_none() { + ic_logger_msg!(log_collector, "Buffer authority is not optional"); + return Err(InstructionError::IncorrectAuthority); + } + if authority_address.is_none() { + ic_logger_msg!(log_collector, "Buffer is immutable"); + return Err(InstructionError::Immutable); + } + if authority_address != Some(*present_authority_key) { + ic_logger_msg!(log_collector, "Incorrect buffer authority provided"); + return Err(InstructionError::IncorrectAuthority); + } + if !instruction_context.is_instruction_account_signer(1)? { + ic_logger_msg!(log_collector, "Buffer authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + account.set_state(&UpgradeableLoaderState::Buffer { + authority_address: new_authority.cloned(), + })?; + } + UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address, + } => { + if upgrade_authority_address.is_none() { + ic_logger_msg!(log_collector, "Program not upgradeable"); + return Err(InstructionError::Immutable); + } + if upgrade_authority_address != Some(*present_authority_key) { + ic_logger_msg!(log_collector, "Incorrect upgrade authority provided"); + return Err(InstructionError::IncorrectAuthority); + } + if !instruction_context.is_instruction_account_signer(1)? { + ic_logger_msg!(log_collector, "Upgrade authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + if new_authority_must_sign && !instruction_context.is_instruction_account_signer(2)? { + ic_logger_msg!(log_collector, "New authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + account.set_state(&UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address: new_authority.cloned(), + })?; + } + _ => { + ic_logger_msg!(log_collector, "Account does not support authorities"); + return Err(InstructionError::InvalidArgument); + } + } + + ic_logger_msg!(log_collector, "New authority {:?}", new_authority); + + Ok(()) +} + fn process_loader_upgradeable_instruction( first_instruction_account: IndexOfAccount, invoke_context: &mut InvokeContext, @@ -913,69 +989,20 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "Upgraded program {:?}", new_program_id); } UpgradeableLoaderInstruction::SetAuthority => { - instruction_context.check_number_of_instruction_accounts(2)?; - let mut account = - instruction_context.try_borrow_instruction_account(transaction_context, 0)?; - let present_authority_key = transaction_context.get_key_of_account_at_index( - instruction_context.get_index_of_instruction_account_in_transaction(1)?, + process_set_authority( + instruction_context, + transaction_context, + &log_collector, + false, + )?; + } + UpgradeableLoaderInstruction::SetAuthorityChecked => { + process_set_authority( + instruction_context, + transaction_context, + &log_collector, + true, )?; - let new_authority = instruction_context - .get_index_of_instruction_account_in_transaction(2) - .and_then(|index_in_transaction| { - transaction_context.get_key_of_account_at_index(index_in_transaction) - }) - .ok(); - - match account.get_state()? { - UpgradeableLoaderState::Buffer { authority_address } => { - if new_authority.is_none() { - ic_logger_msg!(log_collector, "Buffer authority is not optional"); - return Err(InstructionError::IncorrectAuthority); - } - if authority_address.is_none() { - ic_logger_msg!(log_collector, "Buffer is immutable"); - return Err(InstructionError::Immutable); - } - if authority_address != Some(*present_authority_key) { - ic_logger_msg!(log_collector, "Incorrect buffer authority provided"); - return Err(InstructionError::IncorrectAuthority); - } - if !instruction_context.is_instruction_account_signer(1)? { - ic_logger_msg!(log_collector, "Buffer authority did not sign"); - return Err(InstructionError::MissingRequiredSignature); - } - account.set_state(&UpgradeableLoaderState::Buffer { - authority_address: new_authority.cloned(), - })?; - } - UpgradeableLoaderState::ProgramData { - slot, - upgrade_authority_address, - } => { - if upgrade_authority_address.is_none() { - ic_logger_msg!(log_collector, "Program not upgradeable"); - return Err(InstructionError::Immutable); - } - if upgrade_authority_address != Some(*present_authority_key) { - ic_logger_msg!(log_collector, "Incorrect upgrade authority provided"); - return Err(InstructionError::IncorrectAuthority); - } - if !instruction_context.is_instruction_account_signer(1)? { - ic_logger_msg!(log_collector, "Upgrade authority did not sign"); - return Err(InstructionError::MissingRequiredSignature); - } - account.set_state(&UpgradeableLoaderState::ProgramData { - slot, - upgrade_authority_address: new_authority.cloned(), - })?; - } - _ => { - ic_logger_msg!(log_collector, "Account does not support authorities"); - return Err(InstructionError::InvalidArgument); - } - } - - ic_logger_msg!(log_collector, "New authority {:?}", new_authority); } UpgradeableLoaderInstruction::Close => { instruction_context.check_number_of_instruction_accounts(2)?; @@ -3604,6 +3631,192 @@ mod tests { ); } + #[test] + fn test_bpf_loader_upgradeable_set_upgrade_authority_checked() { + let instruction = + bincode::serialize(&UpgradeableLoaderInstruction::SetAuthorityChecked).unwrap(); + let loader_id = bpf_loader_upgradeable::id(); + let slot = 0; + let upgrade_authority_address = Pubkey::new_unique(); + let upgrade_authority_account = AccountSharedData::new(1, 0, &Pubkey::new_unique()); + let new_upgrade_authority_address = Pubkey::new_unique(); + let new_upgrade_authority_account = AccountSharedData::new(1, 0, &Pubkey::new_unique()); + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address( + &[program_address.as_ref()], + &bpf_loader_upgradeable::id(), + ); + let mut programdata_account = AccountSharedData::new( + 1, + UpgradeableLoaderState::size_of_programdata(0), + &bpf_loader_upgradeable::id(), + ); + programdata_account + .set_state(&UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address: Some(upgrade_authority_address), + }) + .unwrap(); + let programdata_meta = AccountMeta { + pubkey: programdata_address, + is_signer: false, + is_writable: true, + }; + let upgrade_authority_meta = AccountMeta { + pubkey: upgrade_authority_address, + is_signer: true, + is_writable: false, + }; + let new_upgrade_authority_meta = AccountMeta { + pubkey: new_upgrade_authority_address, + is_signer: true, + is_writable: false, + }; + + // Case: Set to new authority + let accounts = process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (programdata_address, programdata_account.clone()), + (upgrade_authority_address, upgrade_authority_account.clone()), + ( + new_upgrade_authority_address, + new_upgrade_authority_account.clone(), + ), + ], + vec![ + programdata_meta.clone(), + upgrade_authority_meta.clone(), + new_upgrade_authority_meta.clone(), + ], + Ok(()), + ); + + let state: UpgradeableLoaderState = accounts.first().unwrap().state().unwrap(); + assert_eq!( + state, + UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address: Some(new_upgrade_authority_address), + } + ); + + // Case: Authority did not sign + process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (programdata_address, programdata_account.clone()), + (upgrade_authority_address, upgrade_authority_account.clone()), + ], + vec![ + programdata_meta.clone(), + AccountMeta { + pubkey: upgrade_authority_address, + is_signer: false, + is_writable: false, + }, + new_upgrade_authority_meta.clone(), + ], + Err(InstructionError::MissingRequiredSignature), + ); + + // Case: New authority did not sign + process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (programdata_address, programdata_account.clone()), + (upgrade_authority_address, upgrade_authority_account.clone()), + ], + vec![ + programdata_meta.clone(), + upgrade_authority_meta.clone(), + AccountMeta { + pubkey: new_upgrade_authority_address, + is_signer: false, + is_writable: false, + }, + ], + Err(InstructionError::MissingRequiredSignature), + ); + + // Case: wrong authority + let invalid_upgrade_authority_address = Pubkey::new_unique(); + process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (programdata_address, programdata_account.clone()), + ( + invalid_upgrade_authority_address, + upgrade_authority_account.clone(), + ), + (new_upgrade_authority_address, new_upgrade_authority_account), + ], + vec![ + programdata_meta.clone(), + AccountMeta { + pubkey: invalid_upgrade_authority_address, + is_signer: true, + is_writable: false, + }, + new_upgrade_authority_meta.clone(), + ], + Err(InstructionError::IncorrectAuthority), + ); + + // Case: No authority + programdata_account + .set_state(&UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address: None, + }) + .unwrap(); + process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (programdata_address, programdata_account.clone()), + (upgrade_authority_address, upgrade_authority_account.clone()), + ], + vec![ + programdata_meta.clone(), + upgrade_authority_meta.clone(), + new_upgrade_authority_meta.clone(), + ], + Err(InstructionError::Immutable), + ); + + // Case: Not a ProgramData account + programdata_account + .set_state(&UpgradeableLoaderState::Program { + programdata_address: Pubkey::new_unique(), + }) + .unwrap(); + process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (programdata_address, programdata_account.clone()), + (upgrade_authority_address, upgrade_authority_account), + ], + vec![ + programdata_meta, + upgrade_authority_meta, + new_upgrade_authority_meta, + ], + Err(InstructionError::InvalidArgument), + ); + } + #[test] fn test_bpf_loader_upgradeable_set_buffer_authority() { let instruction = bincode::serialize(&UpgradeableLoaderInstruction::SetAuthority).unwrap(); diff --git a/sdk/program/src/bpf_loader_upgradeable.rs b/sdk/program/src/bpf_loader_upgradeable.rs index a2d2aa49119cc0..0e7cdaa826657f 100644 --- a/sdk/program/src/bpf_loader_upgradeable.rs +++ b/sdk/program/src/bpf_loader_upgradeable.rs @@ -224,7 +224,7 @@ pub fn is_upgrade_instruction(instruction_data: &[u8]) -> bool { } pub fn is_set_authority_instruction(instruction_data: &[u8]) -> bool { - !instruction_data.is_empty() && 4 == instruction_data[0] + !instruction_data.is_empty() && (4 == instruction_data[0] || 7 == instruction_data[0]) } pub fn is_close_instruction(instruction_data: &[u8]) -> bool { @@ -266,6 +266,27 @@ pub fn set_upgrade_authority( Instruction::new_with_bincode(id(), &UpgradeableLoaderInstruction::SetAuthority, metas) } +/// Returns the instructions required to set a program's authority. If using this instruction, the new authority +/// must sign . +pub fn set_upgrade_authority_checked( + program_address: &Pubkey, + current_authority_address: &Pubkey, + new_authority_address: &Pubkey, +) -> Instruction { + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + + let metas = vec![ + AccountMeta::new(programdata_address, false), + AccountMeta::new_readonly(*current_authority_address, true), + AccountMeta::new_readonly(*new_authority_address, true), + ]; + Instruction::new_with_bincode( + id(), + &UpgradeableLoaderInstruction::SetAuthorityChecked, + metas, + ) +} + /// Returns the instructions required to close a buffer account pub fn close( close_address: &Pubkey, @@ -453,6 +474,10 @@ mod tests { is_set_authority_instruction, UpgradeableLoaderInstruction::SetAuthority {}, ); + assert_is_instruction( + is_set_authority_instruction, + UpgradeableLoaderInstruction::SetAuthorityChecked {}, + ); } #[test] diff --git a/sdk/program/src/loader_upgradeable_instruction.rs b/sdk/program/src/loader_upgradeable_instruction.rs index b8832c5329e9de..1d2a9a8f9bfad9 100644 --- a/sdk/program/src/loader_upgradeable_instruction.rs +++ b/sdk/program/src/loader_upgradeable_instruction.rs @@ -147,4 +147,17 @@ pub enum UpgradeableLoaderInstruction { /// Number of bytes to extend the program data. additional_bytes: u32, }, + + /// Set a new authority that is allowed to write the buffer or upgrade the + /// program. + /// + /// This instruction differs from SetAuthority in that the new authority is a + /// required signer. + /// + /// # Account references + /// 0. `[writable]` The Buffer or ProgramData account to change the + /// authority of. + /// 1. `[signer]` The current authority. + /// 2. `[signer]` The new authority. + SetAuthorityChecked, } diff --git a/transaction-status/src/parse_bpf_loader.rs b/transaction-status/src/parse_bpf_loader.rs index ee1a744599975b..76561d2d1bfe3c 100644 --- a/transaction-status/src/parse_bpf_loader.rs +++ b/transaction-status/src/parse_bpf_loader.rs @@ -139,6 +139,17 @@ pub fn parse_bpf_upgradeable_loader( }), }) } + UpgradeableLoaderInstruction::SetAuthorityChecked => { + check_num_bpf_upgradeable_loader_accounts(&instruction.accounts, 2)?; + Ok(ParsedInstructionEnum { + instruction_type: "setAuthority".to_string(), + info: json!({ + "account": account_keys[instruction.accounts[0] as usize].to_string(), + "authority": account_keys[instruction.accounts[1] as usize].to_string(), + "newAuthority": account_keys[instruction.accounts[2] as usize].to_string(), + }), + }) + } UpgradeableLoaderInstruction::Close => { check_num_bpf_upgradeable_loader_accounts(&instruction.accounts, 3)?; Ok(ParsedInstructionEnum { From 15f8d73d0f4a9900fbb2d6ebb1e898befb970d01 Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Wed, 19 Oct 2022 15:28:22 -0400 Subject: [PATCH 02/15] restore old logic for loader --- programs/bpf_loader/src/lib.rs | 150 ++++++++++++++------------------- 1 file changed, 63 insertions(+), 87 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index bc463af475efd7..76f65e586eec4c 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -495,82 +495,6 @@ fn process_instruction_common( } } -fn process_set_authority( - instruction_context: &InstructionContext, - transaction_context: &TransactionContext, - log_collector: &Option>>, - new_authority_must_sign: bool, -) -> Result<(), InstructionError> { - instruction_context.check_number_of_instruction_accounts(2)?; - let mut account = instruction_context.try_borrow_instruction_account(transaction_context, 0)?; - let present_authority_key = transaction_context.get_key_of_account_at_index( - instruction_context.get_index_of_instruction_account_in_transaction(1)?, - )?; - let new_authority = instruction_context - .get_index_of_instruction_account_in_transaction(2) - .and_then(|index_in_transaction| { - transaction_context.get_key_of_account_at_index(index_in_transaction) - }) - .ok(); - - match account.get_state()? { - UpgradeableLoaderState::Buffer { authority_address } => { - if new_authority.is_none() { - ic_logger_msg!(log_collector, "Buffer authority is not optional"); - return Err(InstructionError::IncorrectAuthority); - } - if authority_address.is_none() { - ic_logger_msg!(log_collector, "Buffer is immutable"); - return Err(InstructionError::Immutable); - } - if authority_address != Some(*present_authority_key) { - ic_logger_msg!(log_collector, "Incorrect buffer authority provided"); - return Err(InstructionError::IncorrectAuthority); - } - if !instruction_context.is_instruction_account_signer(1)? { - ic_logger_msg!(log_collector, "Buffer authority did not sign"); - return Err(InstructionError::MissingRequiredSignature); - } - account.set_state(&UpgradeableLoaderState::Buffer { - authority_address: new_authority.cloned(), - })?; - } - UpgradeableLoaderState::ProgramData { - slot, - upgrade_authority_address, - } => { - if upgrade_authority_address.is_none() { - ic_logger_msg!(log_collector, "Program not upgradeable"); - return Err(InstructionError::Immutable); - } - if upgrade_authority_address != Some(*present_authority_key) { - ic_logger_msg!(log_collector, "Incorrect upgrade authority provided"); - return Err(InstructionError::IncorrectAuthority); - } - if !instruction_context.is_instruction_account_signer(1)? { - ic_logger_msg!(log_collector, "Upgrade authority did not sign"); - return Err(InstructionError::MissingRequiredSignature); - } - if new_authority_must_sign && !instruction_context.is_instruction_account_signer(2)? { - ic_logger_msg!(log_collector, "New authority did not sign"); - return Err(InstructionError::MissingRequiredSignature); - } - account.set_state(&UpgradeableLoaderState::ProgramData { - slot, - upgrade_authority_address: new_authority.cloned(), - })?; - } - _ => { - ic_logger_msg!(log_collector, "Account does not support authorities"); - return Err(InstructionError::InvalidArgument); - } - } - - ic_logger_msg!(log_collector, "New authority {:?}", new_authority); - - Ok(()) -} - fn process_loader_upgradeable_instruction( first_instruction_account: IndexOfAccount, invoke_context: &mut InvokeContext, @@ -989,20 +913,72 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "Upgraded program {:?}", new_program_id); } UpgradeableLoaderInstruction::SetAuthority => { - process_set_authority( - instruction_context, - transaction_context, - &log_collector, - false, + instruction_context.check_number_of_instruction_accounts(2)?; + let mut account = + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let present_authority_key = transaction_context.get_key_of_account_at_index( + instruction_context.get_index_of_instruction_account_in_transaction(1)?, )?; + let new_authority = instruction_context + .get_index_of_instruction_account_in_transaction(2) + .and_then(|index_in_transaction| { + transaction_context.get_key_of_account_at_index(index_in_transaction) + }) + .ok(); + + match account.get_state()? { + UpgradeableLoaderState::Buffer { authority_address } => { + if new_authority.is_none() { + ic_logger_msg!(log_collector, "Buffer authority is not optional"); + return Err(InstructionError::IncorrectAuthority); + } + if authority_address.is_none() { + ic_logger_msg!(log_collector, "Buffer is immutable"); + return Err(InstructionError::Immutable); + } + if authority_address != Some(*present_authority_key) { + ic_logger_msg!(log_collector, "Incorrect buffer authority provided"); + return Err(InstructionError::IncorrectAuthority); + } + if !instruction_context.is_instruction_account_signer(1)? { + ic_logger_msg!(log_collector, "Buffer authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + account.set_state(&UpgradeableLoaderState::Buffer { + authority_address: new_authority.cloned(), + })?; + } + UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address, + } => { + if upgrade_authority_address.is_none() { + ic_logger_msg!(log_collector, "Program not upgradeable"); + return Err(InstructionError::Immutable); + } + if upgrade_authority_address != Some(*present_authority_key) { + ic_logger_msg!(log_collector, "Incorrect upgrade authority provided"); + return Err(InstructionError::IncorrectAuthority); + } + if !instruction_context.is_instruction_account_signer(1)? { + ic_logger_msg!(log_collector, "Upgrade authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + account.set_state(&UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address: new_authority.cloned(), + })?; + } + _ => { + ic_logger_msg!(log_collector, "Account does not support authorities"); + return Err(InstructionError::InvalidArgument); + } + } + + ic_logger_msg!(log_collector, "New authority {:?}", new_authority); } UpgradeableLoaderInstruction::SetAuthorityChecked => { - process_set_authority( - instruction_context, - transaction_context, - &log_collector, - true, - )?; + todo!() } UpgradeableLoaderInstruction::Close => { instruction_context.check_number_of_instruction_accounts(2)?; From dbb88873ef762de76fdc72b5533671442d794b45 Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Wed, 19 Oct 2022 16:32:45 -0400 Subject: [PATCH 03/15] add more upgrade authority checked test cases --- programs/bpf_loader/src/lib.rs | 107 +++++++++++++++++++++++++++++++-- 1 file changed, 103 insertions(+), 4 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 76f65e586eec4c..76d38ab7ae21cd 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -978,7 +978,70 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "New authority {:?}", new_authority); } UpgradeableLoaderInstruction::SetAuthorityChecked => { - todo!() + instruction_context.check_number_of_instruction_accounts(3)?; + let mut account = + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let present_authority_key = transaction_context.get_key_of_account_at_index( + instruction_context.get_index_of_instruction_account_in_transaction(1)?, + )?; + let new_authority_key = transaction_context.get_key_of_account_at_index( + instruction_context.get_index_of_instruction_account_in_transaction(2)?, + )?; + + match account.get_state()? { + UpgradeableLoaderState::Buffer { authority_address } => { + if authority_address.is_none() { + ic_logger_msg!(log_collector, "Buffer is immutable"); + return Err(InstructionError::Immutable); + } + if authority_address != Some(*present_authority_key) { + ic_logger_msg!(log_collector, "Incorrect buffer authority provided"); + return Err(InstructionError::IncorrectAuthority); + } + if !instruction_context.is_instruction_account_signer(1)? { + ic_logger_msg!(log_collector, "Buffer authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + if !instruction_context.is_instruction_account_signer(2)? { + ic_logger_msg!(log_collector, "New authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + account.set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(*new_authority_key), + })?; + } + UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address, + } => { + if upgrade_authority_address.is_none() { + ic_logger_msg!(log_collector, "Program not upgradeable"); + return Err(InstructionError::Immutable); + } + if upgrade_authority_address != Some(*present_authority_key) { + ic_logger_msg!(log_collector, "Incorrect upgrade authority provided"); + return Err(InstructionError::IncorrectAuthority); + } + if !instruction_context.is_instruction_account_signer(1)? { + ic_logger_msg!(log_collector, "Upgrade authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + if !instruction_context.is_instruction_account_signer(2)? { + ic_logger_msg!(log_collector, "New authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + account.set_state(&UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address: Some(*new_authority_key), + })?; + } + _ => { + ic_logger_msg!(log_collector, "Account does not support authorities"); + return Err(InstructionError::InvalidArgument); + } + } + + ic_logger_msg!(log_collector, "New authority {:?}", new_authority_key); } UpgradeableLoaderInstruction::Close => { instruction_context.check_number_of_instruction_accounts(2)?; @@ -3679,7 +3742,41 @@ mod tests { } ); - // Case: Authority did not sign + // Case: present authority not in instruction + process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (programdata_address, programdata_account.clone()), + (upgrade_authority_address, upgrade_authority_account.clone()), + (new_upgrade_authority_address, new_upgrade_authority_account.clone()), + ], + vec![ + programdata_meta.clone(), + new_upgrade_authority_meta.clone(), + ], + Err(InstructionError::NotEnoughAccountKeys), + ); + + // Case: new authority not in instruction + process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (programdata_address, programdata_account.clone()), + (upgrade_authority_address, upgrade_authority_account.clone()), + (new_upgrade_authority_address, new_upgrade_authority_account.clone()), + ], + vec![ + programdata_meta.clone(), + upgrade_authority_meta.clone(), + ], + Err(InstructionError::NotEnoughAccountKeys), + ); + + // Case: present authority did not sign process_instruction( &loader_id, &[], @@ -3687,6 +3784,7 @@ mod tests { vec![ (programdata_address, programdata_account.clone()), (upgrade_authority_address, upgrade_authority_account.clone()), + (new_upgrade_authority_address, new_upgrade_authority_account.clone()), ], vec![ programdata_meta.clone(), @@ -3708,6 +3806,7 @@ mod tests { vec![ (programdata_address, programdata_account.clone()), (upgrade_authority_address, upgrade_authority_account.clone()), + (new_upgrade_authority_address, new_upgrade_authority_account.clone()), ], vec![ programdata_meta.clone(), @@ -3721,7 +3820,7 @@ mod tests { Err(InstructionError::MissingRequiredSignature), ); - // Case: wrong authority + // Case: wrong present authority let invalid_upgrade_authority_address = Pubkey::new_unique(); process_instruction( &loader_id, @@ -3747,7 +3846,7 @@ mod tests { Err(InstructionError::IncorrectAuthority), ); - // Case: No authority + // Case: programdata is immutable programdata_account .set_state(&UpgradeableLoaderState::ProgramData { slot, From 518bcc73c6914d37e8eaf066440d0d451d3f22ca Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Wed, 19 Oct 2022 17:29:37 -0400 Subject: [PATCH 04/15] setBufferAuthority checked tests --- programs/bpf_loader/src/lib.rs | 190 +++++++++++++++++++++++++++++++++ 1 file changed, 190 insertions(+) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 76d38ab7ae21cd..78de4ebe4f32f7 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -4064,6 +4064,196 @@ mod tests { ); } + #[test] + fn test_bpf_loader_upgradeable_set_buffer_authority_checked() { + let instruction = bincode::serialize(&UpgradeableLoaderInstruction::SetAuthorityChecked).unwrap(); + let loader_id = bpf_loader_upgradeable::id(); + let invalid_authority_address = Pubkey::new_unique(); + let authority_address = Pubkey::new_unique(); + let authority_account = AccountSharedData::new(1, 0, &Pubkey::new_unique()); + let new_authority_address = Pubkey::new_unique(); + let new_authority_account = AccountSharedData::new(1, 0, &Pubkey::new_unique()); + let buffer_address = Pubkey::new_unique(); + let mut buffer_account = + AccountSharedData::new(1, UpgradeableLoaderState::size_of_buffer(0), &loader_id); + buffer_account + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(authority_address), + }) + .unwrap(); + let mut transaction_accounts = vec![ + (buffer_address, buffer_account.clone()), + (authority_address, authority_account.clone()), + (new_authority_address, new_authority_account.clone()), + ]; + let buffer_meta = AccountMeta { + pubkey: buffer_address, + is_signer: false, + is_writable: true, + }; + let authority_meta = AccountMeta { + pubkey: authority_address, + is_signer: true, + is_writable: false, + }; + let new_authority_meta = AccountMeta { + pubkey: new_authority_address, + is_signer: true, + is_writable: false, + }; + + // Case: Set to new authority + buffer_account + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(authority_address), + }) + .unwrap(); + let accounts = process_instruction( + &loader_id, + &[], + &instruction, + transaction_accounts.clone(), + vec![ + buffer_meta.clone(), + authority_meta.clone(), + new_authority_meta.clone(), + ], + Ok(()), + ); + let state: UpgradeableLoaderState = accounts.first().unwrap().state().unwrap(); + assert_eq!( + state, + UpgradeableLoaderState::Buffer { + authority_address: Some(new_authority_address), + } + ); + + // Case: Missing current authority + process_instruction( + &loader_id, + &[], + &instruction, + transaction_accounts.clone(), + vec![ + buffer_meta.clone(), + new_authority_meta.clone(), + ], + Err(InstructionError::NotEnoughAccountKeys), + ); + + // Case: Missing new authority + process_instruction( + &loader_id, + &[], + &instruction, + transaction_accounts.clone(), + vec![ + buffer_meta.clone(), + authority_meta.clone(), + ], + Err(InstructionError::NotEnoughAccountKeys), + ); + + // Case: wrong present authority + process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (buffer_address, buffer_account.clone()), + (invalid_authority_address, authority_account), + (new_authority_address, new_authority_account), + ], + vec![ + buffer_meta.clone(), + AccountMeta { + pubkey: invalid_authority_address, + is_signer: true, + is_writable: false, + }, + new_authority_meta.clone(), + ], + Err(InstructionError::IncorrectAuthority), + ); + + // Case: present authority did not sign + process_instruction( + &loader_id, + &[], + &instruction, + transaction_accounts.clone(), + vec![ + buffer_meta.clone(), + AccountMeta { + pubkey: authority_address, + is_signer: false, + is_writable: false, + }, + new_authority_meta.clone(), + ], + Err(InstructionError::MissingRequiredSignature), + ); + + // Case: new authority did not sign + process_instruction( + &loader_id, + &[], + &instruction, + transaction_accounts.clone(), + vec![ + buffer_meta.clone(), + authority_meta.clone(), + AccountMeta { + pubkey: new_authority_address, + is_signer: false, + is_writable: false, + }, + ], + Err(InstructionError::MissingRequiredSignature), + ); + + // Case: Not a Buffer account + transaction_accounts + .get_mut(0) + .unwrap() + .1 + .set_state(&UpgradeableLoaderState::Program { + programdata_address: Pubkey::new_unique(), + }) + .unwrap(); + process_instruction( + &loader_id, + &[], + &instruction, + transaction_accounts.clone(), + vec![buffer_meta.clone(), authority_meta.clone(), new_authority_meta.clone()], + Err(InstructionError::InvalidArgument), + ); + + // Case: Buffer is immutable + transaction_accounts + .get_mut(0) + .unwrap() + .1 + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: None, + }) + .unwrap(); + process_instruction( + &loader_id, + &[], + &instruction, + transaction_accounts.clone(), + vec![ + buffer_meta.clone(), + authority_meta.clone(), + new_authority_meta.clone(), + ], + Err(InstructionError::Immutable), + ); + + } + #[test] fn test_bpf_loader_upgradeable_close() { let instruction = bincode::serialize(&UpgradeableLoaderInstruction::Close).unwrap(); From 8a8d537d580b0610f90e4766ed292f5c88f06cf4 Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Wed, 19 Oct 2022 17:30:04 -0400 Subject: [PATCH 05/15] format --- programs/bpf_loader/src/lib.rs | 50 ++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 78de4ebe4f32f7..8ded9fd453e05f 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -3750,12 +3750,12 @@ mod tests { vec![ (programdata_address, programdata_account.clone()), (upgrade_authority_address, upgrade_authority_account.clone()), - (new_upgrade_authority_address, new_upgrade_authority_account.clone()), - ], - vec![ - programdata_meta.clone(), - new_upgrade_authority_meta.clone(), + ( + new_upgrade_authority_address, + new_upgrade_authority_account.clone(), + ), ], + vec![programdata_meta.clone(), new_upgrade_authority_meta.clone()], Err(InstructionError::NotEnoughAccountKeys), ); @@ -3767,12 +3767,12 @@ mod tests { vec![ (programdata_address, programdata_account.clone()), (upgrade_authority_address, upgrade_authority_account.clone()), - (new_upgrade_authority_address, new_upgrade_authority_account.clone()), - ], - vec![ - programdata_meta.clone(), - upgrade_authority_meta.clone(), + ( + new_upgrade_authority_address, + new_upgrade_authority_account.clone(), + ), ], + vec![programdata_meta.clone(), upgrade_authority_meta.clone()], Err(InstructionError::NotEnoughAccountKeys), ); @@ -3784,7 +3784,10 @@ mod tests { vec![ (programdata_address, programdata_account.clone()), (upgrade_authority_address, upgrade_authority_account.clone()), - (new_upgrade_authority_address, new_upgrade_authority_account.clone()), + ( + new_upgrade_authority_address, + new_upgrade_authority_account.clone(), + ), ], vec![ programdata_meta.clone(), @@ -3806,7 +3809,10 @@ mod tests { vec![ (programdata_address, programdata_account.clone()), (upgrade_authority_address, upgrade_authority_account.clone()), - (new_upgrade_authority_address, new_upgrade_authority_account.clone()), + ( + new_upgrade_authority_address, + new_upgrade_authority_account.clone(), + ), ], vec![ programdata_meta.clone(), @@ -4066,7 +4072,8 @@ mod tests { #[test] fn test_bpf_loader_upgradeable_set_buffer_authority_checked() { - let instruction = bincode::serialize(&UpgradeableLoaderInstruction::SetAuthorityChecked).unwrap(); + let instruction = + bincode::serialize(&UpgradeableLoaderInstruction::SetAuthorityChecked).unwrap(); let loader_id = bpf_loader_upgradeable::id(); let invalid_authority_address = Pubkey::new_unique(); let authority_address = Pubkey::new_unique(); @@ -4134,10 +4141,7 @@ mod tests { &[], &instruction, transaction_accounts.clone(), - vec![ - buffer_meta.clone(), - new_authority_meta.clone(), - ], + vec![buffer_meta.clone(), new_authority_meta.clone()], Err(InstructionError::NotEnoughAccountKeys), ); @@ -4147,10 +4151,7 @@ mod tests { &[], &instruction, transaction_accounts.clone(), - vec![ - buffer_meta.clone(), - authority_meta.clone(), - ], + vec![buffer_meta.clone(), authority_meta.clone()], Err(InstructionError::NotEnoughAccountKeys), ); @@ -4226,7 +4227,11 @@ mod tests { &[], &instruction, transaction_accounts.clone(), - vec![buffer_meta.clone(), authority_meta.clone(), new_authority_meta.clone()], + vec![ + buffer_meta.clone(), + authority_meta.clone(), + new_authority_meta.clone(), + ], Err(InstructionError::InvalidArgument), ); @@ -4251,7 +4256,6 @@ mod tests { ], Err(InstructionError::Immutable), ); - } #[test] From 6d336f2ce980b509f30594377f170be3315cbaad Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Wed, 19 Oct 2022 17:38:39 -0400 Subject: [PATCH 06/15] add set_buffer_authority_checked instruction to sdk --- sdk/program/src/bpf_loader_upgradeable.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/sdk/program/src/bpf_loader_upgradeable.rs b/sdk/program/src/bpf_loader_upgradeable.rs index 0e7cdaa826657f..b64d32bc7039ea 100644 --- a/sdk/program/src/bpf_loader_upgradeable.rs +++ b/sdk/program/src/bpf_loader_upgradeable.rs @@ -248,6 +248,24 @@ pub fn set_buffer_authority( ) } +/// Returns the instructions required to set a buffers's authority. If using this instruction, the new authority +/// must sign. +pub fn set_buffer_authority_checked( + buffer_address: &Pubkey, + current_authority_address: &Pubkey, + new_authority_address: &Pubkey, +) -> Instruction { + Instruction::new_with_bincode( + id(), + &UpgradeableLoaderInstruction::SetAuthorityChecked, + vec![ + AccountMeta::new(*buffer_address, false), + AccountMeta::new_readonly(*current_authority_address, true), + AccountMeta::new_readonly(*new_authority_address, false), + ], + ) +} + /// Returns the instructions required to set a program's authority. pub fn set_upgrade_authority( program_address: &Pubkey, @@ -267,7 +285,7 @@ pub fn set_upgrade_authority( } /// Returns the instructions required to set a program's authority. If using this instruction, the new authority -/// must sign . +/// must sign. pub fn set_upgrade_authority_checked( program_address: &Pubkey, current_authority_address: &Pubkey, From a9b3ea42ed386493ee3f9e8457c7c316459bafd4 Mon Sep 17 00:00:00 2001 From: 0xripleys <105607696+0xripleys@users.noreply.github.com> Date: Fri, 21 Oct 2022 15:38:19 +0100 Subject: [PATCH 07/15] Update transaction-status/src/parse_bpf_loader.rs Co-authored-by: Justin Starry --- transaction-status/src/parse_bpf_loader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transaction-status/src/parse_bpf_loader.rs b/transaction-status/src/parse_bpf_loader.rs index 76561d2d1bfe3c..76e7a7ec3231f7 100644 --- a/transaction-status/src/parse_bpf_loader.rs +++ b/transaction-status/src/parse_bpf_loader.rs @@ -140,7 +140,7 @@ pub fn parse_bpf_upgradeable_loader( }) } UpgradeableLoaderInstruction::SetAuthorityChecked => { - check_num_bpf_upgradeable_loader_accounts(&instruction.accounts, 2)?; + check_num_bpf_upgradeable_loader_accounts(&instruction.accounts, 3)?; Ok(ParsedInstructionEnum { instruction_type: "setAuthority".to_string(), info: json!({ From a53dc6badbe6265196cb8d2f4ef11ec3da4efcb3 Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Fri, 21 Oct 2022 15:52:45 +0100 Subject: [PATCH 08/15] add is_set_authority_checked function --- sdk/program/src/bpf_loader_upgradeable.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/sdk/program/src/bpf_loader_upgradeable.rs b/sdk/program/src/bpf_loader_upgradeable.rs index b64d32bc7039ea..b619d15e68c979 100644 --- a/sdk/program/src/bpf_loader_upgradeable.rs +++ b/sdk/program/src/bpf_loader_upgradeable.rs @@ -224,13 +224,17 @@ pub fn is_upgrade_instruction(instruction_data: &[u8]) -> bool { } pub fn is_set_authority_instruction(instruction_data: &[u8]) -> bool { - !instruction_data.is_empty() && (4 == instruction_data[0] || 7 == instruction_data[0]) + !instruction_data.is_empty() && 4 == instruction_data[0] } pub fn is_close_instruction(instruction_data: &[u8]) -> bool { !instruction_data.is_empty() && 5 == instruction_data[0] } +pub fn is_set_authority_checked_instruction(instruction_data: &[u8]) -> bool { + !instruction_data.is_empty() && 7 == instruction_data[0] +} + /// Returns the instructions required to set a buffers's authority. pub fn set_buffer_authority( buffer_address: &Pubkey, @@ -492,8 +496,13 @@ mod tests { is_set_authority_instruction, UpgradeableLoaderInstruction::SetAuthority {}, ); + } + + #[test] + fn test_is_set_authority_checked_instruction() { + assert!(!is_set_authority_checked_instruction(&[])); assert_is_instruction( - is_set_authority_instruction, + is_set_authority_checked_instruction, UpgradeableLoaderInstruction::SetAuthorityChecked {}, ); } From 1c3a88317b52912283ca4f8626e4d469a62f99b5 Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Fri, 21 Oct 2022 15:53:53 +0100 Subject: [PATCH 09/15] fix set_buffer_authority_checked sdk instruction --- sdk/program/src/bpf_loader_upgradeable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/program/src/bpf_loader_upgradeable.rs b/sdk/program/src/bpf_loader_upgradeable.rs index b619d15e68c979..7ad47a58b76e60 100644 --- a/sdk/program/src/bpf_loader_upgradeable.rs +++ b/sdk/program/src/bpf_loader_upgradeable.rs @@ -265,7 +265,7 @@ pub fn set_buffer_authority_checked( vec![ AccountMeta::new(*buffer_address, false), AccountMeta::new_readonly(*current_authority_address, true), - AccountMeta::new_readonly(*new_authority_address, false), + AccountMeta::new_readonly(*new_authority_address, true), ], ) } From 94f1d64efe797437a7166a42b9062ef244f4590e Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Fri, 21 Oct 2022 16:14:29 +0100 Subject: [PATCH 10/15] feature gate setAuthorityChecked --- programs/bpf_loader/src/lib.rs | 8 ++++++++ sdk/src/feature_set.rs | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 8ded9fd453e05f..72f9c2ae27f4bf 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -46,6 +46,7 @@ use { cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts, check_slice_translation_size, disable_deploy_of_alloc_free_syscall, disable_deprecated_loader, enable_bpf_loader_extend_program_ix, + enable_bpf_loader_set_authority_checked_ix, error_on_syscall_bpf_function_hash_collisions, limit_max_instruction_trace_length, reject_callx_r10, }, @@ -978,6 +979,13 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "New authority {:?}", new_authority); } UpgradeableLoaderInstruction::SetAuthorityChecked => { + if !invoke_context + .feature_set + .is_active(&enable_bpf_loader_set_authority_checked_ix::id()) + { + return Err(InstructionError::InvalidInstructionData); + } + instruction_context.check_number_of_instruction_accounts(3)?; let mut account = instruction_context.try_borrow_instruction_account(transaction_context, 0)?; diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index bd6e533b0c69f5..49599513782d4d 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -522,6 +522,10 @@ pub mod limit_max_instruction_trace_length { solana_sdk::declare_id!("GQALDaC48fEhZGWRj9iL5Q889emJKcj3aCvHF7VCbbF4"); } +pub mod enable_bpf_loader_set_authority_checked_ix { + solana_sdk::declare_id!("5x3825XS7M2A3Ekbn5VGGkvFoAg5qrRWkTrY4bARP1GL"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -647,6 +651,7 @@ lazy_static! { (remove_deprecated_request_unit_ix::id(), "remove support for RequestUnitsDeprecated instruction #27500"), (increase_tx_account_lock_limit::id(), "increase tx account lock limit to 128 #27241"), (limit_max_instruction_trace_length::id(), "limit max instruction trace length"), + (enable_bpf_loader_set_authority_checked_ix::id(), "enable bpf upgradeable loader SetAuthorityChecked instruction #28424"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() From 2bf5546da598b08582db4f93fd60403a330b098f Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Fri, 21 Oct 2022 16:25:01 +0100 Subject: [PATCH 11/15] add bpf loader tests for setAuthorityChecked ixs --- transaction-status/src/parse_bpf_loader.rs | 73 +++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/transaction-status/src/parse_bpf_loader.rs b/transaction-status/src/parse_bpf_loader.rs index 76e7a7ec3231f7..313c2dd4fb496e 100644 --- a/transaction-status/src/parse_bpf_loader.rs +++ b/transaction-status/src/parse_bpf_loader.rs @@ -142,7 +142,7 @@ pub fn parse_bpf_upgradeable_loader( UpgradeableLoaderInstruction::SetAuthorityChecked => { check_num_bpf_upgradeable_loader_accounts(&instruction.accounts, 3)?; Ok(ParsedInstructionEnum { - instruction_type: "setAuthority".to_string(), + instruction_type: "setAuthorityChecked".to_string(), info: json!({ "account": account_keys[instruction.accounts[0] as usize].to_string(), "authority": account_keys[instruction.accounts[1] as usize].to_string(), @@ -547,6 +547,39 @@ mod test { .is_err()); } + #[test] + fn test_parse_bpf_upgradeable_loader_set_buffer_authority_checked_ix() { + let buffer_address = Pubkey::new_unique(); + let current_authority_address = Pubkey::new_unique(); + let new_authority_address = Pubkey::new_unique(); + let instruction = bpf_loader_upgradeable::set_buffer_authority_checked( + &buffer_address, + ¤t_authority_address, + &new_authority_address, + ); + let message = Message::new(&[instruction], None); + assert_eq!( + parse_bpf_upgradeable_loader( + &message.instructions[0], + &AccountKeys::new(&message.account_keys, None) + ) + .unwrap(), + ParsedInstructionEnum { + instruction_type: "setAuthorityChecked".to_string(), + info: json!({ + "account": buffer_address.to_string(), + "authority": current_authority_address.to_string(), + "newAuthority": new_authority_address.to_string(), + }), + } + ); + assert!(parse_bpf_upgradeable_loader( + &message.instructions[0], + &AccountKeys::new(&message.account_keys[0..2], None) + ) + .is_err()); + } + #[test] fn test_parse_bpf_upgradeable_loader_set_upgrade_authority_ix() { let program_address = Pubkey::new_unique(); @@ -626,6 +659,44 @@ mod test { .is_err()); } + #[test] + fn test_parse_bpf_upgradeable_loader_set_upgrade_authority_checked_ix() { + let program_address = Pubkey::new_unique(); + let current_authority_address = Pubkey::new_unique(); + let new_authority_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address( + &[program_address.as_ref()], + &bpf_loader_upgradeable::id(), + ); + let instruction = bpf_loader_upgradeable::set_upgrade_authority_checked( + &program_address, + ¤t_authority_address, + &new_authority_address, + ); + let message = Message::new(&[instruction], None); + assert_eq!( + parse_bpf_upgradeable_loader( + &message.instructions[0], + &AccountKeys::new(&message.account_keys, None) + ) + .unwrap(), + ParsedInstructionEnum { + instruction_type: "setAuthorityChecked".to_string(), + info: json!({ + "account": programdata_address.to_string(), + "authority": current_authority_address.to_string(), + "newAuthority": new_authority_address.to_string(), + }), + } + ); + + assert!(parse_bpf_upgradeable_loader( + &message.instructions[0], + &AccountKeys::new(&message.account_keys[0..2], None) + ) + .is_err()); + } + #[test] fn test_parse_bpf_upgradeable_loader_close_buffer_ix() { let close_address = Pubkey::new_unique(); From ab7fef88d0484e8e399815c7bf4453036333c786 Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Fri, 21 Oct 2022 16:41:31 +0100 Subject: [PATCH 12/15] test that you can set to same authority --- programs/bpf_loader/src/lib.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 72f9c2ae27f4bf..3b0e97761ae766 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -3750,6 +3750,23 @@ mod tests { } ); + // Case: set to same authority + process_instruction( + &loader_id, + &[], + &instruction, + vec![ + (programdata_address, programdata_account.clone()), + (upgrade_authority_address, upgrade_authority_account.clone()), + ], + vec![ + programdata_meta.clone(), + upgrade_authority_meta.clone(), + upgrade_authority_meta.clone(), + ], + Ok(()), + ); + // Case: present authority not in instruction process_instruction( &loader_id, @@ -4143,6 +4160,20 @@ mod tests { } ); + // Case: set to same authority + process_instruction( + &loader_id, + &[], + &instruction, + transaction_accounts.clone(), + vec![ + buffer_meta.clone(), + authority_meta.clone(), + authority_meta.clone(), + ], + Ok(()), + ); + // Case: Missing current authority process_instruction( &loader_id, From 1ccf4d19b6f13d990ff866f5630c5cabc57796fc Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Fri, 28 Oct 2022 17:52:01 +0200 Subject: [PATCH 13/15] allow set_authority_checked to be called via cpi (if feature is enabled) --- programs/bpf_loader/src/syscalls/cpi.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index 7eb2290266cb5b..52cc49c8d26580 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -1,6 +1,7 @@ use { super::*, crate::declare_syscall, + solana_sdk::feature_set::enable_bpf_loader_set_authority_checked_ix, solana_sdk::syscalls::{ MAX_CPI_ACCOUNT_INFOS, MAX_CPI_INSTRUCTION_ACCOUNTS, MAX_CPI_INSTRUCTION_DATA_LEN, }, @@ -833,6 +834,12 @@ fn check_authorized_program( || (bpf_loader_upgradeable::check_id(program_id) && !(bpf_loader_upgradeable::is_upgrade_instruction(instruction_data) || bpf_loader_upgradeable::is_set_authority_instruction(instruction_data) + || (invoke_context + .feature_set + .is_active(&enable_bpf_loader_set_authority_checked_ix::id()) + && bpf_loader_upgradeable::is_set_authority_checked_instruction( + instruction_data, + )) || bpf_loader_upgradeable::is_close_instruction(instruction_data))) || is_precompile(program_id, |feature_id: &Pubkey| { invoke_context.feature_set.is_active(feature_id) From cad849f181ea4f557c0bb8ecc7a572fec912a46e Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Sat, 29 Oct 2022 13:59:15 +0200 Subject: [PATCH 14/15] fix ci --- programs/bpf_loader/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 42e8e24b3605bc..cea46473af0ddc 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -3612,9 +3612,9 @@ mod tests { &instruction, transaction_accounts.clone(), vec![ - buffer_meta.clone(), - authority_meta.clone(), - new_authority_meta.clone(), + buffer_meta, + authority_meta, + new_authority_meta, ], Err(InstructionError::Immutable), ); From f34956026e36b45ebbc1da76b20a8173e15aefd2 Mon Sep 17 00:00:00 2001 From: 0xripleys <0xripleys@solend.fi> Date: Sun, 30 Oct 2022 13:30:12 +0100 Subject: [PATCH 15/15] fmt --- programs/bpf_loader/src/lib.rs | 6 +----- programs/bpf_loader/src/syscalls/cpi.rs | 8 +++++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index cea46473af0ddc..b1e7773a7a8e14 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -3611,11 +3611,7 @@ mod tests { &[], &instruction, transaction_accounts.clone(), - vec![ - buffer_meta, - authority_meta, - new_authority_meta, - ], + vec![buffer_meta, authority_meta, new_authority_meta], Err(InstructionError::Immutable), ); } diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index 52cc49c8d26580..b23ee5bf086790 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -1,9 +1,11 @@ use { super::*, crate::declare_syscall, - solana_sdk::feature_set::enable_bpf_loader_set_authority_checked_ix, - solana_sdk::syscalls::{ - MAX_CPI_ACCOUNT_INFOS, MAX_CPI_INSTRUCTION_ACCOUNTS, MAX_CPI_INSTRUCTION_DATA_LEN, + solana_sdk::{ + feature_set::enable_bpf_loader_set_authority_checked_ix, + syscalls::{ + MAX_CPI_ACCOUNT_INFOS, MAX_CPI_INSTRUCTION_ACCOUNTS, MAX_CPI_INSTRUCTION_DATA_LEN, + }, }, };