diff --git a/cli/src/program.rs b/cli/src/program.rs index c921175d29..cf9dee5fb8 100644 --- a/cli/src/program.rs +++ b/cli/src/program.rs @@ -62,8 +62,7 @@ pub enum ProgramCliCommand { program_pubkey: Option, buffer_signer_index: Option, buffer_pubkey: Option, - upgrade_authority_signer_index: Option, - upgrade_authority_pubkey: Option, + upgrade_authority_signer_index: SignerIndex, is_final: bool, max_len: Option, allow_excessive_balance: bool, @@ -73,13 +72,12 @@ pub enum ProgramCliCommand { buffer_signer_index: Option, buffer_pubkey: Option, buffer_authority_signer_index: Option, - is_final: bool, max_len: Option, }, SetBufferAuthority { buffer_pubkey: Pubkey, buffer_authority_index: Option, - new_buffer_authority: Option, + new_buffer_authority: Pubkey, }, SetUpgradeAuthority { program_pubkey: Pubkey, @@ -119,10 +117,12 @@ impl ProgramSubCommands for App<'_, '_> { [default: random address]") ) .arg( - pubkey!(Arg::with_name("upgrade_authority") + Arg::with_name("upgrade_authority") .long("upgrade-authority") - .value_name("UPGRADE_AUTHORITY"), - "Upgrade authority [default: the default configured keypair]"), + .value_name("UPGRADE_AUTHORITY_SIGNER") + .takes_value(true) + .validator(is_valid_signer) + .help("Upgrade authority [default: the default configured keypair]") ) .arg( pubkey!(Arg::with_name("program_id") @@ -179,11 +179,6 @@ impl ProgramSubCommands for App<'_, '_> { .validator(is_valid_signer) .help("Buffer authority [default: the default configured keypair]") ) - .arg( - Arg::with_name("final") - .long("final") - .help("The program will not be upgradeable") - ) .arg( Arg::with_name("max_len") .long("max-len") @@ -217,15 +212,9 @@ impl ProgramSubCommands for App<'_, '_> { pubkey!(Arg::with_name("new_buffer_authority") .long("new-buffer-authority") .value_name("NEW_BUFFER_AUTHORITY") - .required_unless("final"), + .required(true), "Address of the new buffer authority"), ) - .arg( - Arg::with_name("final") - .long("final") - .conflicts_with("new_buffer_authority") - .help("The buffer will be immutable") - ) ) .subcommand( SubCommand::with_name("set-upgrade-authority") @@ -312,10 +301,6 @@ pub fn parse_program_subcommand( { bulk_signers.push(upgrade_authority_signer); Some(upgrade_authority_pubkey) - } else if let Some(upgrade_authority_pubkey) = - pubkey_of_signer(matches, "upgrade_authority", wallet_manager)? - { - Some(upgrade_authority_pubkey) } else { Some( default_signer @@ -337,8 +322,8 @@ pub fn parse_program_subcommand( buffer_signer_index: signer_info.index_of_or_none(buffer_pubkey), buffer_pubkey, upgrade_authority_signer_index: signer_info - .index_of_or_none(upgrade_authority_pubkey), - upgrade_authority_pubkey, + .index_of(upgrade_authority_pubkey) + .unwrap(), is_final: matches.is_present("final"), max_len, allow_excessive_balance: matches.is_present("allow_excessive_balance"), @@ -389,7 +374,6 @@ pub fn parse_program_subcommand( buffer_pubkey, buffer_authority_signer_index: signer_info .index_of_or_none(buffer_authority_pubkey), - is_final: matches.is_present("final"), max_len, }), signers: signer_info.signers, @@ -400,16 +384,14 @@ pub fn parse_program_subcommand( let (buffer_authority_signer, buffer_authority_pubkey) = signer_of(matches, "buffer_authority", wallet_manager)?; - let new_buffer_authority = if matches.is_present("final") { - None - } else if let Some(new_buffer_authority) = + let new_buffer_authority = if let Some(new_buffer_authority) = pubkey_of_signer(matches, "new_buffer_authority", wallet_manager)? { - Some(new_buffer_authority) + new_buffer_authority } else { let (_, new_buffer_authority) = signer_of(matches, "new_buffer_authority", wallet_manager)?; - new_buffer_authority + new_buffer_authority.unwrap() }; let signer_info = default_signer.generate_unique_signers( @@ -482,7 +464,6 @@ pub fn process_program_subcommand( buffer_signer_index, buffer_pubkey, upgrade_authority_signer_index, - upgrade_authority_pubkey, is_final, max_len, allow_excessive_balance, @@ -495,7 +476,6 @@ pub fn process_program_subcommand( *buffer_signer_index, *buffer_pubkey, *upgrade_authority_signer_index, - *upgrade_authority_pubkey, *is_final, *max_len, *allow_excessive_balance, @@ -505,7 +485,6 @@ pub fn process_program_subcommand( buffer_signer_index, buffer_pubkey, buffer_authority_signer_index, - is_final, max_len, } => process_write_buffer( &rpc_client, @@ -514,7 +493,6 @@ pub fn process_program_subcommand( *buffer_signer_index, *buffer_pubkey, *buffer_authority_signer_index, - *is_final, *max_len, ), ProgramCliCommand::SetBufferAuthority { @@ -527,7 +505,7 @@ pub fn process_program_subcommand( None, Some(*buffer_pubkey), *buffer_authority_index, - *new_buffer_authority, + Some(*new_buffer_authority), ), ProgramCliCommand::SetUpgradeAuthority { program_pubkey, @@ -575,8 +553,7 @@ fn process_program_deploy( program_pubkey: Option, buffer_signer_index: Option, buffer_pubkey: Option, - upgrade_authority_signer_index: Option, - upgrade_authority_pubkey: Option, + upgrade_authority_signer_index: SignerIndex, is_final: bool, max_len: Option, allow_excessive_balance: bool, @@ -592,6 +569,7 @@ fn process_program_deploy( buffer_keypair.pubkey(), ) }; + let upgrade_authority_signer = config.signers[upgrade_authority_signer_index]; let default_program_keypair = get_default_program_keypair(&program_location); let (program_signer, program_pubkey) = if let Some(i) = program_signer_index { @@ -628,10 +606,11 @@ fn process_program_deploy( if program_authority_pubkey.is_none() { return Err("Program is no longer upgradeable".into()); } - if program_authority_pubkey != upgrade_authority_pubkey { + if program_authority_pubkey != Some(upgrade_authority_signer.pubkey()) { return Err(format!( "Program's authority {:?} does not match authority provided {:?}", - program_authority_pubkey, upgrade_authority_pubkey, + program_authority_pubkey, + upgrade_authority_signer.pubkey(), ) .into()); } @@ -693,25 +672,22 @@ fn process_program_deploy( buffer_data_len, minimum_balance, &bpf_loader_upgradeable::id(), - Some(program_signer.unwrap()), + Some(&[program_signer.unwrap(), upgrade_authority_signer]), buffer_signer, &buffer_pubkey, - buffer_signer, - upgrade_authority_pubkey, + Some(upgrade_authority_signer), allow_excessive_balance, ) - } else if let Some(upgrade_authority_index) = upgrade_authority_signer_index { + } else { do_process_program_upgrade( rpc_client, config, &program_data, &program_pubkey, - config.signers[upgrade_authority_index], + config.signers[upgrade_authority_signer_index], &buffer_pubkey, buffer_signer, ) - } else { - return Err("Program upgrade requires an authority".into()); }; if result.is_ok() && is_final { process_set_authority( @@ -719,7 +695,7 @@ fn process_program_deploy( config, Some(program_pubkey), None, - upgrade_authority_signer_index, + Some(upgrade_authority_signer_index), None, )?; } @@ -736,7 +712,6 @@ fn process_write_buffer( buffer_signer_index: Option, buffer_pubkey: Option, buffer_authority_signer_index: Option, - is_final: bool, max_len: Option, ) -> ProcessResult { // Create ephemeral keypair to use for Buffer account, if not provided @@ -799,21 +774,9 @@ fn process_write_buffer( buffer_signer, &buffer_pubkey, Some(buffer_authority), - None, true, ); - if result.is_ok() && is_final { - process_set_authority( - rpc_client, - config, - None, - Some(buffer_pubkey), - buffer_authority_signer_index, - None, - )?; - } - if result.is_err() && buffer_signer_index.is_none() && buffer_signer.is_some() { report_ephemeral_mnemonic(words, mnemonic); } @@ -837,24 +800,28 @@ fn process_set_authority( trace!("Set a new authority"); let (blockhash, _) = rpc_client.get_recent_blockhash()?; - let mut tx = if let Some(pubkey) = program_pubkey { + let mut tx = if let Some(ref pubkey) = program_pubkey { Transaction::new_unsigned(Message::new( &[bpf_loader_upgradeable::set_upgrade_authority( - &pubkey, + pubkey, &authority_signer.pubkey(), new_authority.as_ref(), )], Some(&config.signers[0].pubkey()), )) } else if let Some(pubkey) = buffer_pubkey { - Transaction::new_unsigned(Message::new( - &[bpf_loader_upgradeable::set_buffer_authority( - &pubkey, - &authority_signer.pubkey(), - new_authority.as_ref(), - )], - Some(&config.signers[0].pubkey()), - )) + if let Some(ref new_authority) = new_authority { + Transaction::new_unsigned(Message::new( + &[bpf_loader_upgradeable::set_buffer_authority( + &pubkey, + &authority_signer.pubkey(), + new_authority, + )], + Some(&config.signers[0].pubkey()), + )) + } else { + return Err("Buffer authority cannot be None".into()); + } } else { return Err("Program or Buffer not provided".into()); }; @@ -916,11 +883,10 @@ pub fn process_deploy( program_data.len(), minimum_balance, &loader_id, - Some(buffer_signer), + Some(&[buffer_signer]), Some(buffer_signer), &buffer_signer.pubkey(), Some(buffer_signer), - None, allow_excessive_balance, ); if result.is_err() && buffer_signer_index.is_none() { @@ -937,11 +903,10 @@ fn do_process_program_write_and_deploy( buffer_data_len: usize, minimum_balance: u64, loader_id: &Pubkey, - program_signer: Option<&dyn Signer>, + program_signers: Option<&[&dyn Signer]>, buffer_signer: Option<&dyn Signer>, buffer_pubkey: &Pubkey, buffer_authority_signer: Option<&dyn Signer>, - upgrade_authority: Option, allow_excessive_balance: bool, ) -> ProcessResult { // Build messages to calculate fees @@ -972,7 +937,7 @@ fn do_process_program_write_and_deploy( bpf_loader_upgradeable::create_buffer( &config.signers[0].pubkey(), buffer_pubkey, - Some(&buffer_authority_signer.pubkey()), + &buffer_authority_signer.pubkey(), minimum_balance, buffer_data_len, )?, @@ -1006,7 +971,7 @@ fn do_process_program_write_and_deploy( let instruction = if loader_id == &bpf_loader_upgradeable::id() { bpf_loader_upgradeable::write( buffer_pubkey, - Some(&buffer_authority_signer.pubkey()), + &buffer_authority_signer.pubkey(), (i * DATA_CHUNK_SIZE) as u32, chunk.to_vec(), ) @@ -1040,14 +1005,14 @@ fn do_process_program_write_and_deploy( // Create and add final message - let final_message = if let Some(program_signer) = program_signer { + let final_message = if let Some(program_signers) = program_signers { let message = if loader_id == &bpf_loader_upgradeable::id() { Message::new( &bpf_loader_upgradeable::deploy_with_max_program_len( &config.signers[0].pubkey(), - &program_signer.pubkey(), + &program_signers[0].pubkey(), buffer_pubkey, - upgrade_authority.as_ref(), + &program_signers[1].pubkey(), rpc_client.get_minimum_balance_for_rent_exemption( UpgradeableLoaderState::program_len()?, )?, @@ -1079,12 +1044,12 @@ fn do_process_program_write_and_deploy( &final_message, buffer_signer, buffer_authority_signer, - program_signer, + program_signers, )?; - if let Some(program_signer) = program_signer { + if let Some(program_signers) = program_signers { Ok(json!({ - "programId": format!("{}", program_signer.pubkey()), + "programId": format!("{}", program_signers[0].pubkey()), }) .to_string()) } else { @@ -1134,7 +1099,7 @@ fn do_process_program_upgrade( bpf_loader_upgradeable::create_buffer( &config.signers[0].pubkey(), buffer_pubkey, - Some(&buffer_signer.pubkey()), + &upgrade_authority.pubkey(), minimum_balance, data_len, )?, @@ -1156,7 +1121,7 @@ fn do_process_program_upgrade( for (chunk, i) in program_data.chunks(DATA_CHUNK_SIZE).zip(0..) { let instruction = bpf_loader_upgradeable::write( &buffer_signer.pubkey(), - None, + &upgrade_authority.pubkey(), (i * DATA_CHUNK_SIZE) as u32, chunk.to_vec(), ); @@ -1200,8 +1165,8 @@ fn do_process_program_upgrade( &write_messages, &Some(final_message), buffer_signer, - buffer_signer, Some(upgrade_authority), + Some(&[upgrade_authority]), )?; Ok(json!({ @@ -1300,9 +1265,10 @@ fn send_deploy_messages( final_message: &Option, initial_signer: Option<&dyn Signer>, write_signer: Option<&dyn Signer>, - final_signer: Option<&dyn Signer>, + final_signers: Option<&[&dyn Signer]>, ) -> Result<(), Box> { let payer_signer = config.signers[0]; + if let Some(message) = initial_message { if let Some(initial_signer) = initial_signer { trace!("Preparing the required accounts"); @@ -1351,12 +1317,14 @@ fn send_deploy_messages( } if let Some(message) = final_message { - if let Some(final_signer) = final_signer { + if let Some(final_signers) = final_signers { trace!("Deploying program"); let (blockhash, _) = rpc_client.get_recent_blockhash()?; let mut final_tx = Transaction::new_unsigned(message.clone()); - final_tx.try_sign(&[payer_signer, final_signer], blockhash)?; + let mut signers = final_signers.to_vec(); + signers.push(payer_signer); + final_tx.try_sign(&signers, blockhash)?; rpc_client .send_and_confirm_transaction_with_spinner_and_config( &final_tx, @@ -1615,8 +1583,7 @@ mod tests { buffer_pubkey: None, program_signer_index: None, program_pubkey: None, - upgrade_authority_signer_index: Some(0), - upgrade_authority_pubkey: Some(default_keypair.pubkey()), + upgrade_authority_signer_index: 0, is_final: false, max_len: None, allow_excessive_balance: false, @@ -1642,8 +1609,7 @@ mod tests { buffer_pubkey: None, program_signer_index: None, program_pubkey: None, - upgrade_authority_signer_index: Some(0), - upgrade_authority_pubkey: Some(default_keypair.pubkey()), + upgrade_authority_signer_index: 0, is_final: false, max_len: Some(42), allow_excessive_balance: false, @@ -1671,8 +1637,7 @@ mod tests { buffer_pubkey: Some(buffer_keypair.pubkey()), program_signer_index: None, program_pubkey: None, - upgrade_authority_signer_index: Some(0), - upgrade_authority_pubkey: Some(default_keypair.pubkey()), + upgrade_authority_signer_index: 0, is_final: false, max_len: None, allow_excessive_balance: false, @@ -1702,8 +1667,7 @@ mod tests { buffer_pubkey: None, program_signer_index: None, program_pubkey: Some(program_pubkey), - upgrade_authority_signer_index: Some(0), - upgrade_authority_pubkey: Some(default_keypair.pubkey()), + upgrade_authority_signer_index: 0, is_final: false, max_len: None, allow_excessive_balance: false, @@ -1732,8 +1696,7 @@ mod tests { buffer_pubkey: None, program_signer_index: Some(1), program_pubkey: Some(program_keypair.pubkey()), - upgrade_authority_signer_index: Some(0), - upgrade_authority_pubkey: Some(default_keypair.pubkey()), + upgrade_authority_signer_index: 0, is_final: false, max_len: None, allow_excessive_balance: false, @@ -1745,34 +1708,6 @@ mod tests { } ); - let authority_pubkey = Pubkey::new_unique(); - let test_deploy = test_commands.clone().get_matches_from(vec![ - "test", - "program", - "deploy", - "/Users/test/program.so", - "--upgrade-authority", - &authority_pubkey.to_string(), - ]); - assert_eq!( - parse_command(&test_deploy, &default_signer, &mut None).unwrap(), - CliCommandInfo { - command: CliCommand::Program(ProgramCliCommand::Deploy { - program_location: Some("/Users/test/program.so".to_string()), - buffer_signer_index: None, - buffer_pubkey: None, - program_signer_index: None, - program_pubkey: None, - upgrade_authority_signer_index: None, - upgrade_authority_pubkey: Some(authority_pubkey), - is_final: false, - max_len: None, - allow_excessive_balance: false, - }), - signers: vec![read_keypair_file(&keypair_file).unwrap().into()], - } - ); - let authority_keypair = Keypair::new(); let authority_keypair_file = make_tmp_path("authority_keypair_file"); write_keypair_file(&authority_keypair, &authority_keypair_file).unwrap(); @@ -1793,8 +1728,7 @@ mod tests { buffer_pubkey: None, program_signer_index: None, program_pubkey: None, - upgrade_authority_signer_index: Some(1), - upgrade_authority_pubkey: Some(authority_keypair.pubkey()), + upgrade_authority_signer_index: 1, is_final: false, max_len: None, allow_excessive_balance: false, @@ -1822,8 +1756,7 @@ mod tests { buffer_pubkey: None, program_signer_index: None, program_pubkey: None, - upgrade_authority_signer_index: Some(0), - upgrade_authority_pubkey: Some(default_keypair.pubkey()), + upgrade_authority_signer_index: 0, is_final: true, max_len: None, allow_excessive_balance: false, @@ -1861,7 +1794,6 @@ mod tests { buffer_signer_index: None, buffer_pubkey: None, buffer_authority_signer_index: Some(0), - is_final: false, max_len: None, }), signers: vec![read_keypair_file(&keypair_file).unwrap().into()], @@ -1885,7 +1817,6 @@ mod tests { buffer_signer_index: None, buffer_pubkey: None, buffer_authority_signer_index: Some(0), - is_final: false, max_len: Some(42), }), signers: vec![read_keypair_file(&keypair_file).unwrap().into()], @@ -1912,7 +1843,6 @@ mod tests { buffer_signer_index: Some(1), buffer_pubkey: Some(buffer_keypair.pubkey()), buffer_authority_signer_index: Some(0), - is_final: false, max_len: None, }), signers: vec![ @@ -1942,7 +1872,6 @@ mod tests { buffer_signer_index: None, buffer_pubkey: None, buffer_authority_signer_index: Some(1), - is_final: false, max_len: None, }), signers: vec![ @@ -1977,7 +1906,6 @@ mod tests { buffer_signer_index: Some(1), buffer_pubkey: Some(buffer_keypair.pubkey()), buffer_authority_signer_index: Some(2), - is_final: false, max_len: None, }), signers: vec![ @@ -1987,60 +1915,6 @@ mod tests { ], } ); - - // specify authority - let test_deploy = test_commands.clone().get_matches_from(vec![ - "test", - "program", - "write-buffer", - "/Users/test/program.so", - "--final", - ]); - assert_eq!( - parse_command(&test_deploy, &default_signer, &mut None).unwrap(), - CliCommandInfo { - command: CliCommand::Program(ProgramCliCommand::WriteBuffer { - program_location: "/Users/test/program.so".to_string(), - buffer_signer_index: None, - buffer_pubkey: None, - buffer_authority_signer_index: Some(0), - is_final: true, - max_len: None, - }), - signers: vec![read_keypair_file(&keypair_file).unwrap().into()], - } - ); - - // specify both buffer and authority and final - let authority_keypair = Keypair::new(); - let authority_keypair_file = make_tmp_path("authority_keypair_file"); - write_keypair_file(&authority_keypair, &authority_keypair_file).unwrap(); - let test_deploy = test_commands.clone().get_matches_from(vec![ - "test", - "program", - "write-buffer", - "/Users/test/program.so", - "--buffer-authority", - &authority_keypair_file, - "--final", - ]); - assert_eq!( - parse_command(&test_deploy, &default_signer, &mut None).unwrap(), - CliCommandInfo { - command: CliCommand::Program(ProgramCliCommand::WriteBuffer { - program_location: "/Users/test/program.so".to_string(), - buffer_signer_index: None, - buffer_pubkey: None, - buffer_authority_signer_index: Some(1), - is_final: true, - max_len: None, - }), - signers: vec![ - read_keypair_file(&keypair_file).unwrap().into(), - read_keypair_file(&authority_keypair_file).unwrap().into(), - ], - } - ); } #[test] @@ -2186,7 +2060,7 @@ mod tests { command: CliCommand::Program(ProgramCliCommand::SetBufferAuthority { buffer_pubkey, buffer_authority_index: Some(0), - new_buffer_authority: Some(new_authority_pubkey), + new_buffer_authority: new_authority_pubkey, }), signers: vec![read_keypair_file(&keypair_file).unwrap().into()], } @@ -2210,65 +2084,11 @@ mod tests { command: CliCommand::Program(ProgramCliCommand::SetBufferAuthority { buffer_pubkey, buffer_authority_index: Some(0), - new_buffer_authority: Some(new_authority_pubkey.pubkey()), + new_buffer_authority: new_authority_pubkey.pubkey(), }), signers: vec![read_keypair_file(&keypair_file).unwrap().into()], } ); - - let buffer_pubkey = Pubkey::new_unique(); - let new_authority_pubkey = Keypair::new(); - let new_authority_pubkey_file = make_tmp_path("authority_keypair_file"); - write_keypair_file(&new_authority_pubkey, &new_authority_pubkey_file).unwrap(); - let test_deploy = test_commands.clone().get_matches_from(vec![ - "test", - "program", - "set-buffer-authority", - &buffer_pubkey.to_string(), - "--final", - ]); - assert_eq!( - parse_command(&test_deploy, &default_signer, &mut None).unwrap(), - CliCommandInfo { - command: CliCommand::Program(ProgramCliCommand::SetBufferAuthority { - buffer_pubkey, - buffer_authority_index: Some(0), - new_buffer_authority: None, - }), - signers: vec![read_keypair_file(&keypair_file).unwrap().into()], - } - ); - - let buffer_pubkey = Pubkey::new_unique(); - let authority = Keypair::new(); - let authority_keypair_file = make_tmp_path("authority_keypair_file"); - write_keypair_file(&authority, &authority_keypair_file).unwrap(); - let new_authority_pubkey = Keypair::new(); - let new_authority_pubkey_file = make_tmp_path("authority_keypair_file"); - write_keypair_file(&new_authority_pubkey, &new_authority_pubkey_file).unwrap(); - let test_deploy = test_commands.clone().get_matches_from(vec![ - "test", - "program", - "set-buffer-authority", - &buffer_pubkey.to_string(), - "--buffer-authority", - &authority_keypair_file, - "--final", - ]); - assert_eq!( - parse_command(&test_deploy, &default_signer, &mut None).unwrap(), - CliCommandInfo { - command: CliCommand::Program(ProgramCliCommand::SetBufferAuthority { - buffer_pubkey, - buffer_authority_index: Some(1), - new_buffer_authority: None, - }), - signers: vec![ - read_keypair_file(&keypair_file).unwrap().into(), - read_keypair_file(&authority_keypair_file).unwrap().into(), - ], - } - ); } #[test] @@ -2299,8 +2119,7 @@ mod tests { buffer_pubkey: None, program_signer_index: None, program_pubkey: None, - upgrade_authority_signer_index: None, - upgrade_authority_pubkey: Some(default_keypair.pubkey()), + upgrade_authority_signer_index: 0, is_final: false, max_len: None, allow_excessive_balance: false, diff --git a/cli/tests/program.rs b/cli/tests/program.rs index 16fb675ce8..5527bc6282 100644 --- a/cli/tests/program.rs +++ b/cli/tests/program.rs @@ -202,8 +202,8 @@ fn test_cli_program_deploy_no_authority() { config.signers = vec![&keypair]; process_command(&config).unwrap(); - // Deploy a program with no authority - config.signers = vec![&keypair]; + // Deploy a program + config.signers = vec![&keypair, &upgrade_authority]; config.command = CliCommand::Program(ProgramCliCommand::Deploy { program_location: Some(pathbuf.to_str().unwrap().to_string()), program_signer_index: None, @@ -211,9 +211,8 @@ fn test_cli_program_deploy_no_authority() { buffer_signer_index: None, buffer_pubkey: None, allow_excessive_balance: false, - upgrade_authority_signer_index: None, - upgrade_authority_pubkey: None, - is_final: false, + upgrade_authority_signer_index: 1, + is_final: true, max_len: None, }); let response = process_command(&config); @@ -236,8 +235,7 @@ fn test_cli_program_deploy_no_authority() { buffer_signer_index: None, buffer_pubkey: None, allow_excessive_balance: false, - upgrade_authority_signer_index: Some(1), - upgrade_authority_pubkey: Some(upgrade_authority.pubkey()), + upgrade_authority_signer_index: 1, is_final: false, max_len: None, }); @@ -308,8 +306,7 @@ fn test_cli_program_deploy_with_authority() { buffer_signer_index: None, buffer_pubkey: None, allow_excessive_balance: false, - upgrade_authority_signer_index: Some(1), - upgrade_authority_pubkey: Some(upgrade_authority.pubkey()), + upgrade_authority_signer_index: 1, is_final: false, max_len: Some(max_len), }); @@ -355,8 +352,7 @@ fn test_cli_program_deploy_with_authority() { buffer_signer_index: None, buffer_pubkey: None, allow_excessive_balance: false, - upgrade_authority_signer_index: Some(1), - upgrade_authority_pubkey: Some(upgrade_authority.pubkey()), + upgrade_authority_signer_index: 1, is_final: false, max_len: Some(max_len), }); @@ -397,8 +393,7 @@ fn test_cli_program_deploy_with_authority() { buffer_signer_index: None, buffer_pubkey: None, allow_excessive_balance: false, - upgrade_authority_signer_index: Some(1), - upgrade_authority_pubkey: Some(upgrade_authority.pubkey()), + upgrade_authority_signer_index: 1, is_final: false, max_len: Some(max_len), }); @@ -452,8 +447,7 @@ fn test_cli_program_deploy_with_authority() { buffer_signer_index: None, buffer_pubkey: None, allow_excessive_balance: false, - upgrade_authority_signer_index: Some(1), - upgrade_authority_pubkey: Some(new_upgrade_authority.pubkey()), + upgrade_authority_signer_index: 1, is_final: false, max_len: None, }); @@ -503,8 +497,7 @@ fn test_cli_program_deploy_with_authority() { buffer_signer_index: None, buffer_pubkey: None, allow_excessive_balance: false, - upgrade_authority_signer_index: Some(1), - upgrade_authority_pubkey: Some(new_upgrade_authority.pubkey()), + upgrade_authority_signer_index: 1, is_final: false, max_len: None, }); @@ -519,8 +512,7 @@ fn test_cli_program_deploy_with_authority() { buffer_signer_index: None, buffer_pubkey: None, allow_excessive_balance: false, - upgrade_authority_signer_index: Some(1), - upgrade_authority_pubkey: Some(new_upgrade_authority.pubkey()), + upgrade_authority_signer_index: 1, is_final: true, max_len: None, }); @@ -610,7 +602,6 @@ fn test_cli_program_write_buffer() { buffer_signer_index: None, buffer_pubkey: None, buffer_authority_signer_index: None, - is_final: false, max_len: None, }); let response = process_command(&config); @@ -644,7 +635,6 @@ fn test_cli_program_write_buffer() { buffer_signer_index: Some(1), buffer_pubkey: Some(buffer_keypair.pubkey()), buffer_authority_signer_index: None, - is_final: false, max_len: Some(max_len), }); let response = process_command(&config); @@ -682,7 +672,6 @@ fn test_cli_program_write_buffer() { buffer_signer_index: Some(1), buffer_pubkey: Some(buffer_keypair.pubkey()), buffer_authority_signer_index: Some(2), - is_final: false, max_len: None, }); let response = process_command(&config); @@ -720,7 +709,6 @@ fn test_cli_program_write_buffer() { buffer_signer_index: None, buffer_pubkey: None, buffer_authority_signer_index: Some(2), - is_final: false, max_len: None, }); let response = process_command(&config); @@ -746,35 +734,6 @@ fn test_cli_program_write_buffer() { program_data[..] ); - // Specify final - let buffer_keypair = Keypair::new(); - let authority_keypair = Keypair::new(); - config.signers = vec![&keypair, &buffer_keypair, &authority_keypair]; - config.command = CliCommand::Program(ProgramCliCommand::WriteBuffer { - program_location: pathbuf.to_str().unwrap().to_string(), - buffer_signer_index: None, - buffer_pubkey: None, - buffer_authority_signer_index: Some(2), - is_final: true, - max_len: None, - }); - let response = process_command(&config); - let json: Value = serde_json::from_str(&response.unwrap()).unwrap(); - let buffer_pubkey_str = json - .as_object() - .unwrap() - .get("buffer") - .unwrap() - .as_str() - .unwrap(); - let buffer_pubkey = Pubkey::from_str(&buffer_pubkey_str).unwrap(); - let buffer_account = rpc_client.get_account(&buffer_pubkey).unwrap(); - if let UpgradeableLoaderState::Buffer { authority_address } = buffer_account.state().unwrap() { - assert_eq!(authority_address, None); - } else { - panic!("not a buffer account"); - } - server.close().unwrap(); remove_dir_all(ledger_path).unwrap(); } @@ -834,7 +793,6 @@ fn test_cli_program_set_buffer_authority() { buffer_signer_index: Some(1), buffer_pubkey: Some(buffer_keypair.pubkey()), buffer_authority_signer_index: None, - is_final: false, max_len: None, }); process_command(&config).unwrap(); @@ -851,7 +809,7 @@ fn test_cli_program_set_buffer_authority() { config.command = CliCommand::Program(ProgramCliCommand::SetBufferAuthority { buffer_pubkey: buffer_keypair.pubkey(), buffer_authority_index: Some(0), - new_buffer_authority: Some(new_buffer_authority.pubkey()), + new_buffer_authority: new_buffer_authority.pubkey(), }); let response = process_command(&config); let json: Value = serde_json::from_str(&response.unwrap()).unwrap(); @@ -878,7 +836,7 @@ fn test_cli_program_set_buffer_authority() { config.command = CliCommand::Program(ProgramCliCommand::SetBufferAuthority { buffer_pubkey: buffer_keypair.pubkey(), buffer_authority_index: Some(1), - new_buffer_authority: Some(buffer_keypair.pubkey()), + new_buffer_authority: buffer_keypair.pubkey(), }); let response = process_command(&config); let json: Value = serde_json::from_str(&response.unwrap()).unwrap(); @@ -900,30 +858,107 @@ fn test_cli_program_set_buffer_authority() { panic!("not a buffer account"); } - // Set authority to None - config.signers = vec![&keypair, &buffer_keypair]; - config.command = CliCommand::Program(ProgramCliCommand::SetBufferAuthority { - buffer_pubkey: buffer_keypair.pubkey(), - buffer_authority_index: Some(1), - new_buffer_authority: None, - }); - let response = process_command(&config); - let json: Value = serde_json::from_str(&response.unwrap()).unwrap(); - let buffer_authority_str = json - .as_object() - .unwrap() - .get("authority") - .unwrap() - .as_str() + server.close().unwrap(); + remove_dir_all(ledger_path).unwrap(); +} + +#[test] +fn test_cli_program_mismatch_buffer_authority() { + solana_logger::setup(); + + let mut pathbuf = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + pathbuf.push("tests"); + pathbuf.push("fixtures"); + pathbuf.push("noop"); + pathbuf.set_extension("so"); + + let TestValidator { + server, + leader_data, + alice, + ledger_path, + .. + } = TestValidator::run(); + + let (sender, receiver) = channel(); + run_local_faucet(alice, sender, None); + let faucet_addr = receiver.recv().unwrap(); + + let rpc_client = + RpcClient::new_socket_with_commitment(leader_data.rpc, CommitmentConfig::recent()); + + let mut file = File::open(pathbuf.to_str().unwrap()).unwrap(); + let mut program_data = Vec::new(); + file.read_to_end(&mut program_data).unwrap(); + let max_len = program_data.len(); + let minimum_balance_for_buffer = rpc_client + .get_minimum_balance_for_rent_exemption( + UpgradeableLoaderState::programdata_len(max_len).unwrap(), + ) .unwrap(); - assert_eq!(buffer_authority_str, "none"); + + let mut config = CliConfig::recent_for_tests(); + let keypair = Keypair::new(); + config.json_rpc_url = format!("http://{}:{}", leader_data.rpc.ip(), leader_data.rpc.port()); + config.signers = vec![&keypair]; + config.command = CliCommand::Airdrop { + faucet_host: None, + faucet_port: faucet_addr.port(), + pubkey: None, + lamports: 100 * minimum_balance_for_buffer, + }; + process_command(&config).unwrap(); + + // Write a buffer + let buffer_authority = Keypair::new(); + let buffer_keypair = Keypair::new(); + config.signers = vec![&keypair, &buffer_keypair, &buffer_authority]; + config.command = CliCommand::Program(ProgramCliCommand::WriteBuffer { + program_location: pathbuf.to_str().unwrap().to_string(), + buffer_signer_index: Some(1), + buffer_pubkey: Some(buffer_keypair.pubkey()), + buffer_authority_signer_index: Some(2), + max_len: None, + }); + process_command(&config).unwrap(); let buffer_account = rpc_client.get_account(&buffer_keypair.pubkey()).unwrap(); if let UpgradeableLoaderState::Buffer { authority_address } = buffer_account.state().unwrap() { - assert_eq!(authority_address, None); + assert_eq!(authority_address, Some(buffer_authority.pubkey())); } else { panic!("not a buffer account"); } + // Attempt to deploy with mismatched authority + let upgrade_authority = Keypair::new(); + config.signers = vec![&keypair, &upgrade_authority]; + config.command = CliCommand::Program(ProgramCliCommand::Deploy { + program_location: Some(pathbuf.to_str().unwrap().to_string()), + program_signer_index: None, + program_pubkey: None, + buffer_signer_index: None, + buffer_pubkey: Some(buffer_keypair.pubkey()), + allow_excessive_balance: false, + upgrade_authority_signer_index: 1, + is_final: true, + max_len: None, + }); + process_command(&config).unwrap_err(); + + // Attempt to deploy matched authority + config.signers = vec![&keypair, &buffer_authority]; + config.command = CliCommand::Program(ProgramCliCommand::Deploy { + program_location: Some(pathbuf.to_str().unwrap().to_string()), + program_signer_index: None, + program_pubkey: None, + buffer_signer_index: None, + buffer_pubkey: Some(buffer_keypair.pubkey()), + allow_excessive_balance: false, + upgrade_authority_signer_index: 1, + is_final: true, + max_len: None, + }); + process_command(&config).unwrap(); + server.close().unwrap(); remove_dir_all(ledger_path).unwrap(); } diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index e4f44d242a..c48232d75a 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -140,7 +140,13 @@ fn upgrade_bpf_program( }); let mut elf = Vec::new(); file.read_to_end(&mut elf).unwrap(); - load_buffer_account(bank_client, payer_keypair, &buffer_keypair, &elf); + load_buffer_account( + bank_client, + payer_keypair, + &buffer_keypair, + authority_keypair, + &elf, + ); upgrade_program( bank_client, payer_keypair, @@ -1499,6 +1505,93 @@ fn test_program_bpf_upgrade() { ); } +#[cfg(feature = "bpf_rust")] +#[test] +fn test_program_bpf_upgrade_and_invoke_in_same_tx() { + solana_logger::setup(); + + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(50); + let mut bank = Bank::new(&genesis_config); + let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); + bank.add_builtin(&name, id, entrypoint); + let bank = Arc::new(bank); + let bank_client = BankClient::new_shared(&bank); + + // Deploy upgrade program + let buffer_keypair = Keypair::new(); + let program_keypair = Keypair::new(); + let program_id = program_keypair.pubkey(); + let authority_keypair = Keypair::new(); + load_upgradeable_bpf_program( + &bank_client, + &mint_keypair, + &buffer_keypair, + &program_keypair, + &authority_keypair, + "noop", + ); + + let invoke_instruction = Instruction::new( + program_id, + &[0], + vec![ + AccountMeta::new(program_id.clone(), false), + AccountMeta::new(clock::id(), false), + AccountMeta::new(fees::id(), false), + ], + ); + + // Call upgradeable program + let result = + bank_client.send_and_confirm_instruction(&mint_keypair, invoke_instruction.clone()); + assert!(result.is_ok()); + + // Prepare for upgrade + let buffer_keypair = Keypair::new(); + let path = create_bpf_path("panic"); + let mut file = File::open(&path).unwrap_or_else(|err| { + panic!("Failed to open {}: {}", path.display(), err); + }); + let mut elf = Vec::new(); + file.read_to_end(&mut elf).unwrap(); + load_buffer_account( + &bank_client, + &mint_keypair, + &buffer_keypair, + &authority_keypair, + &elf, + ); + + // Invoke, then upgrade the program, and then invoke again in same tx + let message = Message::new( + &[ + invoke_instruction.clone(), + bpf_loader_upgradeable::upgrade( + &program_id, + &buffer_keypair.pubkey(), + &authority_keypair.pubkey(), + &mint_keypair.pubkey(), + ), + invoke_instruction, + ], + Some(&mint_keypair.pubkey()), + ); + let tx = Transaction::new( + &[&mint_keypair, &authority_keypair], + message.clone(), + bank.last_blockhash(), + ); + let (result, _) = process_transaction_and_record_inner(&bank, tx); + assert_eq!( + result.unwrap_err(), + TransactionError::InstructionError(2, InstructionError::ProgramFailedToComplete) + ); +} + #[cfg(feature = "bpf_rust")] #[test] fn test_program_bpf_invoke_upgradeable_via_cpi() { @@ -1710,7 +1803,13 @@ fn test_program_bpf_upgrade_via_cpi() { let mut elf = Vec::new(); file.read_to_end(&mut elf).unwrap(); let buffer_keypair = Keypair::new(); - load_buffer_account(&bank_client, &mint_keypair, &buffer_keypair, &elf); + load_buffer_account( + &bank_client, + &mint_keypair, + &buffer_keypair, + &authority_keypair, + &elf, + ); // Upgrade program via CPI let mut upgrade_instruction = bpf_loader_upgradeable::upgrade( @@ -1737,6 +1836,98 @@ fn test_program_bpf_upgrade_via_cpi() { ); } +#[cfg(feature = "bpf_rust")] +#[test] +fn test_program_bpf_upgrade_self_via_cpi() { + solana_logger::setup(); + + let GenesisConfigInfo { + genesis_config, + mint_keypair, + .. + } = create_genesis_config(50); + let mut bank = Bank::new(&genesis_config); + let (name, id, entrypoint) = solana_bpf_loader_program!(); + bank.add_builtin(&name, id, entrypoint); + let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!(); + bank.add_builtin(&name, id, entrypoint); + let bank = Arc::new(bank); + let bank_client = BankClient::new_shared(&bank); + let noop_program_id = load_bpf_program(&bank_client, &bpf_loader::id(), &mint_keypair, "noop"); + + // Deploy upgradeable program + let buffer_keypair = Keypair::new(); + let program_keypair = Keypair::new(); + let program_id = program_keypair.pubkey(); + let authority_keypair = Keypair::new(); + load_upgradeable_bpf_program( + &bank_client, + &mint_keypair, + &buffer_keypair, + &program_keypair, + &authority_keypair, + "solana_bpf_rust_invoke_and_return", + ); + + let mut invoke_instruction = Instruction::new( + program_id, + &[0], + vec![ + AccountMeta::new(noop_program_id, false), + AccountMeta::new(noop_program_id, false), + AccountMeta::new(clock::id(), false), + AccountMeta::new(fees::id(), false), + ], + ); + + // Call the upgraded program + invoke_instruction.data[0] += 1; + let result = + bank_client.send_and_confirm_instruction(&mint_keypair, invoke_instruction.clone()); + assert!(result.is_ok()); + + // Prepare for upgrade + let buffer_keypair = Keypair::new(); + let path = create_bpf_path("panic"); + let mut file = File::open(&path).unwrap_or_else(|err| { + panic!("Failed to open {}: {}", path.display(), err); + }); + let mut elf = Vec::new(); + file.read_to_end(&mut elf).unwrap(); + load_buffer_account( + &bank_client, + &mint_keypair, + &buffer_keypair, + &authority_keypair, + &elf, + ); + + // Invoke, then upgrade the program, and then invoke again in same tx + let message = Message::new( + &[ + invoke_instruction.clone(), + bpf_loader_upgradeable::upgrade( + &program_id, + &buffer_keypair.pubkey(), + &authority_keypair.pubkey(), + &mint_keypair.pubkey(), + ), + invoke_instruction, + ], + Some(&mint_keypair.pubkey()), + ); + let tx = Transaction::new( + &[&mint_keypair, &authority_keypair], + message.clone(), + bank.last_blockhash(), + ); + let (result, _) = process_transaction_and_record_inner(&bank, tx); + assert_eq!( + result.unwrap_err(), + TransactionError::InstructionError(2, InstructionError::ProgramFailedToComplete) + ); +} + #[cfg(feature = "bpf_rust")] #[test] fn test_program_upgradeable_locks() { @@ -1774,7 +1965,13 @@ fn test_program_upgradeable_locks() { }); let mut elf = Vec::new(); file.read_to_end(&mut elf).unwrap(); - load_buffer_account(&bank_client, &mint_keypair, buffer_keypair, &elf); + load_buffer_account( + &bank_client, + &mint_keypair, + buffer_keypair, + &payer_keypair, + &elf, + ); bank_client .send_and_confirm_instruction( diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 977410d996..69d68d2dc9 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -23,7 +23,10 @@ use solana_sdk::{ bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::Clock, entrypoint::SUCCESS, - feature_set::{bpf_compute_budget_balancing, prevent_upgrade_and_invoke}, + feature_set::{ + bpf_compute_budget_balancing, matching_buffer_upgrade_authorities, + prevent_upgrade_and_invoke, + }, instruction::InstructionError, keyed_account::{from_keyed_account, next_keyed_account, KeyedAccount}, loader_instruction::LoaderInstruction, @@ -242,17 +245,26 @@ fn process_loader_upgradeable_instruction( match limited_deserialize(instruction_data)? { UpgradeableLoaderInstruction::InitializeBuffer => { let buffer = next_keyed_account(account_iter)?; - let authority = next_keyed_account(account_iter) - .ok() - .map(|account| account.unsigned_key()); if UpgradeableLoaderState::Uninitialized != buffer.state()? { log!(logger, "Buffer account already initialized"); return Err(InstructionError::AccountAlreadyInitialized); } - buffer.set_state(&UpgradeableLoaderState::Buffer { - authority_address: authority.cloned(), - })?; + + if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id()) { + let authority = next_keyed_account(account_iter)?; + + buffer.set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(*authority.unsigned_key()), + })?; + } else { + let authority = next_keyed_account(account_iter) + .ok() + .map(|account| account.unsigned_key()); + buffer.set_state(&UpgradeableLoaderState::Buffer { + authority_address: authority.cloned(), + })?; + } } UpgradeableLoaderInstruction::Write { offset, bytes } => { let buffer = next_keyed_account(account_iter)?; @@ -290,9 +302,19 @@ fn process_loader_upgradeable_instruction( let rent = from_keyed_account::(next_keyed_account(account_iter)?)?; let clock = from_keyed_account::(next_keyed_account(account_iter)?)?; let system = next_keyed_account(account_iter)?; - let authority = next_keyed_account(account_iter) - .ok() - .map(|account| account.unsigned_key()); + let (upgrade_authority_address, upgrade_authority_signer) = + if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id()) { + let authority = next_keyed_account(account_iter)?; + ( + Some(*authority.unsigned_key()), + authority.signer_key().is_none(), + ) + } else { + let authority = next_keyed_account(account_iter) + .ok() + .map(|account| account.unsigned_key()); + (authority.cloned(), false) + }; // Verify Program account @@ -311,10 +333,17 @@ fn process_loader_upgradeable_instruction( // Verify Buffer account - if let UpgradeableLoaderState::Buffer { - authority_address: _, - } = buffer.state()? - { + if let UpgradeableLoaderState::Buffer { authority_address } = buffer.state()? { + if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id()) { + if authority_address != upgrade_authority_address { + log!(logger, "Buffer and upgrade authority don't match"); + return Err(InstructionError::IncorrectAuthority); + } + if upgrade_authority_signer { + log!(logger, "Upgrade authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + } } else { log!(logger, "Invalid Buffer account"); return Err(InstructionError::InvalidArgument); @@ -369,15 +398,13 @@ fn process_loader_upgradeable_instruction( )?; // Update the ProgramData account and record the program bits - programdata.set_state(&UpgradeableLoaderState::ProgramData { slot: clock.slot, - upgrade_authority_address: authority.cloned(), + upgrade_authority_address, })?; programdata.try_account_ref_mut()?.data [programdata_data_offset..programdata_data_offset + buffer_data_len] .copy_from_slice(&buffer.try_account_ref()?.data[buffer_data_offset..]); - // Update the Program account program.set_state(&UpgradeableLoaderState::Program { @@ -432,10 +459,17 @@ fn process_loader_upgradeable_instruction( // Verify Buffer account - if let UpgradeableLoaderState::Buffer { - authority_address: _, - } = buffer.state()? - { + if let UpgradeableLoaderState::Buffer { authority_address } = buffer.state()? { + if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id()) { + if authority_address != Some(*authority.unsigned_key()) { + log!(logger, "Buffer and upgrade authority don't match"); + return Err(InstructionError::IncorrectAuthority); + } + if authority.signer_key().is_none() { + log!(logger, "Upgrade authority did not sign"); + return Err(InstructionError::MissingRequiredSignature); + } + } } else { log!(logger, "Invalid Buffer account"); return Err(InstructionError::InvalidArgument); @@ -528,6 +562,12 @@ fn process_loader_upgradeable_instruction( match account.state()? { UpgradeableLoaderState::Buffer { authority_address } => { + if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id()) + && new_authority == None + { + log!(logger, "Buffer authority is not optional"); + return Err(InstructionError::IncorrectAuthority); + } if authority_address == None { log!(logger, "Buffer is immutable"); return Err(InstructionError::Immutable); @@ -1125,55 +1165,14 @@ mod tests { UpgradeableLoaderState::buffer_len(9).unwrap(), &bpf_loader_upgradeable::id(), ); - - // Case: Success - assert_eq!( - Ok(()), - process_instruction( - &bpf_loader_upgradeable::id(), - &[KeyedAccount::new(&buffer_address, false, &buffer_account)], - &instruction, - &mut MockInvokeContext::default() - ) - ); - let state: UpgradeableLoaderState = buffer_account.borrow().state().unwrap(); - assert_eq!( - state, - UpgradeableLoaderState::Buffer { - authority_address: None - } - ); - - // Case: Already initialized - assert_eq!( - Err(InstructionError::AccountAlreadyInitialized), - process_instruction( - &bpf_loader_upgradeable::id(), - &[KeyedAccount::new(&buffer_address, false, &buffer_account)], - &instruction, - &mut MockInvokeContext::default() - ) - ); - let state: UpgradeableLoaderState = buffer_account.borrow().state().unwrap(); - assert_eq!( - state, - UpgradeableLoaderState::Buffer { - authority_address: None - } - ); - - // Case: With authority - let buffer_account = Account::new_ref( - 1, - UpgradeableLoaderState::buffer_len(9).unwrap(), - &bpf_loader_upgradeable::id(), - ); let authority_address = Pubkey::new_unique(); let authority_account = Account::new_ref( 1, UpgradeableLoaderState::buffer_len(9).unwrap(), &bpf_loader_upgradeable::id(), ); + + // Case: Success assert_eq!( Ok(()), process_instruction( @@ -1193,6 +1192,27 @@ mod tests { authority_address: Some(authority_address) } ); + + // Case: Already initialized + assert_eq!( + Err(InstructionError::AccountAlreadyInitialized), + process_instruction( + &bpf_loader_upgradeable::id(), + &[ + KeyedAccount::new(&buffer_address, false, &buffer_account), + KeyedAccount::new(&authority_address, false, &authority_account) + ], + &instruction, + &mut MockInvokeContext::default() + ) + ); + let state: UpgradeableLoaderState = buffer_account.borrow().state().unwrap(); + assert_eq!( + state, + UpgradeableLoaderState::Buffer { + authority_address: Some(authority_address) + } + ); } #[test] @@ -1400,12 +1420,39 @@ mod tests { &mut MockInvokeContext::default() ) ); + + // Case: None authority + let authority_address = Pubkey::new_unique(); + let instruction = bincode::serialize(&UpgradeableLoaderInstruction::Write { + offset: 1, + bytes: vec![42; 9], + }) + .unwrap(); + buffer_account + .borrow_mut() + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: None, + }) + .unwrap(); + assert_eq!( + Err(InstructionError::Immutable), + process_instruction( + &bpf_loader_upgradeable::id(), + &[ + KeyedAccount::new(&buffer_address, false, &buffer_account), + KeyedAccount::new(&authority_address, true, &buffer_account) + ], + &instruction, + &mut MockInvokeContext::default() + ) + ); } #[test] fn test_bpf_loader_upgradeable_deploy_with_max_len() { let (genesis_config, mint_keypair) = create_genesis_config(1_000_000_000); let mut bank = Bank::new(&genesis_config); + bank.feature_set = Arc::new(FeatureSet::all_enabled()); bank.add_builtin( "solana_bpf_loader_upgradeable_program", bpf_loader_upgradeable::id(), @@ -1420,7 +1467,7 @@ mod tests { &[program_keypair.pubkey().as_ref()], &bpf_loader_upgradeable::id(), ); - let upgrade_authority_address = Pubkey::new_unique(); + let upgrade_authority_keypair = Keypair::new(); let mut file = File::open("test_elfs/noop_aligned.so").expect("file open failed"); let mut elf = Vec::new(); file.read_to_end(&mut elf).unwrap(); @@ -1437,11 +1484,21 @@ mod tests { ); buffer_account .set_state(&UpgradeableLoaderState::Buffer { - authority_address: Some(buffer_address), + authority_address: Some(upgrade_authority_keypair.pubkey()), }) .unwrap(); buffer_account.data[UpgradeableLoaderState::buffer_data_offset().unwrap()..] .copy_from_slice(&elf); + let program_account = Account::new( + min_programdata_balance, + UpgradeableLoaderState::program_len().unwrap(), + &bpf_loader_upgradeable::id(), + ); + let programdata_account = Account::new( + 1, + UpgradeableLoaderState::programdata_len(elf.len()).unwrap(), + &bpf_loader_upgradeable::id(), + ); // Test successful deploy bank.clear_signatures(); @@ -1454,7 +1511,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - Some(&upgrade_authority_address), + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len(), ) @@ -1462,7 +1519,10 @@ mod tests { Some(&mint_keypair.pubkey()), ); assert!(bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .is_ok()); assert_eq!( bank.get_balance(&mint_keypair.pubkey()), @@ -1492,7 +1552,7 @@ mod tests { state, UpgradeableLoaderState::ProgramData { slot: bank_client.get_slot().unwrap(), - upgrade_authority_address: Some(upgrade_authority_address) + upgrade_authority_address: Some(upgrade_authority_keypair.pubkey()) } ); for (i, byte) in post_programdata_account.data @@ -1520,6 +1580,7 @@ mod tests { AccountMeta::new_readonly(sysvar::rent::id(), false), AccountMeta::new_readonly(sysvar::clock::id(), false), AccountMeta::new_readonly(system_program::id(), false), + AccountMeta::new_readonly(upgrade_authority_keypair.pubkey(), true), ], )], Some(&mint_keypair.pubkey()), @@ -1527,58 +1588,11 @@ mod tests { assert_eq!( TransactionError::InstructionError(0, InstructionError::AccountAlreadyInitialized), bank_client - .send_and_confirm_message(&[&mint_keypair], message) + .send_and_confirm_message(&[&mint_keypair, &upgrade_authority_keypair], message) .unwrap_err() .unwrap() ); - // Test successful deploy no authority - bank.clear_signatures(); - bank.store_account(&buffer_address, &buffer_account); - bank.store_account(&program_keypair.pubkey(), &Account::default()); - bank.store_account(&programdata_address, &Account::default()); - let message = Message::new( - &bpf_loader_upgradeable::deploy_with_max_program_len( - &mint_keypair.pubkey(), - &program_keypair.pubkey(), - &buffer_address, - None, - min_program_balance, - elf.len(), - ) - .unwrap(), - Some(&mint_keypair.pubkey()), - ); - assert!(bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) - .is_ok()); - assert_eq!(None, bank.get_account(&buffer_address)); - let post_program_account = bank.get_account(&program_keypair.pubkey()).unwrap(); - assert_eq!(post_program_account.lamports, min_program_balance); - assert_eq!(post_program_account.owner, bpf_loader_upgradeable::id()); - assert_eq!( - post_program_account.data.len(), - UpgradeableLoaderState::program_len().unwrap() - ); - let state: UpgradeableLoaderState = post_program_account.state().unwrap(); - assert_eq!( - state, - UpgradeableLoaderState::Program { - programdata_address - } - ); - let post_programdata_account = bank.get_account(&programdata_address).unwrap(); - assert_eq!(post_programdata_account.lamports, min_programdata_balance); - assert_eq!(post_programdata_account.owner, bpf_loader_upgradeable::id()); - let state: UpgradeableLoaderState = post_programdata_account.state().unwrap(); - assert_eq!( - state, - UpgradeableLoaderState::ProgramData { - slot: bank_client.get_slot().unwrap(), - upgrade_authority_address: None - } - ); - // Test initialized ProgramData account bank.clear_signatures(); bank.store_account(&buffer_address, &buffer_account); @@ -1588,7 +1602,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len(), ) @@ -1598,7 +1612,73 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::Custom(0)), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) + .unwrap_err() + .unwrap() + ); + + // Test deploy no authority + bank.clear_signatures(); + bank.store_account(&buffer_address, &buffer_account); + bank.store_account(&program_keypair.pubkey(), &program_account); + bank.store_account(&programdata_address, &programdata_account); + let message = Message::new( + &[Instruction::new( + bpf_loader_upgradeable::id(), + &UpgradeableLoaderInstruction::DeployWithMaxDataLen { + max_data_len: elf.len(), + }, + vec![ + AccountMeta::new(mint_keypair.pubkey(), true), + AccountMeta::new(programdata_address, false), + AccountMeta::new(program_keypair.pubkey(), false), + AccountMeta::new(buffer_address, false), + AccountMeta::new_readonly(sysvar::rent::id(), false), + AccountMeta::new_readonly(sysvar::clock::id(), false), + AccountMeta::new_readonly(system_program::id(), false), + ], + )], + Some(&mint_keypair.pubkey()), + ); + assert_eq!( + TransactionError::InstructionError(0, InstructionError::NotEnoughAccountKeys), + bank_client + .send_and_confirm_message(&[&mint_keypair], message) + .unwrap_err() + .unwrap() + ); + + // Test deploy authority not a signer + bank.clear_signatures(); + bank.store_account(&buffer_address, &buffer_account); + bank.store_account(&program_keypair.pubkey(), &program_account); + bank.store_account(&programdata_address, &programdata_account); + let message = Message::new( + &[Instruction::new( + bpf_loader_upgradeable::id(), + &UpgradeableLoaderInstruction::DeployWithMaxDataLen { + max_data_len: elf.len(), + }, + vec![ + AccountMeta::new(mint_keypair.pubkey(), true), + AccountMeta::new(programdata_address, false), + AccountMeta::new(program_keypair.pubkey(), false), + AccountMeta::new(buffer_address, false), + AccountMeta::new_readonly(sysvar::rent::id(), false), + AccountMeta::new_readonly(sysvar::clock::id(), false), + AccountMeta::new_readonly(system_program::id(), false), + AccountMeta::new_readonly(upgrade_authority_keypair.pubkey(), false), + ], + )], + Some(&mint_keypair.pubkey()), + ); + assert_eq!( + TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature), + bank_client + .send_and_confirm_message(&[&mint_keypair], message) .unwrap_err() .unwrap() ); @@ -1613,7 +1693,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len(), ) @@ -1623,7 +1703,10 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::InvalidAccountData), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .unwrap_err() .unwrap() ); @@ -1638,7 +1721,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance - 1, elf.len(), ) @@ -1648,7 +1731,10 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::ExecutableAccountNotRentExempt), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .unwrap_err() .unwrap() ); @@ -1662,7 +1748,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len(), ) @@ -1678,7 +1764,10 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::ExecutableAccountNotRentExempt), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .unwrap_err() .unwrap() ); @@ -1692,7 +1781,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len(), ) @@ -1708,7 +1797,10 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::AccountDataTooSmall), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .unwrap_err() .unwrap() ); @@ -1727,7 +1819,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len(), ) @@ -1737,7 +1829,10 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::Custom(1)), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .unwrap_err() .unwrap() ); @@ -1756,7 +1851,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len() - 1, ) @@ -1766,7 +1861,10 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::AccountDataTooSmall), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .unwrap_err() .unwrap() ); @@ -1780,7 +1878,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len(), ) @@ -1790,7 +1888,10 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::MissingAccount), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .unwrap_err() .unwrap() ); @@ -1809,7 +1910,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len(), ) @@ -1819,7 +1920,10 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::InvalidAccountData), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .unwrap_err() .unwrap() ); @@ -1833,7 +1937,7 @@ mod tests { ); modified_buffer_account .set_state(&UpgradeableLoaderState::Buffer { - authority_address: None, + authority_address: Some(upgrade_authority_keypair.pubkey()), }) .unwrap(); modified_buffer_account.data[UpgradeableLoaderState::buffer_data_offset().unwrap()..] @@ -1847,7 +1951,7 @@ mod tests { &mint_keypair.pubkey(), &program_keypair.pubkey(), &buffer_address, - None, + &upgrade_authority_keypair.pubkey(), min_program_balance, elf.len(), ) @@ -1857,7 +1961,90 @@ mod tests { assert_eq!( TransactionError::InstructionError(1, InstructionError::InvalidAccountData), bank_client - .send_and_confirm_message(&[&mint_keypair, &program_keypair], message) + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) + .unwrap_err() + .unwrap() + ); + + // Mismatched buffer and program authority + bank.clear_signatures(); + let mut modified_buffer_account = Account::new( + min_programdata_balance, + UpgradeableLoaderState::buffer_len(elf.len()).unwrap(), + &bpf_loader_upgradeable::id(), + ); + modified_buffer_account + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(buffer_address), + }) + .unwrap(); + modified_buffer_account.data[UpgradeableLoaderState::buffer_data_offset().unwrap()..] + .copy_from_slice(&elf); + bank.store_account(&buffer_address, &modified_buffer_account); + bank.store_account(&program_keypair.pubkey(), &Account::default()); + bank.store_account(&programdata_address, &Account::default()); + let message = Message::new( + &bpf_loader_upgradeable::deploy_with_max_program_len( + &mint_keypair.pubkey(), + &program_keypair.pubkey(), + &buffer_address, + &upgrade_authority_keypair.pubkey(), + min_program_balance, + elf.len(), + ) + .unwrap(), + Some(&mint_keypair.pubkey()), + ); + assert_eq!( + TransactionError::InstructionError(1, InstructionError::IncorrectAuthority), + bank_client + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) + .unwrap_err() + .unwrap() + ); + + // Deploy buffer with mismatched None authority + bank.clear_signatures(); + let mut modified_buffer_account = Account::new( + min_programdata_balance, + UpgradeableLoaderState::buffer_len(elf.len()).unwrap(), + &bpf_loader_upgradeable::id(), + ); + modified_buffer_account + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: None, + }) + .unwrap(); + modified_buffer_account.data[UpgradeableLoaderState::buffer_data_offset().unwrap()..] + .copy_from_slice(&elf); + bank.store_account(&buffer_address, &modified_buffer_account); + bank.store_account(&program_keypair.pubkey(), &Account::default()); + bank.store_account(&programdata_address, &Account::default()); + let message = Message::new( + &bpf_loader_upgradeable::deploy_with_max_program_len( + &mint_keypair.pubkey(), + &program_keypair.pubkey(), + &buffer_address, + &upgrade_authority_keypair.pubkey(), + min_program_balance, + elf.len(), + ) + .unwrap(), + Some(&mint_keypair.pubkey()), + ); + assert_eq!( + TransactionError::InstructionError(1, InstructionError::IncorrectAuthority), + bank_client + .send_and_confirm_message( + &[&mint_keypair, &program_keypair, &upgrade_authority_keypair], + message + ) .unwrap_err() .unwrap() ); @@ -1962,7 +2149,7 @@ mod tests { // Case: Success let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2017,7 +2204,7 @@ mod tests { // Case: not upgradable let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2057,7 +2244,7 @@ mod tests { // Case: wrong authority let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2090,7 +2277,7 @@ mod tests { // Case: authority did not sign let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2123,7 +2310,7 @@ mod tests { // Case: Program account not executable let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2157,7 +2344,7 @@ mod tests { // Case: Program account now owned by loader let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2191,7 +2378,7 @@ mod tests { // Case: Program account not writable let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2224,7 +2411,7 @@ mod tests { // Case: Program account not initialized let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2259,46 +2446,9 @@ mod tests { ) ); - // Case: ProgramData account not initialized - let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, - &programdata_address, - &upgrade_authority_address, - slot, - &elf_orig, - &elf_new, - min_program_balance, - min_programdata_balance, - ); - programdata_account - .borrow_mut() - .set_state(&UpgradeableLoaderState::Uninitialized) - .unwrap(); - assert_eq!( - Err(InstructionError::InvalidAccountData), - process_instruction( - &bpf_loader_upgradeable::id(), - &[ - KeyedAccount::new(&programdata_address, false, &programdata_account), - KeyedAccount::new(&program_address, false, &program_account), - KeyedAccount::new(&buffer_address, false, &buffer_account), - KeyedAccount::new(&spill_address, false, &spill_account), - KeyedAccount::new_readonly(&sysvar::rent::id(), false, &rent_account), - KeyedAccount::new_readonly(&sysvar::clock::id(), false, &clock_account), - KeyedAccount::new_readonly( - &upgrade_authority_address, - true, - &upgrade_authority_account - ) - ], - &instruction, - &mut MockInvokeContext::default() - ) - ); - // Case: Program ProgramData account mismatch let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2331,7 +2481,7 @@ mod tests { // Case: Buffer account not initialized let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2368,7 +2518,7 @@ mod tests { // Case: Buffer account too big let (_, program_account, programdata_account, spill_account) = get_accounts( - &buffer_address, + &upgrade_authority_address, &programdata_address, &upgrade_authority_address, slot, @@ -2385,7 +2535,7 @@ mod tests { buffer_account .borrow_mut() .set_state(&UpgradeableLoaderState::Buffer { - authority_address: Some(buffer_address), + authority_address: Some(upgrade_authority_address), }) .unwrap(); assert_eq!( @@ -2411,6 +2561,79 @@ mod tests { ); // Test small buffer account + let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( + &upgrade_authority_address, + &programdata_address, + &upgrade_authority_address, + slot, + &elf_orig, + &elf_new, + min_program_balance, + min_programdata_balance, + ); + buffer_account + .borrow_mut() + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(upgrade_authority_address), + }) + .unwrap(); + buffer_account.borrow_mut().data.truncate(5); + assert_eq!( + Err(InstructionError::InvalidAccountData), + process_instruction( + &bpf_loader_upgradeable::id(), + &[ + KeyedAccount::new(&programdata_address, false, &programdata_account), + KeyedAccount::new(&program_address, false, &program_account), + KeyedAccount::new(&buffer_address, false, &buffer_account), + KeyedAccount::new(&spill_address, false, &spill_account), + KeyedAccount::new_readonly(&sysvar::rent::id(), false, &rent_account), + KeyedAccount::new_readonly(&sysvar::clock::id(), false, &clock_account), + KeyedAccount::new_readonly( + &upgrade_authority_address, + true, + &upgrade_authority_account + ) + ], + &instruction, + &mut MockInvokeContext::default() + ) + ); + + // Case: Mismatched buffer and program authority + let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( + &buffer_address, + &programdata_address, + &upgrade_authority_address, + slot, + &elf_orig, + &elf_new, + min_program_balance, + min_programdata_balance, + ); + assert_eq!( + Err(InstructionError::IncorrectAuthority), + process_instruction( + &bpf_loader_upgradeable::id(), + &[ + KeyedAccount::new(&programdata_address, false, &programdata_account), + KeyedAccount::new(&program_address, false, &program_account), + KeyedAccount::new(&buffer_address, false, &buffer_account), + KeyedAccount::new(&spill_address, false, &spill_account), + KeyedAccount::new_readonly(&sysvar::rent::id(), false, &rent_account), + KeyedAccount::new_readonly(&sysvar::clock::id(), false, &clock_account), + KeyedAccount::new_readonly( + &upgrade_authority_address, + true, + &upgrade_authority_account + ) + ], + &instruction, + &mut MockInvokeContext::default() + ) + ); + + // Case: None buffer authority let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( &buffer_address, &programdata_address, @@ -2427,9 +2650,54 @@ mod tests { authority_address: None, }) .unwrap(); - buffer_account.borrow_mut().data.truncate(5); assert_eq!( - Err(InstructionError::InvalidAccountData), + Err(InstructionError::IncorrectAuthority), + process_instruction( + &bpf_loader_upgradeable::id(), + &[ + KeyedAccount::new(&programdata_address, false, &programdata_account), + KeyedAccount::new(&program_address, false, &program_account), + KeyedAccount::new(&buffer_address, false, &buffer_account), + KeyedAccount::new(&spill_address, false, &spill_account), + KeyedAccount::new_readonly(&sysvar::rent::id(), false, &rent_account), + KeyedAccount::new_readonly(&sysvar::clock::id(), false, &clock_account), + KeyedAccount::new_readonly( + &upgrade_authority_address, + true, + &upgrade_authority_account + ) + ], + &instruction, + &mut MockInvokeContext::default() + ) + ); + + // Case: None buffer and program authority + let (buffer_account, program_account, programdata_account, spill_account) = get_accounts( + &buffer_address, + &programdata_address, + &upgrade_authority_address, + slot, + &elf_orig, + &elf_new, + min_program_balance, + min_programdata_balance, + ); + buffer_account + .borrow_mut() + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: None, + }) + .unwrap(); + programdata_account + .borrow_mut() + .set_state(&UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address: None, + }) + .unwrap(); + assert_eq!( + Err(InstructionError::IncorrectAuthority), process_instruction( &bpf_loader_upgradeable::id(), &[ @@ -2690,7 +2958,7 @@ mod tests { } ); - // Case: Not upgradeable + // Case: New authority required buffer_account .borrow_mut() .set_state(&UpgradeableLoaderState::Buffer { @@ -2698,7 +2966,7 @@ mod tests { }) .unwrap(); assert_eq!( - Ok(()), + Err(InstructionError::IncorrectAuthority), process_instruction( &bpf_loader_upgradeable::id(), &[ @@ -2713,7 +2981,7 @@ mod tests { assert_eq!( state, UpgradeableLoaderState::Buffer { - authority_address: None, + authority_address: Some(authority_address), } ); @@ -2731,6 +2999,11 @@ mod tests { &[ KeyedAccount::new(&buffer_address, false, &buffer_account), KeyedAccount::new_readonly(&authority_address, false, &authority_account), + KeyedAccount::new_readonly( + &new_authority_address, + false, + &new_authority_account + ) ], &instruction, &mut MockInvokeContext::default() @@ -2776,6 +3049,11 @@ mod tests { &[ KeyedAccount::new(&buffer_address, false, &buffer_account), KeyedAccount::new_readonly(&Pubkey::new_unique(), true, &authority_account), + KeyedAccount::new_readonly( + &new_authority_address, + false, + &new_authority_account + ) ], &bincode::serialize(&UpgradeableLoaderInstruction::SetAuthority).unwrap(), &mut MockInvokeContext::default() @@ -2801,6 +3079,26 @@ mod tests { &mut MockInvokeContext::default() ) ); + + // Case: Set to no authority + buffer_account + .borrow_mut() + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(authority_address), + }) + .unwrap(); + assert_eq!( + Err(InstructionError::IncorrectAuthority), + process_instruction( + &bpf_loader_upgradeable::id(), + &[ + KeyedAccount::new(&buffer_address, false, &buffer_account), + KeyedAccount::new_readonly(&authority_address, true, &authority_account), + ], + &instruction, + &mut MockInvokeContext::default() + ) + ); } /// fuzzing utility function diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index dfa0c454f0..cef06f548d 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4583,8 +4583,8 @@ impl Bank { self.rent_collector.rent.burn_percent = 50; // 50% rent burn } - if new_feature_activations.contains(&feature_set::spl_token_v2_self_transfer_fix::id()) { - self.apply_spl_token_v2_self_transfer_fix(); + if new_feature_activations.contains(&feature_set::spl_token_v2_multisig_fix::id()) { + self.apply_spl_token_v2_multisig_fix(); } // Remove me after a while around v1.6 if !self.no_stake_rewrite.load(Relaxed) @@ -4676,7 +4676,7 @@ impl Bank { } } - fn apply_spl_token_v2_self_transfer_fix(&mut self) { + fn apply_spl_token_v2_multisig_fix(&mut self) { if let Some(mut account) = self.get_account(&inline_spl_token_v2_0::id()) { self.capitalization.fetch_sub(account.lamports, Relaxed); account.lamports = 0; @@ -10840,7 +10840,7 @@ pub(crate) mod tests { } #[test] - fn test_spl_token_v2_self_transfer_fix() { + fn test_spl_token_v2_multisig_fix() { let (genesis_config, _mint_keypair) = create_genesis_config(0); let mut bank = Bank::new(&genesis_config); @@ -10855,7 +10855,7 @@ pub(crate) mod tests { assert_eq!(bank.get_balance(&inline_spl_token_v2_0::id()), 100); let original_capitalization = bank.capitalization(); - bank.apply_spl_token_v2_self_transfer_fix(); + bank.apply_spl_token_v2_multisig_fix(); // Account is now empty, and the account lamports were burnt assert_eq!(bank.get_balance(&inline_spl_token_v2_0::id()), 0); diff --git a/runtime/src/loader_utils.rs b/runtime/src/loader_utils.rs index 3f8305d00e..4fa5343e8b 100644 --- a/runtime/src/loader_utils.rs +++ b/runtime/src/loader_utils.rs @@ -58,9 +58,11 @@ pub fn load_buffer_account( bank_client: &T, from_keypair: &Keypair, buffer_keypair: &Keypair, + buffer_authority_keypair: &Keypair, program: &[u8], ) { let buffer_pubkey = buffer_keypair.pubkey(); + let buffer_authority_pubkey = buffer_authority_keypair.pubkey(); bank_client .send_and_confirm_message( @@ -69,7 +71,7 @@ pub fn load_buffer_account( &bpf_loader_upgradeable::create_buffer( &from_keypair.pubkey(), &buffer_pubkey, - Some(&buffer_pubkey), + &buffer_authority_pubkey, 1.max( bank_client .get_minimum_balance_for_rent_exemption(program.len()) @@ -89,14 +91,14 @@ pub fn load_buffer_account( let message = Message::new( &[bpf_loader_upgradeable::write( &buffer_pubkey, - None, + &buffer_authority_pubkey, offset, chunk.to_vec(), )], Some(&from_keypair.pubkey()), ); bank_client - .send_and_confirm_message(&[from_keypair, &buffer_keypair], message) + .send_and_confirm_message(&[from_keypair, &buffer_authority_keypair], message) .unwrap(); offset += chunk_size as u32; } @@ -113,14 +115,20 @@ pub fn load_upgradeable_program( let program_pubkey = executable_keypair.pubkey(); let authority_pubkey = authority_keypair.pubkey(); - load_buffer_account(bank_client, &from_keypair, buffer_keypair, &program); + load_buffer_account( + bank_client, + &from_keypair, + buffer_keypair, + authority_keypair, + &program, + ); let message = Message::new( &bpf_loader_upgradeable::deploy_with_max_program_len( &from_keypair.pubkey(), &program_pubkey, &buffer_keypair.pubkey(), - Some(&authority_pubkey), + &authority_pubkey, 1.max( bank_client .get_minimum_balance_for_rent_exemption( @@ -134,7 +142,10 @@ pub fn load_upgradeable_program( Some(&from_keypair.pubkey()), ); bank_client - .send_and_confirm_message(&[from_keypair, &executable_keypair], message) + .send_and_confirm_message( + &[from_keypair, &executable_keypair, &authority_keypair], + message, + ) .unwrap(); } diff --git a/sdk/program/src/bpf_loader_upgradeable.rs b/sdk/program/src/bpf_loader_upgradeable.rs index 3453946bb1..6ed8c467d0 100644 --- a/sdk/program/src/bpf_loader_upgradeable.rs +++ b/sdk/program/src/bpf_loader_upgradeable.rs @@ -87,14 +87,10 @@ impl UpgradeableLoaderState { pub fn create_buffer( payer_address: &Pubkey, buffer_address: &Pubkey, - authority_address: Option<&Pubkey>, + authority_address: &Pubkey, lamports: u64, program_len: usize, ) -> Result, InstructionError> { - let mut metas = vec![AccountMeta::new(*buffer_address, false)]; - if let Some(authority_address) = authority_address { - metas.push(AccountMeta::new(*authority_address, false)); - } Ok(vec![ system_instruction::create_account( payer_address, @@ -103,7 +99,14 @@ pub fn create_buffer( UpgradeableLoaderState::buffer_len(program_len)? as u64, &id(), ), - Instruction::new(id(), &UpgradeableLoaderInstruction::InitializeBuffer, metas), + Instruction::new( + id(), + &UpgradeableLoaderInstruction::InitializeBuffer, + vec![ + AccountMeta::new(*buffer_address, false), + AccountMeta::new_readonly(*authority_address, false), + ], + ), ]) } @@ -111,21 +114,17 @@ pub fn create_buffer( /// buffer account. pub fn write( buffer_address: &Pubkey, - authority_address: Option<&Pubkey>, + authority_address: &Pubkey, offset: u32, bytes: Vec, ) -> Instruction { - let mut metas = vec![ - AccountMeta::new(*buffer_address, false), - AccountMeta::new(*buffer_address, true), - ]; - if let Some(authority_address) = authority_address { - metas[1] = AccountMeta::new(*authority_address, true); - } Instruction::new( id(), &UpgradeableLoaderInstruction::Write { offset, bytes }, - metas, + vec![ + AccountMeta::new(*buffer_address, false), + AccountMeta::new_readonly(*authority_address, true), + ], ) } @@ -136,23 +135,11 @@ pub fn deploy_with_max_program_len( payer_address: &Pubkey, program_address: &Pubkey, buffer_address: &Pubkey, - upgrade_authority_address: Option<&Pubkey>, + upgrade_authority_address: &Pubkey, program_lamports: u64, max_data_len: usize, ) -> Result, InstructionError> { let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); - let mut metas = vec![ - AccountMeta::new(*payer_address, true), - AccountMeta::new(programdata_address, false), - AccountMeta::new(*program_address, false), - AccountMeta::new(*buffer_address, false), - AccountMeta::new_readonly(sysvar::rent::id(), false), - AccountMeta::new_readonly(sysvar::clock::id(), false), - AccountMeta::new_readonly(crate::system_program::id(), false), - ]; - if let Some(address) = upgrade_authority_address { - metas.push(AccountMeta::new_readonly(*address, false)); - } Ok(vec![ system_instruction::create_account( payer_address, @@ -164,7 +151,16 @@ pub fn deploy_with_max_program_len( Instruction::new( id(), &UpgradeableLoaderInstruction::DeployWithMaxDataLen { max_data_len }, - metas, + vec![ + AccountMeta::new(*payer_address, true), + AccountMeta::new(programdata_address, false), + AccountMeta::new(*program_address, false), + AccountMeta::new(*buffer_address, false), + AccountMeta::new_readonly(sysvar::rent::id(), false), + AccountMeta::new_readonly(sysvar::clock::id(), false), + AccountMeta::new_readonly(crate::system_program::id(), false), + AccountMeta::new_readonly(*upgrade_authority_address, true), + ], ), ]) } @@ -200,16 +196,17 @@ pub fn is_upgrade_instruction(instruction_data: &[u8]) -> bool { pub fn set_buffer_authority( buffer_address: &Pubkey, current_authority_address: &Pubkey, - new_authority_address: Option<&Pubkey>, + new_authority_address: &Pubkey, ) -> Instruction { - let mut metas = vec![ - AccountMeta::new(*buffer_address, false), - AccountMeta::new_readonly(*current_authority_address, true), - ]; - if let Some(address) = new_authority_address { - metas.push(AccountMeta::new_readonly(*address, false)); - } - Instruction::new(id(), &UpgradeableLoaderInstruction::SetAuthority, metas) + Instruction::new( + id(), + &UpgradeableLoaderInstruction::SetAuthority, + 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. diff --git a/sdk/program/src/loader_upgradeable_instruction.rs b/sdk/program/src/loader_upgradeable_instruction.rs index 4f58f0e85d..9adfbd6962 100644 --- a/sdk/program/src/loader_upgradeable_instruction.rs +++ b/sdk/program/src/loader_upgradeable_instruction.rs @@ -36,7 +36,7 @@ pub enum UpgradeableLoaderInstruction { /// Deploy an executable program. /// /// A program consists of a Program and ProgramData account pair. - /// - The Program account's address will serve as the program id any + /// - The Program account's address will serve as the program id for any /// instructions that execute this program. /// - The ProgramData account will remain mutable by the loader only and /// holds the program data and authority information. The ProgramData @@ -54,21 +54,21 @@ pub enum UpgradeableLoaderInstruction { /// The `DeployWithMaxDataLen` instruction does not require the ProgramData /// account be a signer and therefore MUST be included within the same /// Transaction as the system program's `CreateAccount` instruction that - /// creates the Program account. Otherwise another party may initialize - /// the account. + /// creates the Program account. Otherwise another party may initialize the + /// account. /// /// # Account references - /// 0. [Signer] The payer account that will pay to create the ProgramData + /// 0. [signer] The payer account that will pay to create the ProgramData /// account. /// 1. [writable] The uninitialized ProgramData account. /// 2. [writable] The uninitialized Program account. /// 3. [writable] The Buffer account where the program data has been - /// written. + /// written. The buffer account's authority must match the program's + /// authority /// 4. [] Rent sysvar. /// 5. [] Clock sysvar. /// 6. [] System program (`solana_sdk::system_program::id()`). - /// 7. [] The program's authority, optional, if omitted then the program - /// will no longer upgradeable. + /// 7. [signer] The program's authority DeployWithMaxDataLen { /// Maximum length that the program can be upgraded to. max_data_len: usize, @@ -81,14 +81,15 @@ pub enum UpgradeableLoaderInstruction { /// /// The Buffer account must contain sufficient lamports to fund the /// ProgramData account to be rent-exempt, any additional lamports left over - /// will be transferred to the spill, account leaving the Buffer account + /// will be transferred to the spill account, leaving the Buffer account /// balance at zero. /// /// # Account references /// 0. [writable] The ProgramData account. /// 1. [writable] The Program account. /// 2. [writable] The Buffer account where the program data has been - /// written. + /// written. The buffer account's authority must match the program's + /// authority /// 3. [writable] The spill account. /// 4. [] Rent sysvar. /// 5. [] Clock sysvar. diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 375fe5bc95..4887026179 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -162,8 +162,8 @@ pub mod prevent_upgrade_and_invoke { solana_sdk::declare_id!("BiNjYd8jCYDgAwMqP91uwZs6skWpuHtKrZbckuKESs8N"); } -pub mod spl_token_v2_self_transfer_fix { - solana_sdk::declare_id!("BL99GYhdjjcv6ys22C9wPgn2aTVERDbPHHo4NbS3hgp7"); +pub mod matching_buffer_upgrade_authorities { + solana_sdk::declare_id!("B5PSjDEJvKJEUQSL7q94N7XCEoWJCYum8XfUg7yuugUU"); } lazy_static! { @@ -205,7 +205,7 @@ lazy_static! { (prevent_upgrade_and_invoke::id(), "Prevent upgrade and invoke in same tx batch"), (full_inflation::candidate_example::vote::id(), "Community vote allowing candidate_example to enable full inflation"), (full_inflation::candidate_example::enable::id(), "Full inflation enabled by candidate_example"), - (spl_token_v2_self_transfer_fix::id(), "spl-token self-transfer fix"), + (matching_buffer_upgrade_authorities::id(), "Upgradeable buffer and program authorities must match"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()