Skip to content

Commit

Permalink
improve simulate_bundle errors and response
Browse files Browse the repository at this point in the history
  • Loading branch information
segfaultdoc committed Oct 10, 2023
1 parent 012ee99 commit abcb20b
Show file tree
Hide file tree
Showing 23 changed files with 343 additions and 199 deletions.
5 changes: 5 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions bundle/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ license = { workspace = true }
edition = { workspace = true }

[dependencies]
anchor-lang = { workspace = true }
itertools = { workspace = true }
log = { workspace = true }
serde = { workspace = true }
solana-accounts-db = { workspace = true }
solana-ledger = { workspace = true }
solana-logger = { workspace = true }
solana-measure = { workspace = true }
solana-poh = { workspace = true }
solana-program-runtime = { workspace = true }
solana-runtime = { workspace = true }
solana-sdk = { workspace = true }
Expand Down
22 changes: 15 additions & 7 deletions bundle/src/bundle_execution.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use {
itertools::izip,
log::{debug, trace},
log::*,
solana_accounts_db::{
account_overrides::AccountOverrides, accounts::TransactionLoadResult,
transaction_results::TransactionExecutionResult,
Expand Down Expand Up @@ -77,16 +77,24 @@ impl<'a> LoadAndExecuteBundleOutput<'a> {

#[derive(Clone, Debug, Error)]
pub enum LoadAndExecuteBundleError {
#[error("The bundle processing time was exceeded")]
#[error("Bundle execution timed out")]
ProcessingTimeExceeded(Duration),

#[error("A transaction in the bundle encountered a lock error")]
#[error(
"A transaction in the bundle encountered a lock error: [signature={:?}, transaction_error={:?}]",
signature,
transaction_error
)]
LockError {
signature: Signature,
transaction_error: TransactionError,
},

#[error("A transaction in the bundle failed to execute")]
#[error(
"A transaction in the bundle failed to execute: [signature={:?}, execution_result={:?}",
signature,
execution_result
)]
TransactionError {
signature: Signature,
// Box reduces the size between variants in the Error
Expand Down Expand Up @@ -318,7 +326,7 @@ pub fn load_and_execute_bundle<'a>(
});
saturating_add_assign!(metrics.collect_balances_us, collect_balances_us);

let end = max(
let end = min(
chunk_start.saturating_add(batch.sanitized_transactions().len()),
pre_execution_accounts.len(),
);
Expand Down Expand Up @@ -495,7 +503,7 @@ mod tests {
solana_ledger::genesis_utils::create_genesis_config,
solana_runtime::{bank::Bank, genesis_utils::GenesisConfigInfo},
solana_sdk::{
bundle::{derive_bundle_id_from_sanizited_transactions, SanitizedBundle},
bundle::{derive_bundle_id_from_sanitized_transactions, SanitizedBundle},
clock::MAX_PROCESSING_AGE,
pubkey::Pubkey,
signature::{Keypair, Signer},
Expand Down Expand Up @@ -525,7 +533,7 @@ mod tests {
.map(|tx| SanitizedTransaction::try_from_legacy_transaction(tx.clone()).unwrap())
.collect();

let bundle_id = derive_bundle_id_from_sanizited_transactions(&transactions);
let bundle_id = derive_bundle_id_from_sanitized_transactions(&transactions);

SanitizedBundle {
transactions,
Expand Down
59 changes: 59 additions & 0 deletions bundle/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1 +1,60 @@
use {
crate::bundle_execution::LoadAndExecuteBundleError,
anchor_lang::error::Error,
serde::{Deserialize, Serialize},
solana_poh::poh_recorder::PohRecorderError,
solana_sdk::pubkey::Pubkey,
thiserror::Error,
};

pub mod bundle_execution;

#[derive(Error, Debug, Clone, Serialize, Deserialize, PartialEq)]
pub enum TipError {
#[error("account is missing from bank: {0}")]
AccountMissing(Pubkey),

#[error("Anchor error: {0}")]
AnchorError(String),

#[error("Lock error")]
LockError,

#[error("Error executing initialize programs")]
InitializeProgramsError,

#[error("Error cranking tip programs")]
CrankTipError,
}

impl From<anchor_lang::error::Error> for TipError {
fn from(anchor_err: Error) -> Self {
match anchor_err {
Error::AnchorError(e) => Self::AnchorError(e.error_msg),
Error::ProgramError(e) => Self::AnchorError(e.to_string()),
}
}
}

pub type BundleExecutionResult<T> = Result<T, BundleExecutionError>;

#[derive(Error, Debug, Clone)]
pub enum BundleExecutionError {
#[error("The bank has hit the max allotted time for processing transactions")]
BankProcessingTimeLimitReached,

#[error("The bundle exceeds the cost model")]
ExceedsCostModel,

#[error("Runtime error while executing the bundle: {0}")]
TransactionFailure(#[from] LoadAndExecuteBundleError),

#[error("Error locking bundle because a transaction is malformed")]
LockError,

#[error("PoH record error: {0}")]
PohRecordError(#[from] PohRecorderError),

#[error("Tip payment error {0}")]
TipError(#[from] TipError),
}
3 changes: 3 additions & 0 deletions ci/check-crates.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ printf "%s\n" "${files[@]}"
error_count=0
for file in "${files[@]}"; do
read -r crate_name package_publish workspace < <(toml get "$file" . | jq -r '(.package.name | tostring)+" "+(.package.publish | tostring)+" "+(.workspace | tostring)')
if [ "$crate_name" == "solana-bundle" ]; then
continue
fi
echo "=== $crate_name ($file) ==="

if [[ $package_publish = 'false' ]]; then
Expand Down
1 change: 1 addition & 0 deletions core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ serde_json = { workspace = true }
serial_test = { workspace = true }
solana-accounts-db = { workspace = true }
# See order-crates-for-publishing.py for using this unusual `path = "."`
solana-bundle = { workspace = true }
solana-core = { path = ".", features = ["dev-context-only-utils"] }
solana-logger = { workspace = true }
solana-program-runtime = { workspace = true }
Expand Down
18 changes: 10 additions & 8 deletions core/src/banking_stage/unprocessed_transaction_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@ use {
BankingStageStats, FilterForwardingResults, ForwardOption,
},
crate::{
bundle_stage::{
bundle_stage_leader_metrics::BundleStageLeaderMetrics, result::BundleExecutionError,
},
bundle_stage::bundle_stage_leader_metrics::BundleStageLeaderMetrics,
immutable_deserialized_bundle::ImmutableDeserializedBundle,
},
itertools::Itertools,
min_max_heap::MinMaxHeap,
solana_accounts_db::transaction_error_metrics::TransactionErrorMetrics,
solana_bundle::BundleExecutionError,
solana_measure::{measure, measure_us},
solana_runtime::bank::Bank,
solana_sdk::{
Expand Down Expand Up @@ -1233,19 +1232,22 @@ impl BundleStorage {
debug!("bundle={} executed ok", sanitized_bundle.bundle_id);
// yippee
}
Err(BundleExecutionError::PohRecordError(_)) => {
Err(BundleExecutionError::PohRecordError(e)) => {
// buffer the bundle to the front of the queue to be attempted next slot
debug!("bundle={} poh record error", sanitized_bundle.bundle_id);
debug!(
"bundle={} poh record error: {e:?}",
sanitized_bundle.bundle_id
);
rebuffered_bundles.push(deserialized_bundle);
is_slot_over = true;
}
Err(BundleExecutionError::BankProcessingDone) => {
Err(BundleExecutionError::BankProcessingTimeLimitReached) => {
// buffer the bundle to the front of the queue to be attempted next slot
debug!("bundle={} bank processing done", sanitized_bundle.bundle_id);
rebuffered_bundles.push(deserialized_bundle);
is_slot_over = true;
}
Err(BundleExecutionError::ExecutionError(e)) => {
Err(BundleExecutionError::TransactionFailure(e)) => {
debug!(
"bundle={} execution error: {:?}",
sanitized_bundle.bundle_id, e
Expand All @@ -1262,7 +1264,7 @@ impl BundleStorage {
// Tip errors are _typically_ due to misconfiguration (except for poh record error, bank processing error, exceeds cost model)
// in order to prevent buffering too many bundles, we'll just drop the bundle
}
Err(BundleExecutionError::LockError(_e)) => {
Err(BundleExecutionError::LockError) => {
// lock errors are irrecoverable due to malformed transactions
debug!("bundle={} lock error", sanitized_bundle.bundle_id);
}
Expand Down
1 change: 0 additions & 1 deletion core/src/bundle_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ mod bundle_packet_receiver;
mod bundle_reserved_space_manager;
pub(crate) mod bundle_stage_leader_metrics;
mod committer;
pub mod result;

const MAX_BUNDLE_RETRY_DURATION: Duration = Duration::from_millis(10);
const SLOT_BOUNDARY_CHECK_PERIOD: Duration = Duration::from_millis(10);
Expand Down
24 changes: 14 additions & 10 deletions core/src/bundle_stage/bundle_consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,17 @@ use {
bundle_reserved_space_manager::BundleReservedSpaceManager,
bundle_stage_leader_metrics::BundleStageLeaderMetrics,
committer::Committer,
result::{BundleExecutionError, BundleExecutionResult},
},
consensus_cache_updater::ConsensusCacheUpdater,
immutable_deserialized_bundle::ImmutableDeserializedBundle,
proxy::block_engine_stage::BlockBuilderFeeInfo,
tip_manager::{TipManager, TipPaymentError},
tip_manager::TipManager,
},
solana_accounts_db::transaction_error_metrics::TransactionErrorMetrics,
solana_bundle::bundle_execution::{load_and_execute_bundle, BundleExecutionMetrics},
solana_bundle::{
bundle_execution::{load_and_execute_bundle, BundleExecutionMetrics},
BundleExecutionError, BundleExecutionResult, TipError,
},
solana_cost_model::transaction_cost::TransactionCost,
solana_gossip::cluster_info::ClusterInfo,
solana_measure::{measure, measure_us},
Expand Down Expand Up @@ -246,7 +248,9 @@ impl BundleConsumer {
.increment_process_packets_transactions_us(measure);
r
}
Err(e) => Err(e.into()),
Err(_) => {
Err(BundleExecutionError::LockError)
}
})
.collect::<Vec<_>>());

Expand Down Expand Up @@ -283,7 +287,7 @@ impl BundleConsumer {
&bank_start.bank_creation_time,
bank_start.working_bank.ns_per_slot,
) {
return Err(BundleExecutionError::BankProcessingDone);
return Err(BundleExecutionError::BankProcessingTimeLimitReached);
}

if Self::bundle_touches_tip_pdas(
Expand Down Expand Up @@ -364,7 +368,7 @@ impl BundleConsumer {

let locked_init_tip_programs_bundle = bundle_account_locker
.prepare_locked_bundle(&bundle, &bank_start.working_bank)
.map_err(|_| BundleExecutionError::TipError(TipPaymentError::LockError))?;
.map_err(|_| BundleExecutionError::TipError(TipError::LockError))?;

Self::update_qos_and_execute_record_commit_bundle(
committer,
Expand All @@ -386,7 +390,7 @@ impl BundleConsumer {
locked_init_tip_programs_bundle.sanitized_bundle().bundle_id,
e
);
BundleExecutionError::TipError(TipPaymentError::InitializeProgramsError)
BundleExecutionError::TipError(TipError::InitializeProgramsError)
})?;

bundle_stage_leader_metrics
Expand Down Expand Up @@ -417,7 +421,7 @@ impl BundleConsumer {

let locked_tip_crank_bundle = bundle_account_locker
.prepare_locked_bundle(&bundle, &bank_start.working_bank)
.map_err(|_| BundleExecutionError::TipError(TipPaymentError::LockError))?;
.map_err(|_| BundleExecutionError::TipError(TipError::LockError))?;

Self::update_qos_and_execute_record_commit_bundle(
committer,
Expand All @@ -439,7 +443,7 @@ impl BundleConsumer {
locked_tip_crank_bundle.sanitized_bundle().bundle_id,
e
);
BundleExecutionError::TipError(TipPaymentError::CrankTipError)
BundleExecutionError::TipError(TipError::CrankTipError)
})?;

bundle_stage_leader_metrics
Expand Down Expand Up @@ -555,7 +559,7 @@ impl BundleConsumer {
.accumulate_process_transactions_summary(&ProcessTransactionsSummary {
reached_max_poh_height: matches!(
result.result,
Err(BundleExecutionError::BankProcessingDone)
Err(BundleExecutionError::BankProcessingTimeLimitReached)
| Err(BundleExecutionError::PohRecordError(_))
),
transactions_attempted_execution_count: sanitized_bundle.transactions.len(),
Expand Down
23 changes: 7 additions & 16 deletions core/src/bundle_stage/bundle_packet_receiver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,11 @@ impl BundleReceiver {
mod tests {
use {
super::*,
crate::{
bundle_stage::{
bundle_account_locker::BundleAccountLockerError, result::BundleExecutionError,
},
tip_manager::TipPaymentError,
},
crossbeam_channel::unbounded,
rand::{thread_rng, RngCore},
solana_bundle::bundle_execution::LoadAndExecuteBundleError,
solana_bundle::{
bundle_execution::LoadAndExecuteBundleError, BundleExecutionError, TipError,
},
solana_ledger::genesis_utils::create_genesis_config,
solana_perf::packet::PacketBatch,
solana_poh::poh_recorder::PohRecorderError,
Expand Down Expand Up @@ -474,7 +470,7 @@ mod tests {
let mut results = vec![Ok(()); bundles_to_process.len()];

(bank_processing_done_index..bundles_to_process.len()).for_each(|index| {
results[index] = Err(BundleExecutionError::BankProcessingDone);
results[index] = Err(BundleExecutionError::BankProcessingTimeLimitReached);
});
results
}
Expand Down Expand Up @@ -538,7 +534,7 @@ mod tests {
|bundles_to_process, _stats| {
assert_bundles_same(&bundles, bundles_to_process);
vec![
Err(BundleExecutionError::ExecutionError(
Err(BundleExecutionError::TransactionFailure(
LoadAndExecuteBundleError::ProcessingTimeExceeded(Duration::from_secs(1)),
));
bundles_to_process.len()
Expand Down Expand Up @@ -591,7 +587,7 @@ mod tests {
|bundles_to_process, _stats| {
assert_bundles_same(&bundles, bundles_to_process);
vec![
Err(BundleExecutionError::TipError(TipPaymentError::LockError));
Err(BundleExecutionError::TipError(TipError::LockError));
bundles_to_process.len()
]
}
Expand Down Expand Up @@ -640,12 +636,7 @@ mod tests {
&mut bundle_stage_leader_metrics,
&HashSet::default(),
|bundles_to_process, _stats| {
vec![
Err(BundleExecutionError::LockError(
BundleAccountLockerError::LockingError
));
bundles_to_process.len()
]
vec![Err(BundleExecutionError::LockError); bundles_to_process.len()]
}
));
assert_eq!(bundle_storage.unprocessed_bundles_len(), 0);
Expand Down
Loading

0 comments on commit abcb20b

Please sign in to comment.