Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(builder): handle more tx sender errors #918

Merged
merged 1 commit into from
Nov 25, 2024
Merged

fix(builder): handle more tx sender errors #918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 52 additions & 11 deletions crates/builder/src/bundle_sender.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,14 @@ enum SendBundleAttemptResult {
NoOperationsAfterFeeFilter,
// There were no operations after the bundle was simulated
NoOperationsAfterSimulation,
// Underpriced
Underpriced,
// Replacement Underpriced
ReplacementUnderpriced,
// Condition not met
ConditionNotMet,
// Rejected
Rejected,
// Nonce too low
NonceTooLow,
}
Expand Down Expand Up @@ -296,19 +300,33 @@ where
info!("Nonce too low, starting new bundle attempt");
state.reset();
}
Ok(SendBundleAttemptResult::Underpriced) => {
info!(
"Bundle underpriced, marking as underpriced. Num fee increases {:?}",
inner.fee_increase_count
);
state.update(InnerState::Building(inner.underpriced(block_number)));
}
Ok(SendBundleAttemptResult::ReplacementUnderpriced) => {
info!("Replacement transaction underpriced, marking as underpriced. Num fee increases {:?}", inner.fee_increase_count);
// unabandon to allow fee estimation to consider any submitted transactions, wait for next trigger
state.transaction_tracker.unabandon();
state.update(InnerState::Building(
inner.replacement_underpriced(block_number),
));
state.update(InnerState::Building(inner.underpriced(block_number)));
}
Ok(SendBundleAttemptResult::ConditionNotMet) => {
info!("Condition not met, notifying proposer and starting new bundle attempt");
self.proposer.notify_condition_not_met();
state.update(InnerState::Building(inner.retry()));
}
Ok(SendBundleAttemptResult::Rejected) => {
// Bundle was rejected, try with a higher price
// May want to consider a simple retry instead of increasing fees, but this should be rare
info!(
"Bundle rejected, assuming underpriced. Num fee increases {:?}",
inner.fee_increase_count
);
state.update(InnerState::Building(inner.underpriced(block_number)));
}
Err(error) => {
error!("Bundle send error {error:?}");
self.metrics.bundle_txns_failed.increment(1);
Expand Down Expand Up @@ -418,8 +436,10 @@ where
self.metrics.soft_cancellations.increment(1);
state.reset();
}
Err(TransactionTrackerError::ReplacementUnderpriced) => {
info!("Replacement transaction underpriced during cancellation, trying again");
Err(TransactionTrackerError::Rejected)
| Err(TransactionTrackerError::Underpriced)
| Err(TransactionTrackerError::ReplacementUnderpriced) => {
info!("Transaction underpriced/rejected during cancellation, trying again. {cancel_res:?}");
if inner.fee_increase_count >= self.settings.max_cancellation_fee_increases {
// abandon the cancellation
warn!("Abandoning cancellation after max fee increases {}, starting new bundle attempt", inner.fee_increase_count);
Expand All @@ -439,7 +459,14 @@ where
info!("Nonce too low during cancellation, starting new bundle attempt");
state.reset();
}
Err(e) => {
Err(TransactionTrackerError::ConditionNotMet) => {
error!(
"Unexpected condition not met during cancellation, starting new bundle attempt"
);
self.metrics.cancellation_txns_failed.increment(1);
state.reset();
}
Err(TransactionTrackerError::Other(e)) => {
error!("Failed to cancel transaction, moving back to building state: {e:#?}");
self.metrics.cancellation_txns_failed.increment(1);
state.reset();
Expand Down Expand Up @@ -580,6 +607,11 @@ where
warn!("Bundle attempt nonce too low");
Ok(SendBundleAttemptResult::NonceTooLow)
}
Err(TransactionTrackerError::Underpriced) => {
self.metrics.bundle_txn_underpriced.increment(1);
warn!("Bundle attempt underpriced");
Ok(SendBundleAttemptResult::Underpriced)
}
Err(TransactionTrackerError::ReplacementUnderpriced) => {
self.metrics.bundle_replacement_underpriced.increment(1);
warn!("Bundle attempt replacement transaction underpriced");
Expand All @@ -590,9 +622,14 @@ where
warn!("Bundle attempt condition not met");
Ok(SendBundleAttemptResult::ConditionNotMet)
}
Err(e) => {
Err(TransactionTrackerError::Rejected) => {
self.metrics.bundle_txn_rejected.increment(1);
warn!("Bundle attempt rejected");
Ok(SendBundleAttemptResult::Rejected)
}
Err(TransactionTrackerError::Other(e)) => {
error!("Failed to send bundle with unexpected error: {e:?}");
Err(e.into())
Err(e)
}
}
}
Expand Down Expand Up @@ -882,10 +919,10 @@ impl BuildingState {
self
}

// Mark a replacement as underpriced
// Mark as underpriced
//
// The next state will wait for a trigger to reduce bundle building loops
fn replacement_underpriced(self, block_number: u64) -> Self {
fn underpriced(self, block_number: u64) -> Self {
let ui = if let Some(underpriced_info) = self.underpriced_info {
underpriced_info
} else {
Expand Down Expand Up @@ -1179,12 +1216,16 @@ struct BuilderMetric {
bundle_txns_nonce_used: Counter,
#[metric(describe = "the count of bundle transactions fee increase events.")]
bundle_txn_fee_increases: Counter,
#[metric(describe = "the count of bundle transactions underprice replaced events.")]
#[metric(describe = "the count of bundle transactions underpriced events.")]
bundle_txn_underpriced: Counter,
#[metric(describe = "the count of bundle transactions underpriced replacement events.")]
bundle_replacement_underpriced: Counter,
#[metric(describe = "the count of bundle transactions nonce too low events.")]
bundle_txn_nonce_too_low: Counter,
#[metric(describe = "the count of bundle transactions condition not met events.")]
bundle_txn_condition_not_met: Counter,
#[metric(describe = "the count of bundle transactions rejected.")]
bundle_txn_rejected: Counter,
#[metric(describe = "the count of cancellation bundle transactions sent events.")]
cancellation_txns_sent: Counter,
#[metric(describe = "the count of cancellation bundle transactions mined events.")]
Expand Down
13 changes: 5 additions & 8 deletions crates/builder/src/sender/bloxroute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,12 @@ struct BloxrouteResponse {

impl From<jsonrpsee::core::ClientError> for TxSenderError {
fn from(value: jsonrpsee::core::ClientError) -> Self {
match &value {
jsonrpsee::core::ClientError::Call(e) => {
if super::is_replacement_underpriced(e.message()) {
TxSenderError::ReplacementUnderpriced
} else {
TxSenderError::Other(value.into())
}
if let jsonrpsee::core::ClientError::Call(e) = &value {
if let Some(e) = super::parse_known_call_execution_failed(e.message()) {
return e;
}
_ => TxSenderError::Other(value.into()),
}

TxSenderError::Other(value.into())
}
}
71 changes: 50 additions & 21 deletions crates/builder/src/sender/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ pub(crate) enum TxStatus {
/// Errors from transaction senders
#[derive(Debug, thiserror::Error)]
pub(crate) enum TxSenderError {
/// Transaction was underpriced and dropped
#[error("transaction underpriced")]
Underpriced,
/// Replacement transaction was underpriced
#[error("replacement transaction underpriced")]
ReplacementUnderpriced,
Expand All @@ -61,6 +64,12 @@ pub(crate) enum TxSenderError {
/// Conditional value not met
#[error("storage slot value condition not met")]
ConditionNotMet,
/// Transaction was rejected
///
/// This is a catch-all for when a transaction is rejected for any reason
/// that can be solved with a retry.
#[error("transaction rejected")]
Rejected,
/// Soft cancellation failed
#[error("soft cancel failed")]
SoftCancelFailed,
Expand Down Expand Up @@ -230,33 +239,53 @@ impl From<ProviderError> for TxSenderError {
match &value {
ProviderError::RPC(e) => {
if let Some(e) = e.as_error_resp() {
if is_replacement_underpriced(&e.message) {
return TxSenderError::ReplacementUnderpriced;
// geth, erigon, reth
} else if e.message.contains("nonce too low") {
return TxSenderError::NonceTooLow;
// Arbitrum conditional sender error message
// TODO push them to use a specific error code and to return the specific slot that is not met.
} else if e
.message
.to_lowercase()
.contains("storage slot value condition not met")
{
return TxSenderError::ConditionNotMet;
// Client impls use different error codes, just match on the message
if let Some(e) = parse_known_call_execution_failed(&e.message) {
e
} else {
TxSenderError::Other(value.into())
}
} else {
TxSenderError::Other(value.into())
}
TxSenderError::Other(value.into())
}
_ => TxSenderError::Other(value.into()),
}
}
}

fn is_replacement_underpriced(e: &str) -> bool {
// geth
e.to_lowercase().contains("replacement transaction underpriced") ||
// erigon
e.to_lowercase().contains("could not replace existing tx") ||
// reth
e.to_lowercase().contains("insufficient gas price to replace existing transaction")
fn parse_known_call_execution_failed(e: &str) -> Option<TxSenderError> {
match &e.to_lowercase() {
// geth
x if x.contains("transaction underpriced") => Some(TxSenderError::Underpriced),
// erigon
x if x.contains("underpriced") => Some(TxSenderError::Underpriced),
// reth
x if x.contains("transaction discarded outright due to pool size constraints") => {
Some(TxSenderError::Underpriced)
}
// geth. Reth & erigon don't have similar
x if x.contains("future transaction tries to replace pending") => {
Some(TxSenderError::Rejected)
}
// geth
x if x.contains("replacement transaction underpriced") => {
Some(TxSenderError::ReplacementUnderpriced)
}
// erigon
x if x.contains("could not replace existing tx") => {
Some(TxSenderError::ReplacementUnderpriced)
}
// reth
x if x.contains("insufficient gas price to replace existing transaction") => {
Some(TxSenderError::ReplacementUnderpriced)
}
// geth, erigon, reth
x if x.contains("nonce too low") => Some(TxSenderError::NonceTooLow),
// Arbitrum conditional sender error message
x if x.contains("storage slot value condition not met") => {
Some(TxSenderError::ConditionNotMet)
}
_ => None,
}
}
44 changes: 6 additions & 38 deletions crates/builder/src/transaction_tracker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,14 @@ pub(crate) trait TransactionTracker: Send + Sync {
pub(crate) enum TransactionTrackerError {
#[error("nonce too low")]
NonceTooLow,
#[error("transaction underpriced")]
Underpriced,
#[error("replacement transaction underpriced")]
ReplacementUnderpriced,
#[error("storage slot value condition not met")]
ConditionNotMet,
#[error("rejected")]
Rejected,
/// All other errors
#[error(transparent)]
Other(#[from] anyhow::Error),
Expand Down Expand Up @@ -507,10 +511,12 @@ impl From<TxSenderError> for TransactionTrackerError {
fn from(value: TxSenderError) -> Self {
match value {
TxSenderError::NonceTooLow => TransactionTrackerError::NonceTooLow,
TxSenderError::Underpriced => TransactionTrackerError::Underpriced,
TxSenderError::ReplacementUnderpriced => {
TransactionTrackerError::ReplacementUnderpriced
}
TxSenderError::ConditionNotMet => TransactionTrackerError::ConditionNotMet,
TxSenderError::Rejected => TransactionTrackerError::Rejected,
TxSenderError::SoftCancelFailed => {
TransactionTrackerError::Other(anyhow::anyhow!("soft cancel failed"))
}
Expand Down Expand Up @@ -728,44 +734,6 @@ mod tests {
tracker.send_transaction(tx, &exp).await.unwrap();
}

// TODO(#295): fix dropped status
// #[tokio::test]
// async fn test_wait_for_update_dropped() {
// let (mut sender, mut provider) = create_base_config();
// sender.expect_address().return_const(Address::ZERO);

// sender
// .expect_get_transaction_status()
// .returning(move |_a| Box::pin(async { Ok(TxStatus::Dropped) }));

// sender.expect_send_transaction().returning(move |_a, _b| {
// Box::pin(async {
// Ok(SentTxInfo {
// nonce: U256::ZERO,
// tx_hash: B256::zero(),
// })
// })
// });

// provider
// .expect_get_transaction_count()
// .returning(move |_a| Ok(U256::ZERO));

// provider.expect_get_block_number().returning(move || Ok(1));

// let tracker = create_tracker(sender, provider).await;

// let tx = Eip1559TransactionRequest::new().nonce(0);
// let exp = ExpectedStorage::default();
// let _sent_transaction = tracker.send_transaction(tx.into(), &exp).await.unwrap();
// let tracker_update = tracker.wait_for_update().await.unwrap();

// assert!(matches!(
// tracker_update,
// TrackerUpdate::LatestTxDropped { .. }
// ));
// }

#[tokio::test]
async fn test_check_for_update_nonce_used() {
let (mut sender, mut provider) = create_base_config();
Expand Down
Loading