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.

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

Filter in-flight orders #2123

Merged
merged 2 commits into from
Dec 5, 2023
Merged

Filter in-flight orders #2123

merged 2 commits into from
Dec 5, 2023

Conversation

MartinquaXD
Copy link
Contributor

Description

Although the autopilot currently waits until the driver reports that the settlement was mined the auction of the next run loop might not yet be aware that the orders of the previous settlement were solved.
If a solver then tries to solve those orders again it will run into simulation failures.

Changes

We now filter out in-flight orders from the auction if it was built before the block where the in-flight orders were settled.

@MartinquaXD MartinquaXD requested a review from a team as a code owner December 5, 2023 16:55
@@ -54,6 +55,7 @@ pub struct RunLoop {
pub score_cap: U256,
pub max_settlement_transaction_wait: Duration,
pub solve_deadline: Duration,
pub in_flight_orders: Arc<Mutex<InFlightOrders>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need synchronisation here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we populate the data in a method that takes a shared reference to self.

/// to fill an order a second time.
async fn remove_in_flight_orders(&self, mut auction: Auction) -> Auction {
let prev_settlement = self.in_flight_orders.lock().unwrap().tx_hash;
let tx_receipt = self.web3.eth().transaction_receipt(prev_settlement).await;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if in the first iteration the tx_hash is initialized with Default::default()?

Copy link
Contributor Author

@MartinquaXD MartinquaXD Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then transaction_receipt() would try to find the block that contains tx_hash 0x000... which does not exist so we fallback to assuming the previous auction was settled in block u64::MAX so we discard all in-flight orders we have at the time (which are 0 due to Default::default()).

Copy link
Contributor

@sunce86 sunce86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lg

Copy link
Contributor

@fleupold fleupold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fleupold fleupold merged commit 593aff3 into main Dec 5, 2023
8 checks passed
@fleupold fleupold deleted the filter-in-flight-orders branch December 5, 2023 17:34
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants