From b52ec20625d1af27812751b4cab2968976ada15f Mon Sep 17 00:00:00 2001 From: Jon C Date: Thu, 27 Jun 2024 14:04:16 +0200 Subject: [PATCH 1/4] cli: Plumb down `ComputeUnitLimit` to resolve tx --- clap-utils/src/compute_budget.rs | 1 + cli/src/cluster_query.rs | 4 +++- cli/src/feature.rs | 4 +++- cli/src/nonce.rs | 6 ++++-- cli/src/spend_utils.rs | 10 +++++++++- cli/src/stake.rs | 8 ++++++-- cli/src/validator_info.rs | 6 ++++-- cli/src/vote.rs | 8 ++++++-- cli/src/wallet.rs | 6 ++++-- 9 files changed, 40 insertions(+), 13 deletions(-) diff --git a/clap-utils/src/compute_budget.rs b/clap-utils/src/compute_budget.rs index be8142738368eb..24f64ec13b091f 100644 --- a/clap-utils/src/compute_budget.rs +++ b/clap-utils/src/compute_budget.rs @@ -33,6 +33,7 @@ pub fn compute_unit_limit_arg<'a, 'b>() -> Arg<'a, 'b> { .help(COMPUTE_UNIT_LIMIT_ARG.help) } +#[derive(Clone, Copy, Debug, PartialEq)] pub enum ComputeUnitLimit { /// Do not include a compute unit limit instruction, which will give the /// transaction a compute unit limit of: diff --git a/cli/src/cluster_query.rs b/cli/src/cluster_query.rs index 6d2bd30714dad6..bc2388b335f19d 100644 --- a/cli/src/cluster_query.rs +++ b/cli/src/cluster_query.rs @@ -1520,6 +1520,7 @@ pub fn process_ping( let to = config.signers[0].pubkey(); lamports = lamports.saturating_add(1); + let compute_unit_limit = ComputeUnitLimit::Default; let build_message = |lamports| { let ixs = vec![system_instruction::transfer( &config.signers[0].pubkey(), @@ -1528,7 +1529,7 @@ pub fn process_ping( )] .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }); Message::new(&ixs, Some(&config.signers[0].pubkey())) }; @@ -1538,6 +1539,7 @@ pub fn process_ping( SpendAmount::Some(lamports), &blockhash, &config.signers[0].pubkey(), + compute_unit_limit, build_message, config.commitment, )?; diff --git a/cli/src/feature.rs b/cli/src/feature.rs index c6e397d2e01088..66696b11c74190 100644 --- a/cli/src/feature.rs +++ b/cli/src/feature.rs @@ -10,7 +10,8 @@ use { console::style, serde::{Deserialize, Serialize}, solana_clap_utils::{ - fee_payer::*, hidden_unless_forced, input_parsers::*, input_validators::*, keypair::*, + compute_budget::ComputeUnitLimit, fee_payer::*, hidden_unless_forced, input_parsers::*, + input_validators::*, keypair::*, }, solana_cli_output::{cli_version::CliVersion, QuietDisplay, VerboseDisplay}, solana_remote_wallet::remote_wallet::RemoteWalletManager, @@ -972,6 +973,7 @@ fn process_activate( SpendAmount::Some(rent), &blockhash, &fee_payer.pubkey(), + ComputeUnitLimit::Default, |lamports| { Message::new( &feature::activate_with_lamports(&feature_id, &fee_payer.pubkey(), lamports), diff --git a/cli/src/nonce.rs b/cli/src/nonce.rs index dc5664607c5ae5..0f0bc15aa9295a 100644 --- a/cli/src/nonce.rs +++ b/cli/src/nonce.rs @@ -462,6 +462,7 @@ pub fn process_create_nonce_account( let nonce_authority = nonce_authority.unwrap_or_else(|| config.signers[0].pubkey()); + let compute_unit_limit = ComputeUnitLimit::Default; let build_message = |lamports| { let ixs = if let Some(seed) = seed.clone() { create_nonce_account_with_seed( @@ -475,7 +476,7 @@ pub fn process_create_nonce_account( .with_memo(memo) .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }) } else { create_nonce_account( @@ -487,7 +488,7 @@ pub fn process_create_nonce_account( .with_memo(memo) .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }) }; Message::new(&ixs, Some(&config.signers[0].pubkey())) @@ -501,6 +502,7 @@ pub fn process_create_nonce_account( amount, &latest_blockhash, &config.signers[0].pubkey(), + compute_unit_limit, build_message, config.commitment, )?; diff --git a/cli/src/spend_utils.rs b/cli/src/spend_utils.rs index 3868fb6160c5ed..2d4e2893aa8516 100644 --- a/cli/src/spend_utils.rs +++ b/cli/src/spend_utils.rs @@ -4,7 +4,9 @@ use { cli::CliError, }, clap::ArgMatches, - solana_clap_utils::{input_parsers::lamports_of_sol, offline::SIGN_ONLY_ARG}, + solana_clap_utils::{ + compute_budget::ComputeUnitLimit, input_parsers::lamports_of_sol, offline::SIGN_ONLY_ARG, + }, solana_rpc_client::rpc_client::RpcClient, solana_sdk::{ commitment_config::CommitmentConfig, hash::Hash, message::Message, @@ -52,6 +54,7 @@ pub fn resolve_spend_tx_and_check_account_balance( amount: SpendAmount, blockhash: &Hash, from_pubkey: &Pubkey, + compute_unit_limit: ComputeUnitLimit, build_message: F, commitment: CommitmentConfig, ) -> Result<(Message, u64), CliError> @@ -65,6 +68,7 @@ where blockhash, from_pubkey, from_pubkey, + compute_unit_limit, build_message, commitment, ) @@ -77,6 +81,7 @@ pub fn resolve_spend_tx_and_check_account_balances( blockhash: &Hash, from_pubkey: &Pubkey, fee_pubkey: &Pubkey, + compute_unit_limit: ComputeUnitLimit, build_message: F, commitment: CommitmentConfig, ) -> Result<(Message, u64), CliError> @@ -92,6 +97,7 @@ where from_pubkey, fee_pubkey, 0, + compute_unit_limit, build_message, )?; Ok((message, spend)) @@ -113,6 +119,7 @@ where from_pubkey, fee_pubkey, from_rent_exempt_minimum, + compute_unit_limit, build_message, )?; if from_pubkey == fee_pubkey { @@ -150,6 +157,7 @@ fn resolve_spend_message( from_pubkey: &Pubkey, fee_pubkey: &Pubkey, from_rent_exempt_minimum: u64, + _compute_unit_limit: ComputeUnitLimit, build_message: F, ) -> Result<(Message, SpendAndFee), CliError> where diff --git a/cli/src/stake.rs b/cli/src/stake.rs index 3c94ffb9507903..4d2b69bcc626c8 100644 --- a/cli/src/stake.rs +++ b/cli/src/stake.rs @@ -1437,6 +1437,7 @@ pub fn process_create_stake_account( let fee_payer = config.signers[fee_payer]; let nonce_authority = config.signers[nonce_authority]; + let compute_unit_limit = ComputeUnitLimit::Default; let build_message = |lamports| { let authorized = Authorized { staker: staker.unwrap_or(from.pubkey()), @@ -1480,7 +1481,7 @@ pub fn process_create_stake_account( .with_memo(memo) .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }); if let Some(nonce_account) = &nonce_account { Message::new_with_nonce( @@ -1503,6 +1504,7 @@ pub fn process_create_stake_account( &recent_blockhash, &from.pubkey(), &fee_payer.pubkey(), + compute_unit_limit, build_message, config.commitment, )?; @@ -1869,6 +1871,7 @@ pub fn process_withdraw_stake( let fee_payer = config.signers[fee_payer]; let nonce_authority = config.signers[nonce_authority]; + let compute_unit_limit = ComputeUnitLimit::Default; let build_message = |lamports| { let ixs = vec![stake_instruction::withdraw( &stake_account_address, @@ -1880,7 +1883,7 @@ pub fn process_withdraw_stake( .with_memo(memo) .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }); if let Some(nonce_account) = &nonce_account { @@ -1902,6 +1905,7 @@ pub fn process_withdraw_stake( &recent_blockhash, &stake_account_address, &fee_payer.pubkey(), + compute_unit_limit, build_message, config.commitment, )?; diff --git a/cli/src/validator_info.rs b/cli/src/validator_info.rs index f4a3af3b3af49d..e50d74b2d4359f 100644 --- a/cli/src/validator_info.rs +++ b/cli/src/validator_info.rs @@ -349,6 +349,7 @@ pub fn process_set_validator_info( vec![config.signers[0]] }; + let compute_unit_limit = ComputeUnitLimit::Default; let build_message = |lamports| { let keys = keys.clone(); if balance == 0 { @@ -364,7 +365,7 @@ pub fn process_set_validator_info( ) .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }); instructions.extend_from_slice(&[config_instruction::store( &info_pubkey, @@ -387,7 +388,7 @@ pub fn process_set_validator_info( )] .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }); Message::new(&instructions, Some(&config.signers[0].pubkey())) } @@ -401,6 +402,7 @@ pub fn process_set_validator_info( SpendAmount::Some(lamports), &latest_blockhash, &config.signers[0].pubkey(), + compute_unit_limit, build_message, config.commitment, )?; diff --git a/cli/src/vote.rs b/cli/src/vote.rs index f89740f7a6f8f0..0dfb9c2887d05f 100644 --- a/cli/src/vote.rs +++ b/cli/src/vote.rs @@ -827,6 +827,7 @@ pub fn process_create_vote_account( .map_or(false, |feature| feature.activated_at.is_some()); let space = VoteStateVersions::vote_state_size_of(is_feature_active) as u64; + let compute_unit_limit = ComputeUnitLimit::Default; let build_message = |lamports| { let vote_init = VoteInit { node_pubkey: identity_pubkey, @@ -855,7 +856,7 @@ pub fn process_create_vote_account( .with_memo(memo) .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }); if let Some(nonce_account) = &nonce_account { @@ -879,6 +880,7 @@ pub fn process_create_vote_account( &recent_blockhash, &config.signers[0].pubkey(), &fee_payer.pubkey(), + compute_unit_limit, build_message, config.commitment, )?; @@ -1322,6 +1324,7 @@ pub fn process_withdraw_from_vote_account( let fee_payer = config.signers[fee_payer]; let nonce_authority = config.signers[nonce_authority]; + let compute_unit_limit = ComputeUnitLimit::Default; let build_message = |lamports| { let ixs = vec![withdraw( vote_account_pubkey, @@ -1332,7 +1335,7 @@ pub fn process_withdraw_from_vote_account( .with_memo(memo) .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }); if let Some(nonce_account) = &nonce_account { @@ -1354,6 +1357,7 @@ pub fn process_withdraw_from_vote_account( &recent_blockhash, vote_account_pubkey, &fee_payer.pubkey(), + compute_unit_limit, build_message, config.commitment, )?; diff --git a/cli/src/wallet.rs b/cli/src/wallet.rs index 97dd8c6b567212..3589f0d5cb7096 100644 --- a/cli/src/wallet.rs +++ b/cli/src/wallet.rs @@ -911,6 +911,7 @@ pub fn process_transfer( None }; + let compute_unit_limit = ComputeUnitLimit::Default; let build_message = |lamports| { let ixs = if let Some((base_pubkey, seed, program_id, from_pubkey)) = with_seed.as_ref() { vec![system_instruction::transfer_with_seed( @@ -924,14 +925,14 @@ pub fn process_transfer( .with_memo(memo) .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }) } else { vec![system_instruction::transfer(&from_pubkey, to, lamports)] .with_memo(memo) .with_compute_unit_config(&ComputeUnitConfig { compute_unit_price, - compute_unit_limit: ComputeUnitLimit::Default, + compute_unit_limit, }) }; @@ -954,6 +955,7 @@ pub fn process_transfer( &recent_blockhash, &from_pubkey, &fee_payer.pubkey(), + compute_unit_limit, build_message, config.commitment, )?; From 17a97df8d6acd81a2ca98425ac5f27e752414c74 Mon Sep 17 00:00:00 2001 From: Jon C Date: Thu, 27 Jun 2024 14:19:51 +0200 Subject: [PATCH 2/4] Use simulated compute units in transaction --- cli/src/spend_utils.rs | 43 +++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/cli/src/spend_utils.rs b/cli/src/spend_utils.rs index 2d4e2893aa8516..a03e351de26862 100644 --- a/cli/src/spend_utils.rs +++ b/cli/src/spend_utils.rs @@ -2,6 +2,7 @@ use { crate::{ checks::{check_account_for_balance_with_commitment, get_fee_for_messages}, cli::CliError, + compute_budget::{simulate_and_update_compute_unit_limit, UpdateComputeUnitLimitResult}, }, clap::ArgMatches, solana_clap_utils::{ @@ -157,42 +158,57 @@ fn resolve_spend_message( from_pubkey: &Pubkey, fee_pubkey: &Pubkey, from_rent_exempt_minimum: u64, - _compute_unit_limit: ComputeUnitLimit, + compute_unit_limit: ComputeUnitLimit, build_message: F, ) -> Result<(Message, SpendAndFee), CliError> where F: Fn(u64) -> Message, { - let fee = match blockhash { + let (fee, compute_unit_info) = match blockhash { Some(blockhash) => { let mut dummy_message = build_message(0); dummy_message.recent_blockhash = *blockhash; - get_fee_for_messages(rpc_client, &[&dummy_message])? + let compute_unit_info = if compute_unit_limit == ComputeUnitLimit::Simulated { + // Simulate for correct compute units + if let UpdateComputeUnitLimitResult::UpdatedInstructionIndex(ix_index) = + simulate_and_update_compute_unit_limit(rpc_client, &mut dummy_message)? + { + Some((ix_index, dummy_message.instructions[ix_index].data.clone())) + } else { + None + } + } else { + None + }; + ( + get_fee_for_messages(rpc_client, &[&dummy_message])?, + compute_unit_info, + ) } - None => 0, // Offline, cannot calculate fee + None => (0, None), // Offline, cannot calculate fee }; - match amount { - SpendAmount::Some(lamports) => Ok(( + let (mut message, spend_and_fee) = match amount { + SpendAmount::Some(lamports) => ( build_message(lamports), SpendAndFee { spend: lamports, fee, }, - )), + ), SpendAmount::All => { let lamports = if from_pubkey == fee_pubkey { from_balance.saturating_sub(fee) } else { from_balance }; - Ok(( + ( build_message(lamports), SpendAndFee { spend: lamports, fee, }, - )) + ) } SpendAmount::RentExempt => { let mut lamports = if from_pubkey == fee_pubkey { @@ -201,13 +217,18 @@ where from_balance }; lamports = lamports.saturating_sub(from_rent_exempt_minimum); - Ok(( + ( build_message(lamports), SpendAndFee { spend: lamports, fee, }, - )) + ) } + }; + // After build message, update with correct compute units + if let Some((ix_index, ix_data)) = compute_unit_info { + message.instructions[ix_index].data = ix_data; } + Ok((message, spend_and_fee)) } From 663216268acf7a8962f0dc174a7f8c72584988a7 Mon Sep 17 00:00:00 2001 From: Jon C Date: Thu, 27 Jun 2024 14:20:09 +0200 Subject: [PATCH 3/4] wallet: Use simulated compute units for non-nonce transactions --- cli/src/wallet.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cli/src/wallet.rs b/cli/src/wallet.rs index 3589f0d5cb7096..728a529af246c9 100644 --- a/cli/src/wallet.rs +++ b/cli/src/wallet.rs @@ -911,7 +911,11 @@ pub fn process_transfer( None }; - let compute_unit_limit = ComputeUnitLimit::Default; + let compute_unit_limit = if nonce_account.is_some() { + ComputeUnitLimit::Default + } else { + ComputeUnitLimit::Simulated + }; let build_message = |lamports| { let ixs = if let Some((base_pubkey, seed, program_id, from_pubkey)) = with_seed.as_ref() { vec![system_instruction::transfer_with_seed( From d1717b7acd8054284fcc7a07bee64de2c5b37fdf Mon Sep 17 00:00:00 2001 From: Jon C Date: Sat, 29 Jun 2024 00:24:00 +0200 Subject: [PATCH 4/4] Modify transfer test --- cli/tests/transfer.rs | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/cli/tests/transfer.rs b/cli/tests/transfer.rs index e5ebac60976814..b1a750972543e1 100644 --- a/cli/tests/transfer.rs +++ b/cli/tests/transfer.rs @@ -12,15 +12,18 @@ use { solana_rpc_client_nonce_utils::blockhash_query::{self, BlockhashQuery}, solana_sdk::{ commitment_config::CommitmentConfig, + compute_budget::ComputeBudgetInstruction, fee::FeeStructure, + message::Message, native_token::sol_to_lamports, nonce::State as NonceState, pubkey::Pubkey, signature::{keypair_from_seed, Keypair, NullSigner, Signer}, - stake, + stake, system_instruction, }, solana_streamer::socket::SocketAddrSpace, solana_test_validator::TestValidator, + test_case::test_case, }; #[test] @@ -474,16 +477,17 @@ fn test_transfer_multisession_signing() { check_balance!(sol_to_lamports(42.0), &rpc_client, &to_pubkey); } -#[test] -fn test_transfer_all() { +#[test_case(None; "default")] +#[test_case(Some(100_000); "with_compute_unit_price")] +fn test_transfer_all(compute_unit_price: Option) { solana_logger::setup(); - let fee = FeeStructure::default().get_max_fee(1, 0); + let lamports_per_signature = FeeStructure::default().get_max_fee(1, 0); let mint_keypair = Keypair::new(); let mint_pubkey = mint_keypair.pubkey(); let faucet_addr = run_local_faucet(mint_keypair, None); let test_validator = TestValidator::with_custom_fees( mint_pubkey, - fee, + lamports_per_signature, Some(faucet_addr), SocketAddrSpace::Unspecified, ); @@ -492,13 +496,36 @@ fn test_transfer_all() { RpcClient::new_with_commitment(test_validator.rpc_url(), CommitmentConfig::processed()); let default_signer = Keypair::new(); + let recipient_pubkey = Pubkey::from([1u8; 32]); + + let fee = { + let mut instructions = vec![system_instruction::transfer( + &default_signer.pubkey(), + &recipient_pubkey, + 0, + )]; + if let Some(compute_unit_price) = compute_unit_price { + // This is brittle and will need to be updated if the compute unit + // limit for the system program or compute budget program are changed, + // or if they're converted to BPF. + // See `solana_system_program::system_processor::DEFAULT_COMPUTE_UNITS` + // and `solana_compute_budget_program::DEFAULT_COMPUTE_UNITS` + instructions.push(ComputeBudgetInstruction::set_compute_unit_limit(450)); + instructions.push(ComputeBudgetInstruction::set_compute_unit_price( + compute_unit_price, + )); + } + let blockhash = rpc_client.get_latest_blockhash().unwrap(); + let sample_message = + Message::new_with_blockhash(&instructions, Some(&default_signer.pubkey()), &blockhash); + rpc_client.get_fee_for_message(&sample_message).unwrap() + }; let mut config = CliConfig::recent_for_tests(); config.json_rpc_url = test_validator.rpc_url(); config.signers = vec![&default_signer]; let sender_pubkey = config.signers[0].pubkey(); - let recipient_pubkey = Pubkey::from([1u8; 32]); request_and_confirm_airdrop(&rpc_client, &config, &sender_pubkey, 500_000).unwrap(); check_balance!(500_000, &rpc_client, &sender_pubkey); @@ -522,7 +549,7 @@ fn test_transfer_all() { fee_payer: 0, derived_address_seed: None, derived_address_program_id: None, - compute_unit_price: None, + compute_unit_price, }; process_command(&config).unwrap(); check_balance!(0, &rpc_client, &sender_pubkey);