From 6c1c82ec2b76b39d12f70575e874108b5afe5303 Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Tue, 12 Mar 2024 22:32:24 -0400 Subject: [PATCH] dex: hotfix to avoid discarding EventPositionExecution 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 318d3d18b0add9436d259c9dbc0a9dc5a9c47e8e) --- .../component/dex/src/component/router/fill_route.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/core/component/dex/src/component/router/fill_route.rs b/crates/core/component/dex/src/component/router/fill_route.rs index 999b4e21e0..6f815f41ea 100644 --- a/crates/core/component/dex/src/component/router/fill_route.rs +++ b/crates/core/component/dex/src/component/router/fill_route.rs @@ -255,7 +255,16 @@ async fn fill_route_inner( 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);