Skip to content

refactor: replace DFS market price with Bellman-Ford SPFA#103

Open
Haikane wants to merge 5 commits into
mainfrom
ms/bf-market-price
Open

refactor: replace DFS market price with Bellman-Ford SPFA#103
Haikane wants to merge 5 commits into
mainfrom
ms/bf-market-price

Conversation

@Haikane

@Haikane Haikane commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace DFS-based TokenGasPriceComputation with Bellman-Ford SPFA one-to-all pricing
  • New bellman_ford_pricing.rs module: single SPFA pass from WETH prices all tokens using actual get_amount_out() simulation during relaxation, replacing DFS path enumeration + spot-price heuristic scoring
  • Remove SpotPriceComputation dependency from token gas pricing (still used by pool depths and MostLiquid)

Benchmark results

Config Bootstrap Incr median Tokens
Eth all protocols TVL 50 104ms (was 2,599ms, 25x) 101ms (was 1,352ms, 13x) 803 (was 817)
Eth native TVL 50 3ms (was 20ms, 6.7x) 7ms (was 9.5ms, 1.4x) 726 (was 730)
Eth native TVL 10 11ms (was 51ms, 4.6x) 7ms (was 21.5ms, 3x) 1,974 (was 1,976)
Base all TVL 50 4ms (was 57ms, 14x) 3ms (was 25ms, 8x) 205 (was 209)

Token coverage is 0.1-1.9% lower due to BF's forbid-revisits pruning marginal paths through low-liquidity pools.

Test plan

  • 238/240 lib tests pass (2 pre-existing CLI test failures on base branch)
  • cargo clippy --lib -- -D warnings: zero warnings
  • Benchmarked on 4 configurations against DFS baseline
  • Review the 0.1-1.9% token coverage gap on real data

@Haikane Haikane force-pushed the ms/bf-market-price branch from ae6e485 to 856187b Compare March 17, 2026 18:43
@Haikane Haikane changed the base branch from ms/atomic-searcher-showcase to main March 17, 2026 18:43
@Haikane Haikane changed the title Replace DFS market price with Bellman-Ford SPFA perf: replace DFS market price with Bellman-Ford SPFA Mar 17, 2026
@Haikane Haikane changed the title perf: replace DFS market price with Bellman-Ford SPFA refactor: replace DFS market price with Bellman-Ford SPFA Mar 17, 2026

@dianacarvalho1 dianacarvalho1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah this PR should be bundled with #43 so we don't have repeated code?
What is your merge plan here? I would suggest cleaning up #43 in a way that the necessary methods for this PR are available and can just be called here instead of repeating the same code twice

//! propagating to all reachable tokens within `max_hops`. Uses forbid-revisits to prevent
//! paths through arbitrage loops that would distort prices.
//!
//! This is the pricing counterpart to the routing BF in `bellman_ford.rs`:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

bellman_ford.rs doesn't exist in this branch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Rebased on main (which now includes PR #43), and changed this to a proper module link: [bellman_ford](super::bellman_ford).

Comment on lines +41 to +44
#[allow(dead_code)]
pub fn amount_at(&self, node: NodeIndex) -> &BigUint {
&self.distance[node.index()]
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

let's not introduce dead code if we don't need it 🧹

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed from production code. Moved behind #[cfg(test)] since it's only used in test assertions.

Comment on lines +245 to +249
let state_override = if is_vm {
vm_state_override.as_ref()
} else {
native_state_overrides.get(component_id)
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, it wasn't needed. Forbid-revisits guarantees each pool appears at most once in a path, so state overrides can never trigger. Removed the entire VM/native state-override tracking, the EVMPoolState downcast, and the related imports.

Haikane and others added 3 commits March 23, 2026 14:56
Add bellman_ford_pricing.rs with flat-array SPFA that prices all tokens
from gas_token in a single traversal, using simulation during relaxation
instead of the DFS + spot-price heuristic approach.

Rewrite token_gas_price.rs to use BF forward pass + reverse simulation
per token. Removes SpotPrices dependency entirely. Selects paths by
highest forward output (most liquid) rather than lowest spread.

12/12 tests pass. No regressions in full suite.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused Path::reversed() method (was only needed by DFS-based pricing)
- Update dependency graph docs: TokenGasPriceComputation no longer depends on spot_prices
- Fix PoolDepthComputation dependency comment (it depends on spot_prices, not "no dependencies")

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… resimulate

- Extract shared helpers (path_has_conflict, reconstruct_path,
  extract_subgraph_edges) into bf_helpers.rs, used by both routing
  and pricing BF modules
- Remove dead amount_at method (moved to #[cfg(test)] only)
- Simplify resimulate_path: remove state-override tracking since
  forbid-revisits guarantees each pool appears at most once
- Fix doc comment to use proper module link instead of file reference
- Remove unused ProtocolSim import and dead simulation_amount accessor

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Haikane Haikane force-pushed the ms/bf-market-price branch from 856187b to b90fb3e Compare March 23, 2026 21:40
@Haikane

Haikane commented Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all review comments. Rebased on main (which now includes PR #43) and deduplicated the shared code:

  • Created bf_helpers.rs with shared free functions (path_has_conflict, reconstruct_path, extract_subgraph_edges) used by both routing and pricing BF
  • Updated bellman_ford.rs to call bf_helpers:: instead of associated methods
  • Removed dead code and unnecessary state-override logic from pricing module

285 lib tests pass, zero clippy warnings.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@dianacarvalho1 dianacarvalho1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @Haikane

Resolve conflict in token_gas_price.rs by taking the BF branch version,
which replaces the DFS-based pricing entirely with Bellman-Ford SPFA.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tvinagre

Copy link
Copy Markdown
Collaborator

Let's not merge this now as it interferes with https://github.com/propeller-heads/fynd/pull/95/changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants