Skip to content

Commit

Permalink
Add asserts to verify specific fork behavior
Browse files Browse the repository at this point in the history
When a fork happens such that we have separate requests submitted for the builder,
we are expecting the builder to be aware of these and to issue any missing transactions
for a block in a subsequent block.  At the moment, this is not occurring, and the
assert at the bottom of the `test_builder_order_chain_fork` that ensures that the
two transaction history forks don't match is backwards.

Additional asserts have been added when producing the next propsoal to verify that
the transaction lists are the same in the event that their previous lists were the
same, or differ if their previous lists differ while not having any transactions
in the fork, or if the transactions are the same when the previous proposal contained
all of the missing transactions.
  • Loading branch information
Ayiga committed Sep 5, 2024
1 parent 444336d commit 0e1b39a
Showing 1 changed file with 17 additions and 7 deletions.
24 changes: 17 additions & 7 deletions crates/marketplace/src/testing/order_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ async fn test_builder_order_chain_fork() {
// get transactions submitted in previous rounds, [] for genesis
// and simulate the block built from those
let transactions = prev_proposed_transactions_branch_1
.take()
.clone()
.unwrap_or_default();
let (quorum_proposal, quorum_proposal_msg, da_proposal_msg, builder_state_id) =
calc_proposal_msg(
Expand Down Expand Up @@ -397,7 +397,7 @@ async fn test_builder_order_chain_fork() {
// get transactions submitted in previous rounds, [] for genesis
// and simulate the block built from those
let transactions = prev_proposed_transactions_branch_2
.take()
.clone()
.unwrap_or_default();
let (quorum_proposal, quorum_proposal_msg, da_proposal_msg, builder_state_id) =
calc_proposal_msg(
Expand All @@ -412,7 +412,7 @@ async fn test_builder_order_chain_fork() {

// send quorum and DA proposals for this round
// we also need to send out the message for the fork-ed chain although it's not forked yet
// to prevent builders resend the transactions we've already committeed
// to prevent builders resend the transactions we've already committed
senders
.da_proposal
.broadcast(da_proposal_msg)
Expand Down Expand Up @@ -457,24 +457,34 @@ async fn test_builder_order_chain_fork() {
.await
.unwrap();

if prev_proposed_transactions_branch_1 == prev_proposed_transactions_branch_2 {
assert_eq!(transactions_branch_1, transactions_branch_2, "if the previous proposed transactions are the same, then the new transactions should also be the same");
} else if prev_proposed_transactions_branch_2.map(|txs| txs.len()) == Some(0) {
assert_ne!(transactions_branch_1, transactions_branch_2, "if the previous proposed transactions differ and the previous proposed transactions is empty, then the new transactions should also differ");
} else {
assert_eq!(transactions_branch_1, transactions_branch_2, "if the previous proposed transactions differ, then the new transactions should be the same, as they should now have been repaired");
}

let transactions_branch_2 = fork_round_behavior.process_transactions(transactions_branch_2);
if transactions_branch_2 != transactions_branch_1 {
tracing::debug!("Fork Exist.")
} else {
tracing::debug!("No fork.");
}

let transactions_branch_2 = fork_round_behavior.process_transactions(transactions_branch_2);

prev_proposed_transactions_branch_1 = Some(transactions_branch_1.clone());
prev_proposed_transactions_branch_2 = Some(transaction_history_branch_2.clone());
prev_proposed_transactions_branch_2 = Some(transactions_branch_2.clone());

// save transactions to history
transaction_history_branch_1.extend(transactions_branch_1);
transaction_history_branch_2.extend(transactions_branch_2);
}

// This will fail if no fork has occurred
assert!(transaction_history_branch_1 != transaction_history_branch_2);
assert_eq!(
transaction_history_branch_1, transaction_history_branch_2,
"even with a fork, the transaction history branches should match"
);
// the test will fail if any transaction is re-ordered
assert!(order_check(
transaction_history_branch_1,
Expand Down

0 comments on commit 0e1b39a

Please sign in to comment.