From 13956d33e8602d77f81c9b0ac605dab4ffca6e85 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Mon, 20 Dec 2021 14:29:50 -0700 Subject: [PATCH 1/7] Add more-legitimate conversion from legacy Transaction to SanitizedTransaction --- sdk/src/transaction/sanitized.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/sdk/src/transaction/sanitized.rs b/sdk/src/transaction/sanitized.rs index f904aac256b..4bf68597c9f 100644 --- a/sdk/src/transaction/sanitized.rs +++ b/sdk/src/transaction/sanitized.rs @@ -76,20 +76,24 @@ impl SanitizedTransaction { }) } - /// Create a sanitized transaction from a legacy transaction. Used for tests only. - pub fn from_transaction_for_tests(tx: Transaction) -> Self { - tx.sanitize().unwrap(); + pub fn try_from_legacy_transaction(tx: Transaction) -> Result { + tx.sanitize()?; if tx.message.has_duplicates() { - Result::::Err(TransactionError::AccountLoadedTwice).unwrap(); + return Err(TransactionError::AccountLoadedTwice); } - Self { + Ok(Self { message_hash: tx.message.hash(), message: SanitizedMessage::Legacy(tx.message), is_simple_vote_tx: false, signatures: tx.signatures, - } + }) + } + + /// Create a sanitized transaction from a legacy transaction. Used for tests only. + pub fn from_transaction_for_tests(tx: Transaction) -> Self { + Self::try_from_legacy_transaction(tx).unwrap() } /// Return the first signature for this transaction. From b0ae72c0fa4388cfa920709728070c40b9ee4186 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Mon, 20 Dec 2021 14:32:26 -0700 Subject: [PATCH 2/7] Add Banks method with preflight checks --- banks-interface/src/lib.rs | 17 +++++++++++ banks-server/src/banks_server.rs | 52 ++++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/banks-interface/src/lib.rs b/banks-interface/src/lib.rs index ad2ff1ab488..36e07191d87 100644 --- a/banks-interface/src/lib.rs +++ b/banks-interface/src/lib.rs @@ -30,6 +30,19 @@ pub struct TransactionStatus { pub confirmation_status: Option, } +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct TransactionSimulationDetails { + pub logs: Vec, + pub units_consumed: u64, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +pub struct BanksTransactionResult { + pub result: Option>, + pub simulation_details: Option, +} + #[tarpc::service] pub trait Banks { async fn send_transaction_with_context(transaction: Transaction); @@ -44,6 +57,10 @@ pub trait Banks { -> Option; async fn get_slot_with_context(commitment: CommitmentLevel) -> Slot; async fn get_block_height_with_context(commitment: CommitmentLevel) -> u64; + async fn process_transaction_with_preflight_and_commitment_and_context( + transaction: Transaction, + commitment: CommitmentLevel, + ) -> BanksTransactionResult; async fn process_transaction_with_commitment_and_context( transaction: Transaction, commitment: CommitmentLevel, diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index 5cd4372a252..a355a88e5f5 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -2,9 +2,14 @@ use { bincode::{deserialize, serialize}, futures::{future, prelude::stream::StreamExt}, solana_banks_interface::{ - Banks, BanksRequest, BanksResponse, TransactionConfirmationStatus, TransactionStatus, + Banks, BanksRequest, BanksResponse, BanksTransactionResult, TransactionConfirmationStatus, + TransactionSimulationDetails, TransactionStatus, + }, + solana_runtime::{ + bank::{Bank, TransactionSimulationResult}, + bank_forks::BankForks, + commitment::BlockCommitmentCache, }, - solana_runtime::{bank::Bank, bank_forks::BankForks, commitment::BlockCommitmentCache}, solana_sdk::{ account::Account, clock::Slot, @@ -15,7 +20,7 @@ use { message::{Message, SanitizedMessage}, pubkey::Pubkey, signature::Signature, - transaction::{self, Transaction}, + transaction::{self, SanitizedTransaction, Transaction}, }, solana_send_transaction_service::{ send_transaction_service::{SendTransactionService, TransactionInfo}, @@ -242,6 +247,47 @@ impl Banks for BanksServer { self.bank(commitment).block_height() } + async fn process_transaction_with_preflight_and_commitment_and_context( + self, + ctx: Context, + transaction: Transaction, + commitment: CommitmentLevel, + ) -> BanksTransactionResult { + let sanitized_transaction = + match SanitizedTransaction::try_from_legacy_transaction(transaction.clone()) { + Err(err) => { + return BanksTransactionResult { + result: Some(Err(err)), + simulation_details: None, + }; + } + Ok(tx) => tx, + }; + if let TransactionSimulationResult { + result: Err(err), + logs, + post_simulation_accounts: _, + units_consumed, + } = self + .bank(commitment) + .simulate_transaction(sanitized_transaction) + { + return BanksTransactionResult { + result: Some(Err(err)), + simulation_details: Some(TransactionSimulationDetails { + logs, + units_consumed, + }), + }; + } + BanksTransactionResult { + result: self + .process_transaction_with_commitment_and_context(ctx, transaction, commitment) + .await, + simulation_details: None, + } + } + async fn process_transaction_with_commitment_and_context( self, _: Context, From 2738fc698ba4ce8bc30a639e361a5818914674d1 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Wed, 22 Dec 2021 17:59:16 -0700 Subject: [PATCH 3/7] Expose BanksClient method with preflight checks --- banks-client/src/error.rs | 11 ++++++++ banks-client/src/lib.rs | 53 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/banks-client/src/error.rs b/banks-client/src/error.rs index de1bea79c19..0ebb750b99e 100644 --- a/banks-client/src/error.rs +++ b/banks-client/src/error.rs @@ -19,6 +19,13 @@ pub enum BanksClientError { #[error("transport transaction error: {0}")] TransactionError(#[from] TransactionError), + + #[error("simulation error: {err:?}, logs: {logs:?}, units_consumed: {units_consumed:?}")] + SimulationError { + err: TransactionError, + logs: Vec, + units_consumed: u64, + }, } impl BanksClientError { @@ -40,6 +47,9 @@ impl From for io::Error { BanksClientError::TransactionError(err) => { Self::new(io::ErrorKind::Other, err.to_string()) } + BanksClientError::SimulationError { err, .. } => { + Self::new(io::ErrorKind::Other, err.to_string()) + } } } } @@ -57,6 +67,7 @@ impl From for TransportError { Self::IoError(io::Error::new(io::ErrorKind::Other, err.to_string())) } BanksClientError::TransactionError(err) => Self::TransactionError(err), + BanksClientError::SimulationError { err, .. } => Self::TransactionError(err), } } } diff --git a/banks-client/src/lib.rs b/banks-client/src/lib.rs index 520ae0e09a3..758fbd5f561 100644 --- a/banks-client/src/lib.rs +++ b/banks-client/src/lib.rs @@ -10,7 +10,7 @@ use { crate::error::BanksClientError, borsh::BorshDeserialize, futures::{future::join_all, Future, FutureExt, TryFutureExt}, - solana_banks_interface::{BanksRequest, BanksResponse}, + solana_banks_interface::{BanksRequest, BanksResponse, BanksTransactionResult}, solana_program::{ clock::Slot, fee_calculator::FeeCalculator, hash::Hash, program_pack::Pack, pubkey::Pubkey, rent::Rent, sysvar::Sysvar, @@ -120,6 +120,21 @@ impl BanksClient { .map_err(Into::into) } + pub fn process_transaction_with_preflight_and_commitment_and_context( + &mut self, + ctx: Context, + transaction: Transaction, + commitment: CommitmentLevel, + ) -> impl Future> + '_ { + self.inner + .process_transaction_with_preflight_and_commitment_and_context( + ctx, + transaction, + commitment, + ) + .map_err(Into::into) + } + pub fn get_account_with_commitment_and_context( &mut self, ctx: Context, @@ -201,6 +216,42 @@ impl BanksClient { }) } + /// Send a transaction and return any preflight (sanitization or simulation) errors, or return + /// after the transaction has been rejected or reached the given level of commitment. + pub fn process_transaction_with_preflight( + &mut self, + transaction: Transaction, + commitment: CommitmentLevel, + ) -> impl Future> + '_ { + let mut ctx = context::current(); + ctx.deadline += Duration::from_secs(50); + self.process_transaction_with_preflight_and_commitment_and_context( + ctx, + transaction, + commitment, + ) + .map(|result| match result? { + BanksTransactionResult { + result: None, + simulation_details: _, + } => Err(BanksClientError::ClientError( + "invalid blockhash or fee-payer", + )), + BanksTransactionResult { + result: Some(Err(err)), + simulation_details: Some(simulation_details), + } => Err(BanksClientError::SimulationError { + err, + logs: simulation_details.logs, + units_consumed: simulation_details.units_consumed, + }), + BanksTransactionResult { + result: Some(result), + simulation_details: _, + } => result.map_err(Into::into), + }) + } + /// Send a transaction and return until the transaction has been finalized or rejected. pub fn process_transaction( &mut self, From 847862fe5aa6ed98fe6968cf610cd8559446519b Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Mon, 27 Dec 2021 11:20:02 -0700 Subject: [PATCH 4/7] Unwrap simulation err --- banks-client/src/error.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/banks-client/src/error.rs b/banks-client/src/error.rs index 0ebb750b99e..6f27f3ce5f3 100644 --- a/banks-client/src/error.rs +++ b/banks-client/src/error.rs @@ -30,10 +30,10 @@ pub enum BanksClientError { impl BanksClientError { pub fn unwrap(&self) -> TransactionError { - if let BanksClientError::TransactionError(err) = self { - err.clone() - } else { - panic!("unexpected transport error") + match self { + BanksClientError::TransactionError(err) + | BanksClientError::SimulationError { err, .. } => err.clone(), + _ => panic!("unexpected transport error"), } } } From a8b9d32e23166d40101da8c023640d45c95222c2 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Mon, 27 Dec 2021 13:43:29 -0700 Subject: [PATCH 5/7] Add Bank simulation method that works on unfrozen Banks --- banks-server/src/banks_server.rs | 2 +- runtime/src/bank.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index a355a88e5f5..d77f7c550c8 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -270,7 +270,7 @@ impl Banks for BanksServer { units_consumed, } = self .bank(commitment) - .simulate_transaction(sanitized_transaction) + .simulate_transaction_unchecked(sanitized_transaction) { return BanksTransactionResult { result: Some(Err(err)), diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 6f2db139d66..ec394399f4f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3134,6 +3134,15 @@ impl Bank { ) -> TransactionSimulationResult { assert!(self.is_frozen(), "simulation bank must be frozen"); + self.simulate_transaction_unchecked(transaction) + } + + /// Run transactions against a bank without committing the results; does not check if the bank + /// is frozen, enabling use in single-Bank test frameworks + pub fn simulate_transaction_unchecked( + &self, + transaction: SanitizedTransaction, + ) -> TransactionSimulationResult { let number_of_accounts = transaction.message().account_keys_len(); let batch = self.prepare_simulation_batch(transaction); let mut timings = ExecuteTimings::default(); From 7ccefe90cee4fefe294594668032b224d642167e Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Mon, 27 Dec 2021 13:53:17 -0700 Subject: [PATCH 6/7] Add simpler api --- banks-client/src/lib.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/banks-client/src/lib.rs b/banks-client/src/lib.rs index 758fbd5f561..fd9dac06d79 100644 --- a/banks-client/src/lib.rs +++ b/banks-client/src/lib.rs @@ -218,7 +218,7 @@ impl BanksClient { /// Send a transaction and return any preflight (sanitization or simulation) errors, or return /// after the transaction has been rejected or reached the given level of commitment. - pub fn process_transaction_with_preflight( + pub fn process_transaction_with_preflight_and_commitment( &mut self, transaction: Transaction, commitment: CommitmentLevel, @@ -252,6 +252,18 @@ impl BanksClient { }) } + /// Send a transaction and return any preflight (sanitization or simulation) errors, or return + /// after the transaction has been finalized or rejected. + pub fn process_transaction_with_preflight( + &mut self, + transaction: Transaction, + ) -> impl Future> + '_ { + self.process_transaction_with_preflight_and_commitment( + transaction, + CommitmentLevel::default(), + ) + } + /// Send a transaction and return until the transaction has been finalized or rejected. pub fn process_transaction( &mut self, From 00d004f439056d8e8460e0128d545f295052f6fe Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Tue, 28 Dec 2021 10:27:17 -0700 Subject: [PATCH 7/7] Better name: BanksTransactionResultWithSimulation --- banks-client/src/lib.rs | 11 ++++++----- banks-interface/src/lib.rs | 4 ++-- banks-server/src/banks_server.rs | 12 ++++++------ 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/banks-client/src/lib.rs b/banks-client/src/lib.rs index fd9dac06d79..70a03774203 100644 --- a/banks-client/src/lib.rs +++ b/banks-client/src/lib.rs @@ -10,7 +10,7 @@ use { crate::error::BanksClientError, borsh::BorshDeserialize, futures::{future::join_all, Future, FutureExt, TryFutureExt}, - solana_banks_interface::{BanksRequest, BanksResponse, BanksTransactionResult}, + solana_banks_interface::{BanksRequest, BanksResponse, BanksTransactionResultWithSimulation}, solana_program::{ clock::Slot, fee_calculator::FeeCalculator, hash::Hash, program_pack::Pack, pubkey::Pubkey, rent::Rent, sysvar::Sysvar, @@ -125,7 +125,8 @@ impl BanksClient { ctx: Context, transaction: Transaction, commitment: CommitmentLevel, - ) -> impl Future> + '_ { + ) -> impl Future> + '_ + { self.inner .process_transaction_with_preflight_and_commitment_and_context( ctx, @@ -231,13 +232,13 @@ impl BanksClient { commitment, ) .map(|result| match result? { - BanksTransactionResult { + BanksTransactionResultWithSimulation { result: None, simulation_details: _, } => Err(BanksClientError::ClientError( "invalid blockhash or fee-payer", )), - BanksTransactionResult { + BanksTransactionResultWithSimulation { result: Some(Err(err)), simulation_details: Some(simulation_details), } => Err(BanksClientError::SimulationError { @@ -245,7 +246,7 @@ impl BanksClient { logs: simulation_details.logs, units_consumed: simulation_details.units_consumed, }), - BanksTransactionResult { + BanksTransactionResultWithSimulation { result: Some(result), simulation_details: _, } => result.map_err(Into::into), diff --git a/banks-interface/src/lib.rs b/banks-interface/src/lib.rs index 36e07191d87..597cf60167d 100644 --- a/banks-interface/src/lib.rs +++ b/banks-interface/src/lib.rs @@ -38,7 +38,7 @@ pub struct TransactionSimulationDetails { } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -pub struct BanksTransactionResult { +pub struct BanksTransactionResultWithSimulation { pub result: Option>, pub simulation_details: Option, } @@ -60,7 +60,7 @@ pub trait Banks { async fn process_transaction_with_preflight_and_commitment_and_context( transaction: Transaction, commitment: CommitmentLevel, - ) -> BanksTransactionResult; + ) -> BanksTransactionResultWithSimulation; async fn process_transaction_with_commitment_and_context( transaction: Transaction, commitment: CommitmentLevel, diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index d77f7c550c8..6e94d244016 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -2,8 +2,8 @@ use { bincode::{deserialize, serialize}, futures::{future, prelude::stream::StreamExt}, solana_banks_interface::{ - Banks, BanksRequest, BanksResponse, BanksTransactionResult, TransactionConfirmationStatus, - TransactionSimulationDetails, TransactionStatus, + Banks, BanksRequest, BanksResponse, BanksTransactionResultWithSimulation, + TransactionConfirmationStatus, TransactionSimulationDetails, TransactionStatus, }, solana_runtime::{ bank::{Bank, TransactionSimulationResult}, @@ -252,11 +252,11 @@ impl Banks for BanksServer { ctx: Context, transaction: Transaction, commitment: CommitmentLevel, - ) -> BanksTransactionResult { + ) -> BanksTransactionResultWithSimulation { let sanitized_transaction = match SanitizedTransaction::try_from_legacy_transaction(transaction.clone()) { Err(err) => { - return BanksTransactionResult { + return BanksTransactionResultWithSimulation { result: Some(Err(err)), simulation_details: None, }; @@ -272,7 +272,7 @@ impl Banks for BanksServer { .bank(commitment) .simulate_transaction_unchecked(sanitized_transaction) { - return BanksTransactionResult { + return BanksTransactionResultWithSimulation { result: Some(Err(err)), simulation_details: Some(TransactionSimulationDetails { logs, @@ -280,7 +280,7 @@ impl Banks for BanksServer { }), }; } - BanksTransactionResult { + BanksTransactionResultWithSimulation { result: self .process_transaction_with_commitment_and_context(ctx, transaction, commitment) .await,