route_and_fill proptests

crates/bin/pcli/src/command/query/dex.rs

@@ -228,6 +228,7 @@ impl DexCmd {
             swap_execution.output.format(&cache),
         );
 
+        println!("Traces len: {}", swap_execution.traces.len());
         // Try to make a nice table of execution traces. To do this, first find
         // the max length of any subtrace:
         let max_trace_len = swap_execution

crates/core/component/dex/Cargo.toml

@@ -84,3 +84,4 @@ rand = {workspace = true}
 tracing-subscriber = {workspace = true}
 rand_chacha = {workspace = true}
 itertools = "0.11"
+penumbra-test-subscriber = { workspace = true }

crates/core/component/dex/proptest-regressions/component/router/tests.txt

@@ -0,0 +1,7 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc f2dc95dae6b7bbc7082a5aff58b8bbceebe679b754995af2a914b9f3f6f69632 # shrinks to input_amount = 7265245127025560547, num_paths_1 = 11, num_paths_2 = 7, num_paths_3 = 19

crates/core/component/dex/src/component/arb.rs

@@ -64,6 +64,14 @@ pub trait Arbitrage: StateWrite + Sized {
         // output from the route-and-fill, plus the unfilled input.
         let total_output = output + unfilled_input;
 
+        tracing::debug!(
+            ?filled_input,
+            ?output,
+            ?unfilled_input,
+            ?total_output,
+            "arb: finished route-and-fill"
+        );
+
         // Now "repay" the flash loan by subtracting it from the total output.
         let Some(arb_profit) = total_output.checked_sub(&flash_loan.amount) else {
             // This shouldn't happen, but because route-and-fill prioritizes

crates/core/component/dex/src/component/router/fill_route.rs

@@ -1,5 +1,5 @@
 use std::{
-    collections::{BTreeMap, BTreeSet},
+    collections::{BTreeMap, BTreeSet, HashSet},
     pin::Pin,
 };
 
@@ -74,7 +74,7 @@ pub trait FillRoute: StateWrite + Sized {
         input: Value,
         hops: &[asset::Id],
         spill_price: Option<U128x128>,
-    ) -> Result<SwapExecution, FillError> {
+    ) -> Result<(SwapExecution, HashSet<position::Id>), FillError> {
         fill_route_inner(self, input, hops, spill_price, true).await
     }
 }
@@ -87,7 +87,8 @@ async fn fill_route_inner<S: StateWrite + Sized>(
     hops: &[asset::Id],
     spill_price: Option<U128x128>,
     ensure_progress: bool,
-) -> Result<SwapExecution, FillError> {
+) -> Result<(SwapExecution, HashSet<position::Id>), FillError> {
+    tracing::debug!(?spill_price, ?ensure_progress, "filling route inner");
     let fill_start = std::time::Instant::now();
 
     // Build a transaction for this execution, so if we error out at any
@@ -121,7 +122,13 @@ async fn fill_route_inner<S: StateWrite + Sized>(
     };
 
     let mut frontier = Frontier::load(&mut this, pairs).await?;
-    tracing::debug!(?frontier, "assembled initial frontier");
+    tracing::debug!(?frontier, positions = ?frontier.positions.len(), "assembled initial frontier");
+    tracing::debug!(positions = ?frontier.positions.iter().map(|p| p.id()).collect::<Vec<_>>(), ?route, "positions chosen for route");
+    let positions_set = frontier
+        .positions
+        .iter()
+        .map(|p| p.id())
+        .collect::<HashSet<_>>();
 
     // Tracks whether we've already filled at least once, so we can skip the spill price check
     // until we've consumed at least one position.
@@ -168,6 +175,7 @@ async fn fill_route_inner<S: StateWrite + Sized>(
         } else {
             true
         };
+        tracing::debug!(?spill_price, actual_price = ?tx.actual_price(), "should_apply: {}", should_apply);
 
         if !should_apply {
             tracing::debug!(
@@ -267,7 +275,7 @@ async fn fill_route_inner<S: StateWrite + Sized>(
     let fill_elapsed = fill_start.elapsed();
     metrics::histogram!(metrics::DEX_ROUTE_FILL_DURATION).record(fill_elapsed);
     // cleanup / finalization
-    Ok(swap_execution)
+    Ok((swap_execution, positions_set))
 }
 
 /// Breaksdown a route into a collection of `DirectedTradingPair`, this is mostly useful
@@ -671,6 +679,11 @@ impl<S: StateRead + StateWrite> Frontier<S> {
         // Work forwards along the path from the constraining position.
         self.fill_forward(&mut tx, constraining_index + 1, exactly_consumed_reserves);
 
+        tracing::debug!(
+            new_reserves = ?tx.new_reserves,
+            "filled constrained position, propagating changes along frontier"
+        );
+
         tx
     }
 

crates/core/component/dex/src/component/router/path_search.rs

@@ -31,6 +31,9 @@ pub trait PathSearch: StateRead + Clone + 'static {
             price_limit,
         } = params;
 
+        // TODO: use seen_position_sets to avoid re-searching the same paths
+        // to prevent cycles between traces
+
         // Initialize some metrics for calculating time spent on path searching
         // vs route filling. We use vecs so we can count across iterations of the loop.
         let path_start = std::time::Instant::now();

crates/core/component/dex/src/component/router/route_and_fill.rs

@@ -171,6 +171,8 @@ pub trait RouteAndFill: StateWrite + Sized {
 
         let max_delta_1: Amount = MAX_RESERVE_AMOUNT.into();
 
+        let mut seen_position_sets = Vec::new();
+
         // Termination conditions:
         // 1. We have no more delta_1 remaining
         // 2. A path can no longer be found
@@ -214,9 +216,10 @@ pub trait RouteAndFill: StateWrite + Sized {
                 .expect("expected state to have no other refs")
                 .fill_route(delta_1, &path, spill_price)
                 .await;
+            println!("execution: {:?}", execution);
 
-            let execution = match execution {
-                Ok(execution) => execution,
+            let (execution, positions_executed) = match execution {
+                Ok((execution, positions_executed)) => (execution, positions_executed),
                 Err(FillError::ExecutionOverflow(position_id)) => {
                     // We have encountered an overflow during the execution of the route.
                     // To route around this, we will close the position and try to route and fill again.
@@ -236,6 +239,15 @@ pub trait RouteAndFill: StateWrite + Sized {
                 }
             };
 
+            println!("execution positions: {:?}", positions_executed);
+            // If we've seen this set of positions before, avoid executing against them again.
+            // Ideally this would be fed into path search but that's not feasible.
+            if seen_position_sets.contains(&positions_executed) {
+                tracing::debug!("repeated position set");
+                break;
+            }
+            seen_position_sets.push(positions_executed);
+
             // Immediately track the execution in the state.
             (total_output_2, total_unfilled_1) = {
                 let lambda_2 = execution.output;