Remove blockhash from fee calculation (#20641)

This commit is contained in:
Jack May 2021-10-13 13:10:58 -07:00 committed by GitHub
parent 149d224557
commit da45be366a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 53 additions and 119 deletions

View File

@ -271,7 +271,7 @@ fn run_accounts_bench(
}
let fee = client
.get_fee_for_message(&blockhash, &message)
.get_fee_for_message(&message)
.expect("get_fee_for_message");
let lamports = min_balance + fee;

View File

@ -896,10 +896,7 @@ pub fn generate_and_fund_keypairs<T: 'static + Client + Send + Sync>(
)],
None,
);
let blockhash = client.get_latest_blockhash().unwrap();
let max_fee = client
.get_fee_for_message(&blockhash, &single_sig_message)
.unwrap();
let max_fee = client.get_fee_for_message(&single_sig_message).unwrap();
let extra_fees = extra * max_fee;
let total_keypairs = keypairs.len() as u64 + 1; // Add one for funding keypair
let total = lamports_per_account * total_keypairs + extra_fees;

View File

@ -4,30 +4,27 @@ use solana_client::{
rpc_client::RpcClient,
};
use solana_sdk::{
commitment_config::CommitmentConfig, hash::Hash, message::Message,
native_token::lamports_to_sol, pubkey::Pubkey,
commitment_config::CommitmentConfig, message::Message, native_token::lamports_to_sol,
pubkey::Pubkey,
};
pub fn check_account_for_fee(
rpc_client: &RpcClient,
account_pubkey: &Pubkey,
blockhash: &Hash,
message: &Message,
) -> Result<(), CliError> {
check_account_for_multiple_fees(rpc_client, account_pubkey, blockhash, &[message])
check_account_for_multiple_fees(rpc_client, account_pubkey, &[message])
}
pub fn check_account_for_fee_with_commitment(
rpc_client: &RpcClient,
account_pubkey: &Pubkey,
blockhash: &Hash,
message: &Message,
commitment: CommitmentConfig,
) -> Result<(), CliError> {
check_account_for_multiple_fees_with_commitment(
rpc_client,
account_pubkey,
blockhash,
&[message],
commitment,
)
@ -36,13 +33,11 @@ pub fn check_account_for_fee_with_commitment(
pub fn check_account_for_multiple_fees(
rpc_client: &RpcClient,
account_pubkey: &Pubkey,
blockhash: &Hash,
messages: &[&Message],
) -> Result<(), CliError> {
check_account_for_multiple_fees_with_commitment(
rpc_client,
account_pubkey,
blockhash,
messages,
CommitmentConfig::default(),
)
@ -51,7 +46,6 @@ pub fn check_account_for_multiple_fees(
pub fn check_account_for_multiple_fees_with_commitment(
rpc_client: &RpcClient,
account_pubkey: &Pubkey,
blockhash: &Hash,
messages: &[&Message],
commitment: CommitmentConfig,
) -> Result<(), CliError> {
@ -59,7 +53,6 @@ pub fn check_account_for_multiple_fees_with_commitment(
rpc_client,
account_pubkey,
0,
blockhash,
messages,
commitment,
)
@ -69,11 +62,10 @@ pub fn check_account_for_spend_multiple_fees_with_commitment(
rpc_client: &RpcClient,
account_pubkey: &Pubkey,
balance: u64,
blockhash: &Hash,
messages: &[&Message],
commitment: CommitmentConfig,
) -> Result<(), CliError> {
let fee = get_fee_for_message(rpc_client, blockhash, messages)?;
let fee = get_fee_for_message(rpc_client, messages)?;
if !check_account_for_balance_with_commitment(
rpc_client,
account_pubkey,
@ -98,14 +90,10 @@ pub fn check_account_for_spend_multiple_fees_with_commitment(
Ok(())
}
pub fn get_fee_for_message(
rpc_client: &RpcClient,
blockhash: &Hash,
messages: &[&Message],
) -> Result<u64, CliError> {
pub fn get_fee_for_message(rpc_client: &RpcClient, messages: &[&Message]) -> Result<u64, CliError> {
Ok(messages
.iter()
.map(|message| rpc_client.get_fee_for_message(blockhash, message))
.map(|message| rpc_client.get_fee_for_message(message))
.collect::<Result<Vec<_>, _>>()?
.iter()
.sum())
@ -185,9 +173,7 @@ mod tests {
let mut mocks = HashMap::new();
mocks.insert(RpcRequest::GetBalance, account_balance_response.clone());
let rpc_client = RpcClient::new_mock_with_mocks("".to_string(), mocks);
let blockhash = rpc_client.get_latest_blockhash().unwrap();
check_account_for_fee(&rpc_client, &pubkey, &blockhash, &message0)
.expect("unexpected result");
check_account_for_fee(&rpc_client, &pubkey, &message0).expect("unexpected result");
let check_fee_response = json!(Response {
context: RpcResponseContext { slot: 1 },
@ -197,7 +183,7 @@ mod tests {
mocks.insert(RpcRequest::GetFeeForMessage, check_fee_response);
mocks.insert(RpcRequest::GetBalance, account_balance_response.clone());
let rpc_client = RpcClient::new_mock_with_mocks("".to_string(), mocks);
assert!(check_account_for_fee(&rpc_client, &pubkey, &blockhash, &message1).is_err());
assert!(check_account_for_fee(&rpc_client, &pubkey, &message1).is_err());
let check_fee_response = json!(Response {
context: RpcResponseContext { slot: 1 },
@ -207,13 +193,9 @@ mod tests {
mocks.insert(RpcRequest::GetFeeForMessage, check_fee_response);
mocks.insert(RpcRequest::GetBalance, account_balance_response);
let rpc_client = RpcClient::new_mock_with_mocks("".to_string(), mocks);
assert!(check_account_for_multiple_fees(
&rpc_client,
&pubkey,
&blockhash,
&[&message0, &message0]
)
.is_err());
assert!(
check_account_for_multiple_fees(&rpc_client, &pubkey, &[&message0, &message0]).is_err()
);
let account_balance = 2;
let account_balance_response = json!(Response {
@ -230,7 +212,7 @@ mod tests {
mocks.insert(RpcRequest::GetBalance, account_balance_response);
let rpc_client = RpcClient::new_mock_with_mocks("".to_string(), mocks);
check_account_for_multiple_fees(&rpc_client, &pubkey, &blockhash, &[&message0, &message0])
check_account_for_multiple_fees(&rpc_client, &pubkey, &[&message0, &message0])
.expect("unexpected result");
}
@ -261,23 +243,16 @@ mod tests {
let mut mocks = HashMap::new();
mocks.insert(RpcRequest::GetFeeForMessage, check_fee_response);
let rpc_client = RpcClient::new_mock_with_mocks("".to_string(), mocks);
let blockhash = rpc_client.get_latest_blockhash().unwrap();
// No messages, no fee.
assert_eq!(
get_fee_for_message(&rpc_client, &blockhash, &[]).unwrap(),
0
);
assert_eq!(get_fee_for_message(&rpc_client, &[]).unwrap(), 0);
// One message w/ one signature, a fee.
let pubkey0 = Pubkey::new(&[0; 32]);
let pubkey1 = Pubkey::new(&[1; 32]);
let ix0 = system_instruction::transfer(&pubkey0, &pubkey1, 1);
let message0 = Message::new(&[ix0], Some(&pubkey0));
assert_eq!(
get_fee_for_message(&rpc_client, &blockhash, &[&message0]).unwrap(),
1
);
assert_eq!(get_fee_for_message(&rpc_client, &[&message0]).unwrap(), 1);
// No signatures, no fee.
let check_fee_response = json!(Response {
@ -289,7 +264,7 @@ mod tests {
let rpc_client = RpcClient::new_mock_with_mocks("".to_string(), mocks);
let message = Message::default();
assert_eq!(
get_fee_for_message(&rpc_client, &blockhash, &[&message, &message]).unwrap(),
get_fee_for_message(&rpc_client, &[&message, &message]).unwrap(),
0
);
}

View File

@ -367,7 +367,6 @@ pub fn process_authorize_nonce_account(
check_account_for_fee_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
&latest_blockhash,
&tx.message,
config.commitment,
)?;
@ -552,7 +551,6 @@ pub fn process_new_nonce(
check_account_for_fee_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
&latest_blockhash,
&tx.message,
config.commitment,
)?;
@ -628,7 +626,6 @@ pub fn process_withdraw_from_nonce_account(
check_account_for_fee_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
&latest_blockhash,
&tx.message,
config.commitment,
)?;

View File

@ -2071,14 +2071,11 @@ fn check_payer(
balance_needed: u64,
messages: &[&Message],
) -> Result<(), Box<dyn std::error::Error>> {
let blockhash = rpc_client.get_latest_blockhash()?;
// Does the payer have enough?
check_account_for_spend_multiple_fees_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
balance_needed,
&blockhash,
messages,
config.commitment,
)?;

View File

@ -144,9 +144,9 @@ where
F: Fn(u64) -> Message,
{
let fee = match blockhash {
Some(blockhash) => {
Some(_) => {
let dummy_message = build_message(0);
get_fee_for_message(rpc_client, blockhash, &[&dummy_message])?
get_fee_for_message(rpc_client, &[&dummy_message])?
}
None => 0, // Offline, cannot calulate fee
};

View File

@ -1455,7 +1455,6 @@ pub fn process_stake_authorize(
check_account_for_fee_with_commitment(
rpc_client,
&tx.message.account_keys[0],
&recent_blockhash,
&tx.message,
config.commitment,
)?;
@ -1534,7 +1533,6 @@ pub fn process_deactivate_stake_account(
check_account_for_fee_with_commitment(
rpc_client,
&tx.message.account_keys[0],
&recent_blockhash,
&tx.message,
config.commitment,
)?;
@ -1632,7 +1630,6 @@ pub fn process_withdraw_stake(
check_account_for_fee_with_commitment(
rpc_client,
&tx.message.account_keys[0],
&recent_blockhash,
&tx.message,
config.commitment,
)?;
@ -1776,7 +1773,6 @@ pub fn process_split_stake(
check_account_for_fee_with_commitment(
rpc_client,
&tx.message.account_keys[0],
&recent_blockhash,
&tx.message,
config.commitment,
)?;
@ -1882,7 +1878,6 @@ pub fn process_merge_stake(
check_account_for_fee_with_commitment(
rpc_client,
&tx.message.account_keys[0],
&recent_blockhash,
&tx.message,
config.commitment,
)?;
@ -1977,7 +1972,6 @@ pub fn process_stake_set_lockup(
check_account_for_fee_with_commitment(
rpc_client,
&tx.message.account_keys[0],
&recent_blockhash,
&tx.message,
config.commitment,
)?;
@ -2419,7 +2413,6 @@ pub fn process_delegate_stake(
check_account_for_fee_with_commitment(
rpc_client,
&tx.message.account_keys[0],
&recent_blockhash,
&tx.message,
config.commitment,
)?;

View File

@ -774,7 +774,6 @@ pub fn process_vote_authorize(
check_account_for_fee_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
&latest_blockhash,
&tx.message,
config.commitment,
)?;
@ -811,7 +810,6 @@ pub fn process_vote_update_validator(
check_account_for_fee_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
&latest_blockhash,
&tx.message,
config.commitment,
)?;
@ -842,7 +840,6 @@ pub fn process_vote_update_commission(
check_account_for_fee_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
&latest_blockhash,
&tx.message,
config.commitment,
)?;
@ -980,7 +977,6 @@ pub fn process_withdraw_from_vote_account(
check_account_for_fee_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
&latest_blockhash,
&transaction.message,
config.commitment,
)?;
@ -1036,7 +1032,6 @@ pub fn process_close_vote_account(
check_account_for_fee_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
&latest_blockhash,
&transaction.message,
config.commitment,
)?;

View File

@ -4800,7 +4800,7 @@ impl RpcClient {
}
#[allow(deprecated)]
pub fn get_fee_for_message(&self, blockhash: &Hash, message: &Message) -> ClientResult<u64> {
pub fn get_fee_for_message(&self, message: &Message) -> ClientResult<u64> {
if self.get_node_version()? < semver::Version::new(1, 8, 0) {
let Fees { fee_calculator, .. } = self.get_fees()?;
Ok(fee_calculator
@ -4811,7 +4811,7 @@ impl RpcClient {
serialize_and_encode::<Message>(message, UiTransactionEncoding::Base64)?;
let result = self.send::<Response<Option<u64>>>(
RpcRequest::GetFeeForMessage,
json!([blockhash.to_string(), serialized_encoded, self.commitment()]),
json!([serialized_encoded, self.commitment()]),
)?;
result
.value

View File

@ -600,9 +600,9 @@ impl SyncClient for ThinClient {
.map_err(|e| e.into())
}
fn get_fee_for_message(&self, blockhash: &Hash, message: &Message) -> TransportResult<u64> {
fn get_fee_for_message(&self, message: &Message) -> TransportResult<u64> {
self.rpc_client()
.get_fee_for_message(blockhash, message)
.get_fee_for_message(message)
.map_err(|e| e.into())
}

View File

@ -577,7 +577,7 @@ impl TestValidator {
}
println!("Waiting for fees to stabilize {:?}...", num_tries);
match rpc_client.get_latest_blockhash() {
Ok(blockhash) => match rpc_client.get_fee_for_message(&blockhash, &message) {
Ok(_) => match rpc_client.get_fee_for_message(&message) {
Ok(fee) => {
if fee != 0 {
break;

View File

@ -1961,12 +1961,11 @@ impl JsonRpcRequestProcessor {
fn get_fee_for_message(
&self,
blockhash: &Hash,
message: &SanitizedMessage,
commitment: Option<CommitmentConfig>,
) -> Result<RpcResponse<Option<u64>>> {
let bank = self.bank(commitment);
let fee = bank.get_fee_for_message(blockhash, message);
let fee = bank.get_fee_for_message(message);
Ok(new_response(&bank, fee))
}
}
@ -3124,7 +3123,6 @@ pub mod rpc_full {
fn get_fee_for_message(
&self,
meta: Self::Metadata,
blockhash: String,
data: String,
commitment: Option<CommitmentConfig>,
) -> Result<RpcResponse<Option<u64>>>;
@ -3640,20 +3638,17 @@ pub mod rpc_full {
fn get_fee_for_message(
&self,
meta: Self::Metadata,
blockhash: String,
data: String,
commitment: Option<CommitmentConfig>,
) -> Result<RpcResponse<Option<u64>>> {
debug!("get_fee_for_message rpc request received");
let blockhash = Hash::from_str(&blockhash)
.map_err(|e| Error::invalid_params(format!("{:?}", e)))?;
let (_, message) =
decode_and_deserialize::<Message>(data, UiTransactionEncoding::Base64)?;
SanitizedMessage::try_from(message)
.map_err(|err| {
Error::invalid_params(format!("invalid transaction message: {}", err))
})
.and_then(|message| meta.get_fee_for_message(&blockhash, &message, commitment))
.and_then(|message| meta.get_fee_for_message(&message, commitment))
}
}
}

View File

@ -112,7 +112,9 @@ impl TransactionStatusService {
bank.get_fee_calculator(transaction.message().recent_blockhash())
})
.expect("FeeCalculator must exist");
let fee = transaction.message().calculate_fee(&fee_calculator);
let fee = transaction
.message()
.calculate_fee(fee_calculator.lamports_per_signature);
let tx_account_locks =
transaction.get_account_locks(bank.demote_program_write_locks());

View File

@ -486,7 +486,8 @@ impl Accounts {
.cloned()
});
let fee = if let Some(fee_calculator) = fee_calculator {
tx.message().calculate_fee(&fee_calculator)
tx.message()
.calculate_fee(fee_calculator.lamports_per_signature)
} else {
return (Err(TransactionError::BlockhashNotFound), None);
};

View File

@ -3108,11 +3108,8 @@ impl Bank {
&self.fee_rate_governor
}
pub fn get_fee_for_message(&self, hash: &Hash, message: &SanitizedMessage) -> Option<u64> {
let blockhash_queue = self.blockhash_queue.read().unwrap();
#[allow(deprecated)]
let fee_calculator = blockhash_queue.get_fee_calculator(hash)?;
Some(message.calculate_fee(fee_calculator))
pub fn get_fee_for_message(&self, message: &SanitizedMessage) -> Option<u64> {
Some(message.calculate_fee(self.fee_rate_governor.lamports_per_signature))
}
#[deprecated(
@ -4042,7 +4039,9 @@ impl Bank {
});
let fee_calculator = fee_calculator.ok_or(TransactionError::BlockhashNotFound)?;
let fee = tx.message().calculate_fee(&fee_calculator);
let fee = tx
.message()
.calculate_fee(fee_calculator.lamports_per_signature);
match *res {
Err(TransactionError::InstructionError(_, _)) => {
@ -10920,10 +10919,7 @@ pub(crate) mod tests {
/* Check balances */
let mut expected_balance = 4_650_000
- bank
.get_fee_for_message(
&bank.last_blockhash(),
&durable_tx.message.try_into().unwrap(),
)
.get_fee_for_message(&durable_tx.message.try_into().unwrap())
.unwrap();
assert_eq!(bank.get_balance(&custodian_pubkey), expected_balance);
assert_eq!(bank.get_balance(&nonce_pubkey), 250_000);
@ -10977,10 +10973,7 @@ pub(crate) mod tests {
);
/* Check fee charged and nonce has advanced */
expected_balance -= bank
.get_fee_for_message(
&bank.last_blockhash(),
&SanitizedMessage::try_from(durable_tx.message.clone()).unwrap(),
)
.get_fee_for_message(&SanitizedMessage::try_from(durable_tx.message.clone()).unwrap())
.unwrap();
assert_eq!(bank.get_balance(&custodian_pubkey), expected_balance);
assert_ne!(nonce_hash, get_nonce_account(&bank, &nonce_pubkey).unwrap());
@ -11044,10 +11037,7 @@ pub(crate) mod tests {
bank.get_balance(&custodian_pubkey),
initial_custodian_balance
- bank
.get_fee_for_message(
&bank.last_blockhash(),
&durable_tx.message.try_into().unwrap()
)
.get_fee_for_message(&durable_tx.message.try_into().unwrap())
.unwrap()
);
assert_eq!(nonce_hash, get_nonce_account(&bank, &nonce_pubkey).unwrap());
@ -11099,10 +11089,7 @@ pub(crate) mod tests {
bank.get_balance(&nonce_pubkey),
nonce_starting_balance
- bank
.get_fee_for_message(
&bank.last_blockhash(),
&durable_tx.message.try_into().unwrap()
)
.get_fee_for_message(&durable_tx.message.try_into().unwrap())
.unwrap()
);
assert_ne!(nonce_hash, get_nonce_account(&bank, &nonce_pubkey).unwrap());

View File

@ -303,10 +303,10 @@ impl SyncClient for BankClient {
Ok(self.bank.is_blockhash_valid(blockhash))
}
fn get_fee_for_message(&self, blockhash: &Hash, message: &Message) -> Result<u64> {
fn get_fee_for_message(&self, message: &Message) -> Result<u64> {
SanitizedMessage::try_from(message.clone())
.ok()
.and_then(|message| self.bank.get_fee_for_message(blockhash, &message))
.and_then(|message| self.bank.get_fee_for_message(&message))
.ok_or_else(|| {
TransportError::IoError(io::Error::new(
io::ErrorKind::Other,

View File

@ -3049,9 +3049,7 @@ mod tests {
bank2.last_blockhash(),
))
.unwrap();
let fee = bank2
.get_fee_for_message(&bank2.last_blockhash(), tx.message())
.unwrap();
let fee = bank2.get_fee_for_message(tx.message()).unwrap();
let tx = system_transaction::transfer(
&key1,
&key2.pubkey(),

View File

@ -1,6 +1,5 @@
use {
crate::{
fee_calculator::FeeCalculator,
hash::Hash,
instruction::{CompiledInstruction, Instruction},
message::{MappedAddresses, MappedMessage, Message, MessageHeader},
@ -292,7 +291,7 @@ impl SanitizedMessage {
}
/// Calculate the total fees for a transaction given a fee calculator
pub fn calculate_fee(&self, fee_calculator: &FeeCalculator) -> u64 {
pub fn calculate_fee(&self, lamports_per_signature: u64) -> u64 {
let mut num_signatures = u64::from(self.header().num_required_signatures);
for (program_id, instruction) in self.program_instructions_iter() {
if secp256k1_program::check_id(program_id) {
@ -303,7 +302,7 @@ impl SanitizedMessage {
}
}
fee_calculator.lamports_per_signature.saturating_mul(num_signatures)
lamports_per_signature.saturating_mul(num_signatures)
}
/// Inspect all message keys for the bpf upgradeable loader
@ -465,10 +464,10 @@ mod tests {
// Default: no fee.
let message =
SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap();
assert_eq!(message.calculate_fee(&FeeCalculator::default()), 0);
assert_eq!(message.calculate_fee(0), 0);
// One signature, a fee.
assert_eq!(message.calculate_fee(&FeeCalculator::new(1)), 1);
assert_eq!(message.calculate_fee(1), 1);
// Two signatures, double the fee.
let key0 = Pubkey::new_unique();
@ -476,7 +475,7 @@ mod tests {
let ix0 = system_instruction::transfer(&key0, &key1, 1);
let ix1 = system_instruction::transfer(&key1, &key0, 1);
let message = SanitizedMessage::try_from(Message::new(&[ix0, ix1], Some(&key0))).unwrap();
assert_eq!(message.calculate_fee(&FeeCalculator::new(2)), 4);
assert_eq!(message.calculate_fee(2), 4);
}
#[test]
@ -588,7 +587,7 @@ mod tests {
Some(&key0),
))
.unwrap();
assert_eq!(message.calculate_fee(&FeeCalculator::new(1)), 2);
assert_eq!(message.calculate_fee(1), 2);
secp_instruction1.data = vec![0];
secp_instruction2.data = vec![10];
@ -597,6 +596,6 @@ mod tests {
Some(&key0),
))
.unwrap();
assert_eq!(message.calculate_fee(&FeeCalculator::new(1)), 11);
assert_eq!(message.calculate_fee(1), 11);
}
}

View File

@ -168,7 +168,7 @@ pub trait SyncClient {
fn is_blockhash_valid(&self, blockhash: &Hash, commitment: CommitmentConfig) -> Result<bool>;
/// Calculate the fee for a `Message`
fn get_fee_for_message(&self, blockhash: &Hash, message: &Message) -> Result<u64>;
fn get_fee_for_message(&self, message: &Message) -> Result<u64>;
/// Get a new blockhash after the one specified
fn get_new_latest_blockhash(&self, blockhash: &Hash) -> Result<Hash>;

View File

@ -730,10 +730,9 @@ fn check_payer_balances(
) -> Result<(), Error> {
let mut undistributed_tokens: u64 = allocations.iter().map(|x| x.amount).sum();
let blockhash = client.get_latest_blockhash()?;
let fees = messages
.iter()
.map(|message| client.get_fee_for_message(&blockhash, message))
.map(|message| client.get_fee_for_message(message))
.collect::<Result<Vec<_>, _>>()
.unwrap()
.iter()

View File

@ -97,10 +97,9 @@ pub fn check_spl_token_balances(
.expect("spl_token_args must be some");
let allocation_amount: u64 = allocations.iter().map(|x| x.amount).sum();
let blockhash = client.get_latest_blockhash()?;
let fees: u64 = messages
.iter()
.map(|message| client.get_fee_for_message(&blockhash, message))
.map(|message| client.get_fee_for_message(message))
.collect::<Result<Vec<_>, _>>()
.unwrap()
.iter()