Skip to content

Commit

Permalink
Fix native solution submission deadline (#2452)
Browse files Browse the repository at this point in the history
# Description
There is a nasty bug in the native solution submission logic. We would
update the deadline in every iteration of the loop, thus keep it always
1 minute in the future. The cancellation due to expiry would thus never
trigger (leading to continued `FeeTooLowToCompete` errors in xDAI.


# Changes
- [x] Move deadline computation outside of loop
- [x] Build a somewhat involved e2e test that catches this bug. This
involves starting and stopping anvil mining behavior.

## How to test
The new test fails without the change and passes now
  • Loading branch information
fleupold authored Mar 1, 2024
1 parent 842ee0a commit 61b9a6e
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 1 deletion.
3 changes: 2 additions & 1 deletion crates/driver/src/domain/mempools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ impl Mempools {
return Err(Error::Disabled);
}

let deadline = mempool.config().deadline();
let tx = eth::Tx {
// boundary.tx() does not populate the access list
access_list: settlement.access_list.clone(),
Expand All @@ -114,7 +115,7 @@ impl Mempools {
let hash = mempool.submit(tx.clone(), settlement.gas, solver).await?;
loop {
// Wait for the next block to be mined or we time out.
if tokio::time::timeout_at(mempool.config().deadline(), block_stream.next())
if tokio::time::timeout_at(deadline, block_stream.next())
.await
.is_err()
{
Expand Down
21 changes: 21 additions & 0 deletions crates/e2e/src/nodes/local_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,27 @@ impl<T: Transport> TestNodeApi<T> {
CallFuture::new(self.transport.execute("evm_mine", vec![]))
}

pub fn disable_automine(&self) -> CallFuture<(), T::Out> {
CallFuture::new(
self.transport
.execute("evm_setAutomine", vec![serde_json::json!(false)]),
)
}

pub fn set_mining_interval(&self, seconds: usize) -> CallFuture<(), T::Out> {
CallFuture::new(
self.transport
.execute("evm_setIntervalMining", vec![serde_json::json!(seconds)]),
)
}

pub fn set_block_gas_limit(&self, limit: usize) -> CallFuture<bool, T::Out> {
CallFuture::new(
self.transport
.execute("evm_setBlockGasLimit", vec![serde_json::json!(limit)]),
)
}

pub fn set_balance(&self, address: &H160, balance: &U256) -> CallFuture<(), T::Out> {
let json_address = serde_json::json!(address);
let json_balance = serde_json::json!(format!("{:#032x}", balance));
Expand Down
2 changes: 2 additions & 0 deletions crates/e2e/src/setup/colocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ missing-pool-cache-time = "1h"
[submission]
gas-price-cap = "1000000000000"
logic = "native"
max-confirm-time= "2s"
[[submission.mempool]]
mempool = "public"
Expand Down
7 changes: 7 additions & 0 deletions crates/e2e/src/setup/onchain_components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ impl TestAccount {
SecretKeyRef::from(&SecretKey::from_slice(self.private_key()).unwrap()),
)
}

pub async fn nonce(&self, web3: &Web3) -> U256 {
web3.eth()
.transaction_count(self.address(), None)
.await
.unwrap()
}
}

#[derive(Default)]
Expand Down
1 change: 1 addition & 0 deletions crates/e2e/tests/e2e/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ mod refunder;
mod replace_order;
mod smart_contract_orders;
mod solver_competition;
mod submission;
mod tracking_insufficient_funds;
mod univ2;
mod vault_balances;
145 changes: 145 additions & 0 deletions crates/e2e/tests/e2e/submission.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
use {
e2e::{nodes::local_node::TestNodeApi, setup::*, tx, tx_value},
ethcontract::{BlockId, H160, H256, U256},
futures::{Stream, StreamExt},
model::{
order::{OrderCreation, OrderKind},
signature::EcdsaSigningScheme,
},
secp256k1::SecretKey,
shared::ethrpc::Web3,
std::time::Duration,
web3::signing::SecretKeyRef,
};

#[tokio::test]
#[ignore]
async fn local_node_test() {
run_test(test_cancel_on_expiry).await;
}

async fn test_cancel_on_expiry(web3: Web3) {
let mut onchain = OnchainComponents::deploy(web3.clone()).await;

let [solver] = onchain.make_solvers(to_wei(10)).await;
let nonce = solver.nonce(&web3).await;
let [trader] = onchain.make_accounts(to_wei(10)).await;
let [token] = onchain
.deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000))
.await;

tx!(
trader.account(),
onchain
.contracts()
.weth
.approve(onchain.contracts().allowance, to_wei(3))
);
tx_value!(
trader.account(),
to_wei(3),
onchain.contracts().weth.deposit()
);

tracing::info!("Starting services.");
let services = Services::new(onchain.contracts()).await;
services.start_protocol(solver.clone()).await;

// Disable auto-mine so we don't accidentally mine a settlement
web3.api::<TestNodeApi<_>>()
.disable_automine()
.await
.expect("Must be able to disable automine");

tracing::info!("Placing order");
let balance = token.balance_of(trader.address()).call().await.unwrap();
assert_eq!(balance, 0.into());
let order = OrderCreation {
sell_token: onchain.contracts().weth.address(),
sell_amount: to_wei(2),
fee_amount: to_wei(1),
buy_token: token.address(),
buy_amount: to_wei(1),
valid_to: model::time::now_in_epoch_seconds() + 300,
kind: OrderKind::Buy,
..Default::default()
}
.sign(
EcdsaSigningScheme::Eip712,
&onchain.contracts().domain_separator,
SecretKeyRef::from(&SecretKey::from_slice(trader.private_key()).unwrap()),
);
services.create_order(&order).await.unwrap();

// Start tracking confirmed blocks so we can find the transaction later
let block_stream = web3
.eth_filter()
.create_blocks_filter()
.await
.expect("must be able to create blocks filter")
.stream(Duration::from_millis(50));

// Wait for settlement tx to appear in txpool
wait_for_condition(TIMEOUT, || async {
get_pending_tx(solver.account().address(), &web3)
.await
.is_some()
})
.await
.unwrap();

// Restart mining, but with blocks that are too small to fit the settlement
web3.api::<TestNodeApi<_>>()
.set_block_gas_limit(100_000)
.await
.expect("Must be able to set block gas limit");
web3.api::<TestNodeApi<_>>()
.set_mining_interval(1)
.await
.expect("Must be able to set mining interval");

// Wait for cancellation tx to appear
wait_for_condition(TIMEOUT, || async { solver.nonce(&web3).await == nonce + 1 })
.await
.unwrap();

// Check that it's actually a cancellation
let tx = tokio::time::timeout(
TIMEOUT,
get_confirmed_transaction(solver.account().address(), &web3, block_stream),
)
.await
.unwrap();
assert_eq!(tx.to, Some(solver.account().address()))
}

async fn get_pending_tx(account: H160, web3: &Web3) -> Option<web3::types::Transaction> {
let txpool = web3
.txpool()
.content()
.await
.expect("must be able to inspect mempool");
txpool.pending.get(&account)?.values().next().cloned()
}

async fn get_confirmed_transaction(
account: H160,
web3: &Web3,
block_stream: impl Stream<Item = Result<H256, web3::Error>>,
) -> web3::types::Transaction {
let mut block_stream = Box::pin(block_stream);
loop {
let block_hash = block_stream.next().await.unwrap().unwrap();
let block = web3
.eth()
.block_with_txs(BlockId::Hash(block_hash))
.await
.expect("must be able to get block by hash")
.expect("block not found");
for tx in block.transactions {
if tx.from == Some(account) {
return tx;
}
}
}
}

0 comments on commit 61b9a6e

Please sign in to comment.