Skip to content

Commit

Permalink
cli: Use a better amount during transaction simulation (#2709)
Browse files Browse the repository at this point in the history
* cli: Use a better amount during transaction simulation

#### Problem

There are issues with the current strategy of simulating a test
transaction in the `resolve_spend_message`. If the transaction is
creating an account, the test transaction will fail because not enough
lamports are sent to the destination.

#### Summary of changes

It's a tricky situation in which we can't always be correct, since
there's a chicken-and-egg situation with calculating the fee for spend
variants of `All` and `RentExempt` if the sender and the fee payer are
the same account.

To get the simulation correct in almost all situations, we simulate with
the real transfer amount. But if the fee payer is the sender, we
simulate with `0`. But we also add a new variant on `SpendAmount` to
cover some minimum amount required that must be transferred. Currently,
the only situations in which we have an issue are:

* creating a nonce account
* creating a stake account
* transferring SOL

Those first two have a minimum requirement, so use that. The third works
fine with a 0 amount, since it's just a SOL transfer.

* Address feedback
  • Loading branch information
joncinque authored Aug 23, 2024
1 parent 2ff3814 commit 93341c6
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 6 deletions.
10 changes: 8 additions & 2 deletions cli/src/nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ pub fn process_create_nonce_account(
seed: Option<String>,
nonce_authority: Option<Pubkey>,
memo: Option<&String>,
amount: SpendAmount,
mut amount: SpendAmount,
compute_unit_price: Option<u64>,
) -> ProcessResult {
let nonce_account_pubkey = config.signers[nonce_account].pubkey();
Expand All @@ -460,6 +460,13 @@ pub fn process_create_nonce_account(
(&nonce_account_address, "nonce_account".to_string()),
)?;

let minimum_balance = rpc_client.get_minimum_balance_for_rent_exemption(State::size())?;
if amount == SpendAmount::All {
amount = SpendAmount::AllForAccountCreation {
create_account_min_balance: minimum_balance,
};
}

let nonce_authority = nonce_authority.unwrap_or_else(|| config.signers[0].pubkey());

let compute_unit_limit = ComputeUnitLimit::Default;
Expand Down Expand Up @@ -516,7 +523,6 @@ pub fn process_create_nonce_account(
return Err(CliError::BadParameter(err_msg).into());
}

let minimum_balance = rpc_client.get_minimum_balance_for_rent_exemption(State::size())?;
if lamports < minimum_balance {
return Err(CliError::BadParameter(format!(
"need at least {minimum_balance} lamports for nonce account to be rent exempt, \
Expand Down
33 changes: 31 additions & 2 deletions cli/src/spend_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub enum SpendAmount {
All,
Some(u64),
RentExempt,
AllForAccountCreation { create_account_min_balance: u64 },
}

impl Default for SpendAmount {
Expand Down Expand Up @@ -166,7 +167,35 @@ where
{
let (fee, compute_unit_info) = match blockhash {
Some(blockhash) => {
let mut dummy_message = build_message(0);
// If the from account is the same as the fee payer, it's impossible
// to give a correct amount for the simulation with `SpendAmount::All`
// or `SpendAmount::RentExempt`.
// To know how much to transfer, we need to know the transaction fee,
// but the transaction fee is dependent on the amount of compute
// units used, which requires simulation.
// To get around this limitation, we simulate against an amount of
// `0`, since there are few situations in which `SpendAmount` can
// be `All` or `RentExempt` *and also* the from account is the fee
// payer.
let lamports = if from_pubkey == fee_pubkey {
match amount {
SpendAmount::Some(lamports) => lamports,
SpendAmount::AllForAccountCreation {
create_account_min_balance,
} => create_account_min_balance,
SpendAmount::All | SpendAmount::RentExempt => 0,
}
} else {
match amount {
SpendAmount::Some(lamports) => lamports,
SpendAmount::AllForAccountCreation { .. } | SpendAmount::All => from_balance,
SpendAmount::RentExempt => {
from_balance.saturating_sub(from_rent_exempt_minimum)
}
}
};
let mut dummy_message = build_message(lamports);

dummy_message.recent_blockhash = *blockhash;
let compute_unit_info = if compute_unit_limit == ComputeUnitLimit::Simulated {
// Simulate for correct compute units
Expand Down Expand Up @@ -196,7 +225,7 @@ where
fee,
},
),
SpendAmount::All => {
SpendAmount::All | SpendAmount::AllForAccountCreation { .. } => {
let lamports = if from_pubkey == fee_pubkey {
from_balance.saturating_sub(fee)
} else {
Expand Down
11 changes: 9 additions & 2 deletions cli/src/stake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ pub fn process_create_stake_account(
withdrawer: &Option<Pubkey>,
withdrawer_signer: Option<SignerIndex>,
lockup: &Lockup,
amount: SpendAmount,
mut amount: SpendAmount,
sign_only: bool,
dump_transaction_message: bool,
blockhash_query: &BlockhashQuery,
Expand Down Expand Up @@ -1454,6 +1454,14 @@ pub fn process_create_stake_account(

let recent_blockhash = blockhash_query.get_blockhash(rpc_client, config.commitment)?;

if !sign_only && amount == SpendAmount::All {
let minimum_balance =
rpc_client.get_minimum_balance_for_rent_exemption(StakeStateV2::size_of())?;
amount = SpendAmount::AllForAccountCreation {
create_account_min_balance: minimum_balance,
};
}

let (message, lamports) = resolve_spend_tx_and_check_account_balances(
rpc_client,
sign_only,
Expand All @@ -1478,7 +1486,6 @@ pub fn process_create_stake_account(

let minimum_balance =
rpc_client.get_minimum_balance_for_rent_exemption(StakeStateV2::size_of())?;

if lamports < minimum_balance {
return Err(CliError::BadParameter(format!(
"need at least {minimum_balance} lamports for stake account to be rent exempt, \
Expand Down

0 comments on commit 93341c6

Please sign in to comment.