From 04495f4b241ab0f450a19005834f2ce95b1dabb6 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Tue, 4 Oct 2022 19:47:32 +0800 Subject: [PATCH] RPC: Support versioned txs in getFeeForMessage API --- cli/src/checks.rs | 2 +- programs/address-lookup-table/src/error.rs | 12 +- rpc-client/src/nonblocking/rpc_client.rs | 42 +++---- rpc-client/src/rpc_client.rs | 10 +- rpc/src/rpc.rs | 123 +++++++++++++++------ runtime/src/bank/address_lookup_table.rs | 13 ++- sdk/program/src/message/address_loader.rs | 56 ++++++++++ sdk/program/src/message/mod.rs | 3 +- sdk/program/src/message/sanitized.rs | 24 +++- sdk/src/transaction/error.rs | 24 +++- sdk/src/transaction/sanitized.rs | 22 +--- 11 files changed, 235 insertions(+), 96 deletions(-) create mode 100644 sdk/program/src/message/address_loader.rs diff --git a/cli/src/checks.rs b/cli/src/checks.rs index 23be57cc07d8ad..ea23c1b9cdfd13 100644 --- a/cli/src/checks.rs +++ b/cli/src/checks.rs @@ -112,7 +112,7 @@ pub fn get_fee_for_messages( ) -> Result { Ok(messages .iter() - .map(|message| rpc_client.get_fee_for_message(message)) + .map(|message| rpc_client.get_fee_for_message(*message)) .collect::, _>>()? .iter() .sum()) diff --git a/programs/address-lookup-table/src/error.rs b/programs/address-lookup-table/src/error.rs index 6fb67c703a65b0..b427067afc386c 100644 --- a/programs/address-lookup-table/src/error.rs +++ b/programs/address-lookup-table/src/error.rs @@ -1,5 +1,5 @@ #[cfg(not(target_os = "solana"))] -use solana_sdk::transaction::TransactionError; +use solana_program::message::AddressLoaderError; use thiserror::Error; #[derive(Debug, Error, PartialEq, Eq, Clone)] @@ -22,13 +22,13 @@ pub enum AddressLookupError { } #[cfg(not(target_os = "solana"))] -impl From for TransactionError { +impl From for AddressLoaderError { fn from(err: AddressLookupError) -> Self { match err { - AddressLookupError::LookupTableAccountNotFound => Self::AddressLookupTableNotFound, - AddressLookupError::InvalidAccountOwner => Self::InvalidAddressLookupTableOwner, - AddressLookupError::InvalidAccountData => Self::InvalidAddressLookupTableData, - AddressLookupError::InvalidLookupIndex => Self::InvalidAddressLookupTableIndex, + AddressLookupError::LookupTableAccountNotFound => Self::LookupTableAccountNotFound, + AddressLookupError::InvalidAccountOwner => Self::InvalidAccountOwner, + AddressLookupError::InvalidAccountData => Self::InvalidAccountData, + AddressLookupError::InvalidLookupIndex => Self::InvalidLookupIndex, } } } diff --git a/rpc-client/src/nonblocking/rpc_client.rs b/rpc-client/src/nonblocking/rpc_client.rs index 492dea0fe6897b..4ab7cf1d66fce1 100644 --- a/rpc-client/src/nonblocking/rpc_client.rs +++ b/rpc-client/src/nonblocking/rpc_client.rs @@ -19,7 +19,8 @@ use { http_sender::HttpSender, mock_sender::MockSender, rpc_client::{ - GetConfirmedSignaturesForAddress2Config, RpcClientConfig, SerializableTransaction, + GetConfirmedSignaturesForAddress2Config, RpcClientConfig, SerializableMessage, + SerializableTransaction, }, rpc_sender::*, }, @@ -47,10 +48,9 @@ use { epoch_schedule::EpochSchedule, fee_calculator::{FeeCalculator, FeeRateGovernor}, hash::Hash, - message::Message, pubkey::Pubkey, signature::Signature, - transaction::{self}, + transaction, }, solana_transaction_status::{ EncodedConfirmedBlock, EncodedConfirmedTransactionWithStatusMeta, TransactionStatus, @@ -5276,28 +5276,20 @@ impl RpcClient { } #[allow(deprecated)] - pub async fn get_fee_for_message(&self, message: &Message) -> ClientResult { - if self.get_node_version().await? < semver::Version::new(1, 9, 0) { - let fee_calculator = self - .get_fee_calculator_for_blockhash(&message.recent_blockhash) - .await? - .ok_or_else(|| ClientErrorKind::Custom("Invalid blockhash".to_string()))?; - Ok(fee_calculator - .lamports_per_signature - .saturating_mul(message.header.num_required_signatures as u64)) - } else { - let serialized_encoded = - serialize_and_encode::(message, UiTransactionEncoding::Base64)?; - let result = self - .send::>>( - RpcRequest::GetFeeForMessage, - json!([serialized_encoded, self.commitment()]), - ) - .await?; - result - .value - .ok_or_else(|| ClientErrorKind::Custom("Invalid blockhash".to_string()).into()) - } + pub async fn get_fee_for_message( + &self, + message: &impl SerializableMessage, + ) -> ClientResult { + let serialized_encoded = serialize_and_encode(message, UiTransactionEncoding::Base64)?; + let result = self + .send::>>( + RpcRequest::GetFeeForMessage, + json!([serialized_encoded, self.commitment()]), + ) + .await?; + result + .value + .ok_or_else(|| ClientErrorKind::Custom("Invalid blockhash".to_string()).into()) } pub async fn get_new_latest_blockhash(&self, blockhash: &Hash) -> ClientResult { diff --git a/rpc-client/src/rpc_client.rs b/rpc-client/src/rpc_client.rs index d6529c0a5e1d6f..372839d4a4f106 100644 --- a/rpc-client/src/rpc_client.rs +++ b/rpc-client/src/rpc_client.rs @@ -41,7 +41,7 @@ use { epoch_schedule::EpochSchedule, fee_calculator::{FeeCalculator, FeeRateGovernor}, hash::Hash, - message::Message, + message::{v0, Message as LegacyMessage}, pubkey::Pubkey, signature::Signature, transaction::{self, uses_durable_nonce, Transaction, VersionedTransaction}, @@ -68,6 +68,12 @@ impl RpcClientConfig { } } +/// Trait used to add support for versioned messages to RPC APIs while +/// retaining backwards compatibility +pub trait SerializableMessage: Serialize {} +impl SerializableMessage for LegacyMessage {} +impl SerializableMessage for v0::Message {} + /// Trait used to add support for versioned transactions to RPC APIs while /// retaining backwards compatibility pub trait SerializableTransaction: Serialize { @@ -3975,7 +3981,7 @@ impl RpcClient { } #[allow(deprecated)] - pub fn get_fee_for_message(&self, message: &Message) -> ClientResult { + pub fn get_fee_for_message(&self, message: &impl SerializableMessage) -> ClientResult { self.invoke((self.rpc_client.as_ref()).get_fee_for_message(message)) } diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 62a4dc812ee5f1..f7c1a4bda006bb 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -63,7 +63,7 @@ use { feature_set, fee_calculator::FeeCalculator, hash::Hash, - message::{Message, SanitizedMessage}, + message::SanitizedMessage, pubkey::{Pubkey, PUBKEY_BYTES}, signature::{Keypair, Signature, Signer}, stake::state::{StakeActivationStatus, StakeState}, @@ -2164,16 +2164,6 @@ impl JsonRpcRequestProcessor { Ok(new_response(&bank, is_valid)) } - fn get_fee_for_message( - &self, - message: &SanitizedMessage, - config: RpcContextConfig, - ) -> Result>> { - let bank = self.get_bank_with_config(config)?; - let fee = bank.get_fee_for_message(message); - Ok(new_response(&bank, fee)) - } - fn get_stake_minimum_delegation(&self, config: RpcContextConfig) -> Result> { let bank = self.get_bank_with_config(config)?; let stake_minimum_delegation = @@ -3288,7 +3278,10 @@ pub mod rpc_accounts { // Full RPC interface that an API node is expected to provide // (rpc_minimal should also be provided by an API node) pub mod rpc_full { - use super::*; + use { + super::*, + solana_sdk::message::{SanitizedVersionedMessage, VersionedMessage}, + }; #[rpc] pub trait Full { type Metadata; @@ -4001,12 +3994,21 @@ pub mod rpc_full { config: Option, ) -> Result>> { debug!("get_fee_for_message rpc request received"); - let (_, message) = - decode_and_deserialize::(data, TransactionBinaryEncoding::Base64)?; - let sanitized_message = SanitizedMessage::try_from(message).map_err(|err| { - Error::invalid_params(format!("invalid transaction message: {}", err)) - })?; - meta.get_fee_for_message(&sanitized_message, config.unwrap_or_default()) + let (_, message) = decode_and_deserialize::( + data, + TransactionBinaryEncoding::Base64, + )?; + let bank = &*meta.get_bank_with_config(config.unwrap_or_default())?; + let sanitized_versioned_message = SanitizedVersionedMessage::try_from(message) + .map_err(|err| { + Error::invalid_params(format!("invalid transaction message: {}", err)) + })?; + let sanitized_message = SanitizedMessage::try_new(sanitized_versioned_message, bank) + .map_err(|err| { + Error::invalid_params(format!("invalid transaction message: {}", err)) + })?; + let fee = bank.get_fee_for_message(&sanitized_message); + Ok(new_response(bank, fee)) } fn get_stake_minimum_delegation( @@ -4649,10 +4651,13 @@ pub mod tests { account::{Account, WritableAccount}, clock::MAX_RECENT_BLOCKHASHES, compute_budget::ComputeBudgetInstruction, - fee_calculator::DEFAULT_BURN_PERCENT, + fee_calculator::{FeeRateGovernor, DEFAULT_BURN_PERCENT}, hash::{hash, Hash}, instruction::InstructionError, - message::{v0, v0::MessageAddressTableLookup, MessageHeader, VersionedMessage}, + message::{ + v0::{self, MessageAddressTableLookup}, + Message, MessageHeader, VersionedMessage, + }, nonce::{self, state::DurableNonce}, rpc_port, signature::{Keypair, Signer}, @@ -4683,7 +4688,8 @@ pub mod tests { std::{borrow::Cow, collections::HashMap}, }; - const TEST_MINT_LAMPORTS: u64 = 1_000_000; + const TEST_MINT_LAMPORTS: u64 = 1_000_000_000; + const TEST_SIGNATURE_FEE: u64 = 5_000; const TEST_SLOTS_PER_EPOCH: u64 = DELINQUENT_VALIDATOR_SLOT_DISTANCE + 1; fn create_test_request(method: &str, params: Option) -> serde_json::Value { @@ -4829,8 +4835,12 @@ pub mod tests { let keypair3 = Keypair::new(); let bank = self.working_bank(); let rent_exempt_amount = bank.get_minimum_balance_for_rent_exemption(0); - bank.transfer(rent_exempt_amount, mint_keypair, &keypair2.pubkey()) - .unwrap(); + bank.transfer( + rent_exempt_amount + TEST_SIGNATURE_FEE, + mint_keypair, + &keypair2.pubkey(), + ) + .unwrap(); let (entries, signatures) = create_test_transaction_entries( vec![&self.mint_keypair, &keypair1, &keypair2, &keypair3], @@ -5471,7 +5481,7 @@ pub mod tests { "context": {"slot": 0, "apiVersion": RpcApiVersion::default()}, "value":{ "owner": "11111111111111111111111111111111", - "lamports": 1_000_000, + "lamports": TEST_MINT_LAMPORTS, "data": "", "executable": false, "rentEpoch": 0 @@ -5548,7 +5558,7 @@ pub mod tests { let expected = json!([ { "owner": "11111111111111111111111111111111", - "lamports": 1_000_000, + "lamports": TEST_MINT_LAMPORTS, "data": ["", "base64"], "executable": false, "rentEpoch": 0 @@ -5580,7 +5590,7 @@ pub mod tests { let expected = json!([ { "owner": "11111111111111111111111111111111", - "lamports": 1_000_000, + "lamports": TEST_MINT_LAMPORTS, "data": ["", "base58"], "executable": false, "rentEpoch": 0 @@ -6167,7 +6177,7 @@ pub mod tests { "value":{ "blockhash": recent_blockhash.to_string(), "feeCalculator": { - "lamportsPerSignature": 0, + "lamportsPerSignature": TEST_SIGNATURE_FEE, } }, }, @@ -6196,7 +6206,7 @@ pub mod tests { "value": { "blockhash": recent_blockhash.to_string(), "feeCalculator": { - "lamportsPerSignature": 0, + "lamportsPerSignature": TEST_SIGNATURE_FEE, }, "lastValidSlot": MAX_RECENT_BLOCKHASHES, "lastValidBlockHeight": MAX_RECENT_BLOCKHASHES, @@ -6276,9 +6286,9 @@ pub mod tests { "value":{ "feeRateGovernor": { "burnPercent": DEFAULT_BURN_PERCENT, - "maxLamportsPerSignature": 0, - "minLamportsPerSignature": 0, - "targetLamportsPerSignature": 0, + "maxLamportsPerSignature": TEST_SIGNATURE_FEE, + "minLamportsPerSignature": TEST_SIGNATURE_FEE, + "targetLamportsPerSignature": TEST_SIGNATURE_FEE, "targetSignaturesPerSlot": 0 } }, @@ -6547,6 +6557,7 @@ pub mod tests { genesis_config.rent.exemption_threshold = 2.0; genesis_config.epoch_schedule = EpochSchedule::custom(TEST_SLOTS_PER_EPOCH, TEST_SLOTS_PER_EPOCH, false); + genesis_config.fee_rate_governor = FeeRateGovernor::new(TEST_SIGNATURE_FEE, 0); let bank = Bank::new_for_tests(&genesis_config); ( @@ -8457,7 +8468,7 @@ pub mod tests { assert_eq!( sanitize_transaction(versioned_tx, SimpleAddressLoader::Disabled).unwrap_err(), Error::invalid_params( - "invalid transaction: Transaction loads an address table account that doesn't exist".to_string(), + "invalid transaction: Transaction version is unsupported".to_string(), ) ); } @@ -8479,6 +8490,54 @@ pub mod tests { ); } + #[test] + fn test_get_fee_for_message() { + let rpc = RpcHandler::start(); + let bank = rpc.working_bank(); + // Slot hashes is necessary for processing versioned txs. + bank.set_sysvar_for_tests(&SlotHashes::default()); + // Correct blockhash is needed because fees are specific to blockhashes + let recent_blockhash = bank.last_blockhash(); + + { + let legacy_msg = VersionedMessage::Legacy(Message { + header: MessageHeader { + num_required_signatures: 1, + ..MessageHeader::default() + }, + recent_blockhash, + account_keys: vec![Pubkey::new_unique()], + ..Message::default() + }); + + let request = create_test_request( + "getFeeForMessage", + Some(json!([base64::encode(&serialize(&legacy_msg).unwrap())])), + ); + let response: RpcResponse = parse_success_result(rpc.handle_request_sync(request)); + assert_eq!(response.value, TEST_SIGNATURE_FEE); + } + + { + let v0_msg = VersionedMessage::V0(v0::Message { + header: MessageHeader { + num_required_signatures: 1, + ..MessageHeader::default() + }, + recent_blockhash, + account_keys: vec![Pubkey::new_unique()], + ..v0::Message::default() + }); + + let request = create_test_request( + "getFeeForMessage", + Some(json!([base64::encode(&serialize(&v0_msg).unwrap())])), + ); + let response: RpcResponse = parse_success_result(rpc.handle_request_sync(request)); + assert_eq!(response.value, TEST_SIGNATURE_FEE); + } + } + #[test] fn test_rpc_get_recent_prioritization_fees() { fn wait_for_cache_blocks(cache: &PrioritizationFeeCache, num_blocks: usize) { diff --git a/runtime/src/bank/address_lookup_table.rs b/runtime/src/bank/address_lookup_table.rs index 3ea321bc3009ad..6e9cae60aaccfc 100644 --- a/runtime/src/bank/address_lookup_table.rs +++ b/runtime/src/bank/address_lookup_table.rs @@ -3,8 +3,11 @@ use { crate::accounts_db::LoadZeroLamports, solana_address_lookup_table_program::error::AddressLookupError, solana_sdk::{ - message::v0::{LoadedAddresses, MessageAddressTableLookup}, - transaction::{AddressLoader, Result as TransactionResult, TransactionError}, + message::{ + v0::{LoadedAddresses, MessageAddressTableLookup}, + AddressLoaderError, + }, + transaction::AddressLoader, }, }; @@ -12,9 +15,9 @@ impl AddressLoader for &Bank { fn load_addresses( self, address_table_lookups: &[MessageAddressTableLookup], - ) -> TransactionResult { + ) -> Result { if !self.versioned_tx_message_enabled() { - return Err(TransactionError::UnsupportedVersion); + return Err(AddressLoaderError::Disabled); } let load_zero_lamports = LoadZeroLamports::None; @@ -24,7 +27,7 @@ impl AddressLoader for &Bank { .read() .unwrap() .get_slot_hashes() - .map_err(|_| TransactionError::AccountNotFound)?; + .map_err(|_| AddressLoaderError::SlotHashesSysvarNotFound)?; Ok(address_table_lookups .iter() diff --git a/sdk/program/src/message/address_loader.rs b/sdk/program/src/message/address_loader.rs new file mode 100644 index 00000000000000..83e514cd0bd63b --- /dev/null +++ b/sdk/program/src/message/address_loader.rs @@ -0,0 +1,56 @@ +use { + super::v0::{LoadedAddresses, MessageAddressTableLookup}, + thiserror::Error, +}; + +#[derive(Debug, Error, PartialEq, Eq, Clone)] +pub enum AddressLoaderError { + /// Address loading from lookup tables is disabled + #[error("Address loading from lookup tables is disabled")] + Disabled, + + /// Failed to load slot hashes sysvar + #[error("Failed to load slot hashes sysvar")] + SlotHashesSysvarNotFound, + + /// Attempted to lookup addresses from a table that does not exist + #[error("Attempted to lookup addresses from a table that does not exist")] + LookupTableAccountNotFound, + + /// Attempted to lookup addresses from an account owned by the wrong program + #[error("Attempted to lookup addresses from an account owned by the wrong program")] + InvalidAccountOwner, + + /// Attempted to lookup addresses from an invalid account + #[error("Attempted to lookup addresses from an invalid account")] + InvalidAccountData, + + /// Address lookup contains an invalid index + #[error("Address lookup contains an invalid index")] + InvalidLookupIndex, +} + +pub trait AddressLoader: Clone { + fn load_addresses( + self, + lookups: &[MessageAddressTableLookup], + ) -> Result; +} + +#[derive(Clone)] +pub enum SimpleAddressLoader { + Disabled, + Enabled(LoadedAddresses), +} + +impl AddressLoader for SimpleAddressLoader { + fn load_addresses( + self, + _lookups: &[MessageAddressTableLookup], + ) -> Result { + match self { + Self::Disabled => Err(AddressLoaderError::Disabled), + Self::Enabled(loaded_addresses) => Ok(loaded_addresses), + } + } +} diff --git a/sdk/program/src/message/mod.rs b/sdk/program/src/message/mod.rs index 376b81e9590d0b..e507335ae74c66 100644 --- a/sdk/program/src/message/mod.rs +++ b/sdk/program/src/message/mod.rs @@ -44,10 +44,11 @@ pub mod legacy; #[path = ""] mod non_bpf_modules { mod account_keys; + mod address_loader; mod sanitized; mod versions; - pub use {account_keys::*, sanitized::*, versions::*}; + pub use {account_keys::*, address_loader::*, sanitized::*, versions::*}; } use compiled_keys::*; diff --git a/sdk/program/src/message/sanitized.rs b/sdk/program/src/message/sanitized.rs index b104b0267277e1..7721dda7c2e052 100644 --- a/sdk/program/src/message/sanitized.rs +++ b/sdk/program/src/message/sanitized.rs @@ -5,7 +5,8 @@ use { message::{ legacy, v0::{self, LoadedAddresses}, - AccountKeys, MessageHeader, + AccountKeys, AddressLoader, AddressLoaderError, MessageHeader, + SanitizedVersionedMessage, VersionedMessage, }, nonce::NONCED_TX_MARKER_IX_INDEX, program_utils::limited_deserialize, @@ -81,6 +82,8 @@ pub enum SanitizeMessageError { ValueOutOfBounds, #[error("invalid value")] InvalidValue, + #[error("{0}")] + AddressLoaderError(#[from] AddressLoaderError), } impl From for SanitizeMessageError { @@ -102,6 +105,25 @@ impl TryFrom for SanitizedMessage { } impl SanitizedMessage { + /// Create a sanitized message from a sanitized versioned message. + /// If the input message uses address tables, attempt to lookup the + /// address for each table index. + pub fn try_new( + sanitized_msg: SanitizedVersionedMessage, + address_loader: impl AddressLoader, + ) -> Result { + Ok(match sanitized_msg.message { + VersionedMessage::Legacy(message) => { + SanitizedMessage::Legacy(LegacyMessage::new(message)) + } + VersionedMessage::V0(message) => { + let loaded_addresses = + address_loader.load_addresses(&message.address_table_lookups)?; + SanitizedMessage::V0(v0::LoadedMessage::new(message, loaded_addresses)) + } + }) + } + /// Return true if this message contains duplicate account keys pub fn has_duplicates(&self) -> bool { match self { diff --git a/sdk/src/transaction/error.rs b/sdk/src/transaction/error.rs index e061b3ffebf951..47d5856ce0da60 100644 --- a/sdk/src/transaction/error.rs +++ b/sdk/src/transaction/error.rs @@ -1,6 +1,8 @@ use { crate::{ - instruction::InstructionError, message::SanitizeMessageError, sanitize::SanitizeError, + instruction::InstructionError, + message::{AddressLoaderError, SanitizeMessageError}, + sanitize::SanitizeError, }, serde::Serialize, thiserror::Error, @@ -156,7 +158,23 @@ impl From for TransactionError { } impl From for TransactionError { - fn from(_err: SanitizeMessageError) -> Self { - Self::SanitizeFailure + fn from(err: SanitizeMessageError) -> Self { + match err { + SanitizeMessageError::AddressLoaderError(err) => Self::from(err), + _ => Self::SanitizeFailure, + } + } +} + +impl From for TransactionError { + fn from(err: AddressLoaderError) -> Self { + match err { + AddressLoaderError::Disabled => Self::UnsupportedVersion, + AddressLoaderError::SlotHashesSysvarNotFound => Self::AccountNotFound, + AddressLoaderError::LookupTableAccountNotFound => Self::AddressLookupTableNotFound, + AddressLoaderError::InvalidAccountOwner => Self::InvalidAddressLookupTableOwner, + AddressLoaderError::InvalidAccountData => Self::InvalidAddressLookupTableData, + AddressLoaderError::InvalidLookupIndex => Self::InvalidAddressLookupTableIndex, + } } } diff --git a/sdk/src/transaction/sanitized.rs b/sdk/src/transaction/sanitized.rs index 7c5a9df4d662c7..c416c00b0de6ec 100644 --- a/sdk/src/transaction/sanitized.rs +++ b/sdk/src/transaction/sanitized.rs @@ -1,12 +1,13 @@ #![cfg(feature = "full")] +pub use crate::message::{AddressLoader, SimpleAddressLoader}; use { super::SanitizedVersionedTransaction, crate::{ hash::Hash, message::{ legacy, - v0::{self, LoadedAddresses, MessageAddressTableLookup}, + v0::{self, LoadedAddresses}, LegacyMessage, SanitizedMessage, VersionedMessage, }, precompiles::verify_if_precompile, @@ -43,25 +44,6 @@ pub struct TransactionAccountLocks<'a> { pub writable: Vec<&'a Pubkey>, } -pub trait AddressLoader: Clone { - fn load_addresses(self, lookups: &[MessageAddressTableLookup]) -> Result; -} - -#[derive(Clone)] -pub enum SimpleAddressLoader { - Disabled, - Enabled(LoadedAddresses), -} - -impl AddressLoader for SimpleAddressLoader { - fn load_addresses(self, _lookups: &[MessageAddressTableLookup]) -> Result { - match self { - Self::Disabled => Err(TransactionError::AddressLookupTableNotFound), - Self::Enabled(loaded_addresses) => Ok(loaded_addresses), - } - } -} - /// Type that represents whether the transaction message has been precomputed or /// not. pub enum MessageHash {