Skip to content

Commit

Permalink
Fix consistency check for merged sbundles (#82)
Browse files Browse the repository at this point in the history
There were situations where we had merged sbundles that looked like this

bundle1:
   user_tx, allow_revert: no
   backrun1

bundle 2:
  user_tx: allow_revert yes
  backrun

merged sbundle is concat of these two

and consistency check would think that user_tx wat not allowed to revert
enven though there is a situation where we include bundle2
  • Loading branch information
dvush authored and MoeMahhouk committed Jul 29, 2024
1 parent 4c46170 commit 7ae0260
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 20 deletions.
20 changes: 14 additions & 6 deletions crates/rbuilder/src/backtest/backtest_build_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use ahash::HashMap;
use alloy_primitives::utils::format_ether;

use crate::backtest::OrdersWithTimestamp;
use crate::{
backtest::{
execute::{backtest_prepare_ctx_for_block, BacktestBlockInput},
Expand Down Expand Up @@ -76,7 +77,7 @@ pub async fn run_backtest_build_block<ConfigType: LiveBuilderConfig>() -> eyre::
println!("Available orders: {}", orders.len());

if cli.show_orders {
print_order_and_timestamp(&order_and_timestamp, &block_data);
print_order_and_timestamp(&block_data.available_orders, &block_data);
}

let provider_factory = config
Expand Down Expand Up @@ -213,15 +214,22 @@ fn timestamp_ms_to_slot_time(timestamp_ms: u64, block_timestamp: u64) -> i64 {
}

/// Print the available orders sorted by timestamp.
fn print_order_and_timestamp(order_and_timestamp: &HashMap<OrderId, u64>, block_data: &BlockData) {
let mut order_by_ts = order_and_timestamp.clone().into_iter().collect::<Vec<_>>();
order_by_ts.sort_by_key(|(_, ts)| *ts);
for (id, ts) in order_by_ts {
fn print_order_and_timestamp(orders_with_ts: &[OrdersWithTimestamp], block_data: &BlockData) {
let mut order_by_ts = orders_with_ts.to_vec();
order_by_ts.sort_by_key(|owt| owt.timestamp_ms);
for owt in order_by_ts {
let id = owt.order.id();
println!(
"{:>74} ts: {}",
id.to_string(),
timestamp_ms_to_slot_time(ts, timestamp_as_u64(&block_data.onchain_block))
timestamp_ms_to_slot_time(
owt.timestamp_ms,
timestamp_as_u64(&block_data.onchain_block)
)
);
for (tx, optional) in owt.order.list_txs() {
println!(" {:?} {:?}", tx.tx.hash, optional);
}
}
}

Expand Down
54 changes: 40 additions & 14 deletions crates/rbuilder/src/building/built_block_trace.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use super::{BundleErr, ExecutionError, ExecutionResult, OrderErr};
use crate::primitives::{Order, OrderReplacementKey};
use crate::primitives::{Order, OrderId, OrderReplacementKey};
use ahash::{HashMap, HashSet};
use alloy_primitives::{Address, U256};
use alloy_primitives::{Address, TxHash, U256};
use std::collections::hash_map;
use std::time::Duration;
use time::OffsetDateTime;

Expand Down Expand Up @@ -37,8 +38,10 @@ pub enum BuiltBlockTraceError {
DifferentTxsAndReceipts,
#[error("Included order had tx from or to blocked address")]
BlockedAddress,
#[error("Bundle tx reverted that is not revertable")]
BundleTxReverted,
#[error(
"Bundle tx reverted that is not revertable, order: {order_id:?}, tx_hash: {tx_hash:?}"
)]
BundleTxReverted { order_id: OrderId, tx_hash: TxHash },
}

impl BuiltBlockTrace {
Expand Down Expand Up @@ -95,6 +98,8 @@ impl BuiltBlockTrace {
blocklist: &HashSet<Address>,
) -> Result<(), BuiltBlockTraceError> {
let mut replacement_data_count: HashSet<_> = HashSet::default();
let mut bundle_txs_scratchpad = HashMap::default();
let mut executed_tx_hashes_scratchpad = Vec::new();

for res in &self.included_orders {
for order in res.order.original_orders() {
Expand All @@ -110,7 +115,10 @@ impl BuiltBlockTrace {
return Err(BuiltBlockTraceError::DifferentTxsAndReceipts);
}

let mut executed_tx_hashes = Vec::with_capacity(res.txs.len());
let executed_tx_hashes = {
executed_tx_hashes_scratchpad.clear();
&mut executed_tx_hashes_scratchpad
};
for (tx, receipt) in res.txs.iter().zip(res.receipts.iter()) {
let tx = &tx.tx;
executed_tx_hashes.push((tx.hash(), receipt.success));
Expand All @@ -121,16 +129,34 @@ impl BuiltBlockTrace {
}
}

let bundle_txs = res
.order
.list_txs()
.into_iter()
.map(|(tx, can_revert)| (tx.hash(), can_revert))
.collect::<HashMap<_, _>>();
let bundle_txs = {
// we can have the same tx in the list_txs() multiple times(share bundle merging)
// sometimes that tx is marked as revertible and sometimes not
// if tx is marked as revertible in one sub-bundle but not another we consider that tx as revertible
bundle_txs_scratchpad.clear();
for (tx, can_revert) in res.order.list_txs() {
let hash = tx.hash();
match bundle_txs_scratchpad.entry(hash) {
hash_map::Entry::Vacant(entry) => {
entry.insert(can_revert);
}
hash_map::Entry::Occupied(mut entry) => {
let can_revert_stored = entry.get();
if !can_revert_stored && can_revert {
entry.insert(can_revert);
}
}
}
}
&bundle_txs_scratchpad
};
for (executed_hash, success) in executed_tx_hashes {
if let Some(can_revert) = bundle_txs.get(&executed_hash) {
if !success && !can_revert {
return Err(BuiltBlockTraceError::BundleTxReverted);
if let Some(can_revert) = bundle_txs.get(executed_hash) {
if !*success && !can_revert {
return Err(BuiltBlockTraceError::BundleTxReverted {
order_id: res.order.id(),
tx_hash: *executed_hash,
});
}
}
}
Expand Down

0 comments on commit 7ae0260

Please sign in to comment.