refactor: decompose price calculation into orthogonal components#65
Merged
Conversation
`PriceCalculator` is now a thin orchestration façade over five focused submodules: `SwapLogScanner` builds the price-source filter and drives the chunked `eth_getLogs` scan, `extract_swaps` is a pure walk that turns logs into accepted `SwapData`, `TokenMetadataProvider` reads ERC-20 decimals into a provider-free `TokenDecimalsCache`, `normalize_against_pair`/`normalize_swap` are pure amount conversions, and `PriceAggregator` folds normalized swaps into a `TokenPriceResult`. `PriceSource` remains the DEX extension point, and the public surface — `new`, `with_config`, `calculate_price_between_blocks`, and `extract_raw_swaps` — plus the `TokenPriceResult` and `RawSwapResult` result types are unchanged. The motivation is that price calculation previously mixed scanning, decoding, decimals fetching/caching, normalization, and aggregation in one type, making it hard to test or extend. Each step is now independently unit-testable without a provider: swap extraction runs against synthetic logs, relevance and normalization cover both swap directions and missing-pair cases, aggregation folds in-memory amounts, and the decimals cache is exercised directly. The pair-scoped batch decimals fetch and the per-swap failure tolerance are preserved. Closes #26.
These two cases asserted on raw `U256` division into `f64` without calling any semioscan code, so they tested a dependency rather than the crate's own normalization. The behaviour they stood in for is now covered directly by the `normalize` module's unit tests.
Contributor
Author
Self-review passPre-commit
Both reviews independently concluded the decomposition is complete and behavior-preserving (no dead code, no leaked internal |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PriceCalculatoris now a thin orchestration façade over five focused submodules instead of one type that mixed scanning, decoding, decimals fetching/caching, normalization, and aggregation. This makes the price domain testable without a provider and easier to extend for alternate metadata or aggregation strategies.PriceSourcestays the DEX extension point and the public API is unchanged.Closes #26.What's in the diff
src/price/extractor.rs(new) — pureextract_swapsthat walks logs throughPriceSource::extract_swap_from_log+should_include_swap, tracing decode/invalid-swap errors without aborting.src/price/normalize.rs(new) — purenormalize_against_pair(target/USDC, both directions),normalize_swap, and theinvolves_pairrelevance predicate.src/price/aggregator.rs(new) —PriceAggregatorfolds normalized swaps into aTokenPriceResult.src/price/decimals.rs(new) — provider-freeTokenDecimalsCacheplusTokenMetadataProvider(single + parallel batch fetch,CallBatchLayer-friendly).src/price/scanner.rs(new) —SwapLogScannerbuilds the router/topics filter and drives the chunked scan.src/price/calculator.rs— rewritten as orchestration; keeps the publicPriceCalculator,TokenPriceResult,RawSwapResultand their methods. Review first:process_gap_for_priceandextract_raw_swaps.src/price/mod.rs— declares the new private submodules.Acceptance check (from #26)
SwapData, no provider —aggregator.rs+normalize.rsunit tests.decimals.rsTokenDecimalsCachetests.PriceCalculatoris smaller and reads as orchestration.TokenPriceResulttests retained).examples/custom_dex_integration.rsstill compiles —cargo check --examples --all-features.PriceSourceremains the documented extension point.new,with_config,calculate_price_between_blocks, raw-swap API kept source-compatible.CallBatchLayercompatibility preserved (pair-scoped batch retained).Test plan
cargo test --all-features— 509 lib + all integration suites passcargo test/cargo test --no-default-features— passcargo clippy --all-targets --all-features -- -D warningsclean (all three feature combos)cargo fmt --checkcleancargo check --examples --all-featurescleanSelf-review notes
Pre-commit
/code-review(5 angles, extra-high effort) found the refactor behavior-faithful. One pre-existing operator-facing gap surfaced — a persistent decimals-fetch failure caches an empty/partialTokenPriceResultfor the range, so a later same-range query returns the bogus zero without rescanning. This is not introduced here (the originalprocess_gap_for_pricecached skipped-swap results identically), so it's left out of scope and filed as a follow-up.