Skip to content

Commit

Permalink
dex: hotfix to avoid discarding EventPositionExecution
Browse files Browse the repository at this point in the history
Closes #4007

The bug results from a hazardous API I created. The `apply` method does _not_
apply the events to the underlying state, it extracts them and returns them to
the caller, who can silently discard them.

A better fix is outlined in the comment:

```
// We have to manually extract events and push them down to the state to avoid losing them.
// TODO: in a commit not intended to be cherry-picked, we should fix this hazardous API:
// - rename `StateDelta::apply` to `StateDelta::apply_extracting_events`
// - add `StateDelta::apply_with_events` that pushes the events down.
// - go through all uses of `apply_extracting_events` and determine what behavior is correct
```

However, I didn't do this in this commit, because my hope is that we can have
something easily cherry-pickable into a point release, which can be deployed to
unblock the DEX work. After applying it to both `main` and a release branch, we
can then do the fix for the bug class.

(cherry picked from commit 318d3d1)
  • Loading branch information
hdevalence authored and conorsch committed Mar 13, 2024
1 parent 88d8665 commit 6c1c82e
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion crates/core/component/dex/src/component/router/fill_route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,16 @@ async fn fill_route_inner<S: StateWrite + Sized>(
tracing::debug!(?swap_execution, "returning swap execution of filled route");

// Apply the state transaction now that we've reached the end without errors.
this.apply();
//
// We have to manually extract events and push them down to the state to avoid losing them.
// TODO: in a commit not intended to be cherry-picked, we should fix this hazardous API:
// - rename `StateDelta::apply` to `StateDelta::apply_extracting_events`
// - add `StateDelta::apply_with_events` that pushes the events down.
// - go through all uses of `apply_extracting_events` and determine what behavior is correct
let (mut state, events) = this.apply();
for event in events {
state.record(event);
}

let fill_elapsed = fill_start.elapsed();
metrics::histogram!(metrics::DEX_ROUTE_FILL_DURATION).record(fill_elapsed);
Expand Down

0 comments on commit 6c1c82e

Please sign in to comment.