Skip to content

Commit

Permalink
Do not settle after the deadline (#3116)
Browse files Browse the repository at this point in the history
# Description
2 colocated solvers reported that `/settle` requests were sent after the
deadline. The expected behaviour is that on each new block a new auction
is cut. So for each new settlement the deadline should be the
`auction.block + settlement_deadline`. It turned out that this is not
the case and after the auction cut, the deadline is calculated as
`current_block + settlement_deadline`. E.g(deadline config is `3`):
- Auction cut at block 10 in the end of the round.
- New block 11 is mined.
- Only then the deadline is calculated as 11+3=14.
- The next auction is being cut at block 11 and the deadline for the
next settlement is also 11+3=14.
- The first settlement gets executed in the end of the block 13 round.
- A new block 14 gets mined and the next settle call is sent after the
deadline.

This should probably explain how the settle call could be sent after the
deadline.

# Changes

- Calculate the deadline based on the auction block, not the current.
- To properly validate the approach and make sure this never happens
again, the proposal is to not send the `/settle` request once the
deadline is reached.
- Another observation is that a log in the
`wait_for_settlement_transaction` function uses a newly calculated
deadline block number for some reason. The PR fixes that and uses the
same value as was used in the `/settle` request. That should help better
understand the root cause.

So in the end the expected behaviour should be observed, where each
subsequent settle request has a deadline for at least 1 block ahead the
previous deadline. So if a settlement gets executed in the end of the
round 13 or gets discarded at block 14, the next settlement still have
the deadline for block 15 which should be sufficient.

## How to test
Existing tests. This might require increasing the settlement deadline by
1.
  • Loading branch information
squadgazzz authored Nov 15, 2024
1 parent 8a573b8 commit 1fe547f
Showing 1 changed file with 28 additions and 22 deletions.
50 changes: 28 additions & 22 deletions crates/autopilot/src/run_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use {
database::order_events::OrderEventLabel,
ethcontract::U256,
ethrpc::block_stream::BlockInfo,
futures::{future::BoxFuture, TryFutureExt},
futures::{future::BoxFuture, FutureExt, TryFutureExt},
itertools::Itertools,
model::solver_competition::{
CompetitionAuction,
Expand Down Expand Up @@ -239,7 +239,7 @@ impl RunLoop {
}

let competition_simulation_block = self.eth.current_block().borrow().number;
let block_deadline = competition_simulation_block + self.config.submission_deadline;
let block_deadline = auction.block + self.config.submission_deadline;

// Post-processing should not be executed asynchronously since it includes steps
// of storing all the competition/auction-related data to the DB.
Expand Down Expand Up @@ -315,7 +315,7 @@ impl RunLoop {
let driver_ = driver.clone();

let settle_fut = async move {
tracing::info!(driver = %driver_.name, "settling");
tracing::info!(driver = %driver_.name, solution = %solution_id, "settling");
let submission_start = Instant::now();

match self_
Expand Down Expand Up @@ -797,24 +797,31 @@ impl RunLoop {
auction_id: i64,
submission_deadline_latest_block: u64,
) -> Result<TxId, SettleError> {
let request = settle::Request {
solution_id,
submission_deadline_latest_block,
auction_id,
};
let settle = async move {
let current_block = self.eth.current_block().borrow().number;
anyhow::ensure!(
current_block < submission_deadline_latest_block,
"submission deadline was missed while waiting for the settlement queue"
);

let request = settle::Request {
solution_id,
submission_deadline_latest_block,
auction_id,
};
driver
.settle(&request, self.config.max_settlement_transaction_wait)
.await
}
.boxed();

let wait_for_settlement_transaction = self
.wait_for_settlement_transaction(auction_id, solver, submission_deadline_latest_block)
.boxed();

// Wait for either the settlement transaction to be mined or the driver returned
// a result.
let result = match futures::future::select(
Box::pin(self.wait_for_settlement_transaction(
auction_id,
self.config.submission_deadline,
solver,
)),
Box::pin(driver.settle(&request, self.config.max_settlement_transaction_wait)),
)
.await
{
let result = match futures::future::select(wait_for_settlement_transaction, settle).await {
futures::future::Either::Left((res, _)) => res,
futures::future::Either::Right((driver_result, wait_for_settlement_transaction)) => {
match driver_result {
Expand All @@ -841,12 +848,11 @@ impl RunLoop {
async fn wait_for_settlement_transaction(
&self,
auction_id: i64,
max_blocks_wait: u64,
solver: eth::Address,
submission_deadline_latest_block: u64,
) -> Result<eth::TxId, SettleError> {
let current = self.eth.current_block().borrow().number;
let deadline = current.saturating_add(max_blocks_wait);
tracing::debug!(%current, %deadline, %auction_id, "waiting for tag");
tracing::debug!(%current, deadline=%submission_deadline_latest_block, %auction_id, "waiting for tag");
loop {
let block = ethrpc::block_stream::next_block(self.eth.current_block()).await;
// Run maintenance to ensure the system processed the last available block so
Expand All @@ -869,7 +875,7 @@ impl RunLoop {
);
}
}
if block.number >= deadline {
if block.number >= submission_deadline_latest_block {
break;
}
}
Expand Down

0 comments on commit 1fe547f

Please sign in to comment.