Skip to content

feat(api): aggregate quotes across multiple price sources, and fix the stale Cargo.lock - #594

Merged
adenueltech merged 2 commits into
The-Engineer-Network:mainfrom
EmmanuelDesmond:feat/multi-source-price-aggregation
Oct 22, 2026
Merged

adenueltech merged 2 commits into
The-Engineer-Network:mainfrom
EmmanuelDesmond:feat/multi-source-price-aggregation

Conversation

@EmmanuelDesmond

@EmmanuelDesmond EmmanuelDesmond commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

What this changes

Two commits.

1. fix(backend): add the reqwest entry Cargo.lock is missing

crates/api/Cargo.toml declares reqwest.workspace = true, but Cargo.lock does not list it under engipay-api. The manifest and the lock file therefore disagree, and the "Backend (Rust)" job stops at its second step:

error: cannot update the lock file /home/runner/work/engipay/engipay/backend/Cargo.lock because --locked was passed to prevent this

cargo clippy --locked and cargo test --locked cannot start at all until this is fixed. The lock file gains the missing entry and nothing else.

2. feat(api): aggregate quotes across multiple price sources

A conversion quote no longer rests on a single provider. A new AggregatedPriceSource combines independent price sources and refuses to price anything it cannot corroborate.

It drops the observations it cannot use: a source that errored, one that reported an observation older than the configured maximum age or dated in the future, one that repeats a source identifier already counted, and one with a non-positive rate. Fewer than two usable observations left is InsufficientSources; all of them stale is StalePrice.

The surviving rates are ordered and the median is taken. The median of an even number of observations is the mean of the two middle rates, computed as an exact rational, so no floating-point step is introduced into an amount of money. Each observation is then compared against the median and the aggregate is rejected with SourcesDisagree if any of them differs by more than the configured deviation in basis points, so a source that has quietly started quoting something else cannot drag the price with it.

Aggregates are cached for a short window, and the cache is re-checked against the age of the observation it holds, so a warm cache cannot serve a price that has since gone stale.

ConversionPrice now carries the identifiers of the sources that contributed to it and the time of the newest observation. The convert and buy quote responses expose that as price_sources, and it is stored with the quote so a price can be traced back to the providers that produced it. Migration 0021_quote_price_sources.sql adds the column to conversions and buy_quotes.

The new failure modes - no corroborating source, sources that disagree, a stale observation - are reported as 503 rather than as a bad request. The request is well formed in those cases; it is the price that could not be produced.

The aggregator is not wired into AppState yet: there is no second real provider to aggregate, so AppState::with_fakes still installs the fake source. Connecting it is a follow-up, once a second oracle exists to corroborate the first.

How it was tested

  • cargo fmt --all --check in backend: passes. CI runs the same rustfmt (rustc 1.99.1, 58cf8f9c3), so this is the same check CI will run.
  • cargo tree --locked -p engipay-api and cargo metadata --locked --format-version 1 in backend: pass. Both fail on main with "cannot update the lock file ... because --locked was passed to prevent this".
  • Every changed file was checked for a UTF-16 byte order mark, which is what the "Scan config files" job refuses: none has one, and all are UTF-8.
  • CI on this branch. With the lock file fixed, cargo clippy gets past dependency resolution for the first time and actually compiles the workspace. It reports 106 errors, and every one of them is in code this pull request does not touch: routes/auth/stellar.rs (58), routes/transfers.rs (22), routes/auth/evm.rs (16), routes/naira.rs (3), routes/buy.rs (2), routes/batch_payments.rs (2), routes/lib.rs (1), routes/withdrawals.rs (1), routes/receipts.rs (1), plus the chain crate. crates/api/src/services/price_source.rs and crates/api/src/routes/convert.rs come back clean, and the two errors in routes/buy.rs are on main's own lines 21 and 188, not on any line this pull request adds.
  • The new unit tests in price_source.rs cover the median of agreeing sources, the rejection of disagreeing sources, tolerance of one source being down, the rejection of stale observations, and that the cache does not re-query the sources. They could not be executed here, for the reason below.
  • cargo clippy --workspace --all-targets --locked -- -D warnings and cargo test --workspace --locked could not be run locally: this machine has no Windows SDK, so link.exe cannot find kernel32.lib and every build script fails to link. The x86_64-pc-windows-gnu target was tried as well, but there is no 64-bit MinGW toolchain here. Both need CI.
  • The migration was not applied locally: there is no Docker and no Postgres on this machine.

Pre-existing failures this pull request does not fix

These are already broken on main and are listed so they are not mistaken for regressions here.

  • The api crate does not compile on main. routes/buy.rs imports crate::services::receipt, but services/mod.rs declares the module as receipts; routes/buy.rs and routes/convert.rs call Asset::from_symbol, which crates/core/src/asset.rs does not define; routes/auth/stellar.rs uses stellar_xdr::int64/uint32/uint64 and a TransactionEnvelope::TxV1 variant that the pinned stellar-xdr does not have; routes/transfers.rs imports crate::state; routes/batch_payments.rs imports axum::extract::Multipart; routes/receipts.rs imports crate::services::receipts::ReceiptKind and reads Receipt fields that the struct does not have; and create_token is called with the wrong number of arguments in the api tests. This is what the "Backend (Rust)" job now fails on, after the --locked error in front of it is cleared.

  • "Ledger guarantees (Postgres)" fails on main in scripts/verify-migrations.sh, comparing the two schema dumps it takes:

    ? Schema mismatch between pass 1 and pass 2:
    1c1
    < \restrict yGWqR4xDfXdgDJH5H7xwRHasCXfKmEJjrwa8QYjKt99b6gpzXoAH7XDAa4e1lyL
    ---
    > \restrict fFXNwOuKXE0c7wFXOv7ZECw1W3ji23a8u61YqVjM09h61cWre7ALVGcSueSpYuC
    

    The only difference is the random \restrict and \unrestrict tokens that pg_dump writes on recent PostgreSQL. capture_schema filters out -- comments and blank lines but not these, so the two dumps can never match. Filtering them out is all this needs.

  • The "database" job never populates the database its tests use. docker compose up -d --wait postgres creates the engipay database empty, and both verify-migrations.sh and test-db-guarantees.sh apply the migrations to throwaway databases that they drop again. The DB-backed tests connect to engipay and expect every migration to have been applied, so something has to apply them to that database.

  • sqlx::migrate! cannot apply the migrations as they are numbered. 0002_add_xlm.sql and 0002_fee_schedules.sql are both version 2, the three 0018_*.sql files are all version 18, and 0020_idempotency_keys.sql and 0020_receipts.sql are both version 20. _sqlx_migrations.version is a primary key, Migrate::apply inserts into it, and Migrator::run takes its snapshot of the applied migrations before it starts, so the second file carrying a version is applied again and violates that key. This was read out of the sqlx 0.8.6 source rather than executed, because there is no database here. Renumbering would change the version of migrations already applied on live databases, so it needs a plan rather than a rename.

  • "Web app" fails on main at npm audit --audit-level=high: tailwindcss 3.4.x pulls in vulnerable braces, chokidar, micromatch and fast-glob. Clearing that needs a Tailwind v4 migration, which is a separate piece of work.

Checklist

  • npm run typecheck, npm test and npm run build pass (web app changes) - no web app changes
  • cargo fmt, cargo clippy -- -D warnings and cargo test pass (backend changes) - cargo fmt --all --check passes locally; clippy and test need CI, because this machine cannot link. On CI, clippy reaches the compile step and reports no error in the files this pull request touches.
  • No secrets, keys or .env files are included
  • Money is handled as exact integers or decimal strings, never floating point
  • Lockfile changes are intentional and explained above - Cargo.lock gains the reqwest entry that crates/api/Cargo.toml already declared and the lock file was missing

@vercel

vercel Bot commented Oct 21, 2026

Copy link
Copy Markdown

@EmmanuelDesmond is attempting to deploy a commit to the emmanuel adewunmi's projects Team on Vercel.

A member of the Team first needs to authorize it.

`crates/api/Cargo.toml` declares `reqwest.workspace = true`, but
`Cargo.lock` does not list it under `engipay-api`. The manifest and
the lock file therefore disagree, and `cargo clippy --locked` and
`cargo test --locked` refuse to start at all:

    error: cannot update the lock file ... because --locked was passed
    to prevent this

The lock file gains the missing entry and nothing else.
A conversion quote no longer rests on a single provider. An
`AggregatedPriceSource` combines independent price sources and refuses
to price anything it cannot corroborate.

It takes the observations every source returns and drops the ones it
cannot use: a source that errored, one that reported an observation
older than the configured maximum age or dated in the future, one that
repeats a source identifier already counted, and one with a
non-positive rate. Fewer than two usable observations left is
`InsufficientSources`; all of them stale is `StalePrice`.

The surviving rates are ordered and the median is taken. The median of
an even number of observations is the mean of the two middle rates,
computed as an exact rational so no floating-point step is ever
introduced into an amount of money. Each observation is then compared
against the median and the aggregate is rejected with
`SourcesDisagree` if any of them differs by more than the configured
deviation in basis points. A source that has quietly started quoting
something else cannot drag the price with it.

Aggregates are cached for a short window, and the cache is re-checked
against the age of the observation it holds, so a warm cache cannot
serve a price that has since gone stale. The cache is behind a mutex
and a poisoned lock is reported as `CacheUnavailable` rather than
panicking.

`ConversionPrice` now carries the identifiers of the sources that
contributed to it and the time of the newest observation. The convert
and buy quote responses expose that as `price_sources`, and it is
stored with the quote so a price can be traced back to the providers
that produced it. Migration `0021_quote_price_sources.sql` adds the
column to `conversions` and `buy_quotes`.

The new failure modes - no corroborating source, sources that
disagree, a stale observation - are reported as 503 Service
Unavailable rather than as a bad request. The request is well formed
in those cases; it is the price that could not be produced.

The aggregator is not wired into `AppState` yet: there is no second
real provider to aggregate, so `AppState::with_fakes` still installs
the fake source. Connecting it is a follow-up, once a second oracle
exists to corroborate the first.
@EmmanuelDesmond
EmmanuelDesmond force-pushed the feat/multi-source-price-aggregation branch from c181234 to 7d61956 Compare October 21, 2026 23:04
@EmmanuelDesmond EmmanuelDesmond changed the title feat(api): aggregate quotes across multiple price sources, and restore the backend build feat(api): aggregate quotes across multiple price sources, and unblock the backend CI job Oct 21, 2026
@EmmanuelDesmond EmmanuelDesmond changed the title feat(api): aggregate quotes across multiple price sources, and unblock the backend CI job feat(api): aggregate quotes across multiple price sources, and fix the stale Cargo.lock Oct 22, 2026
@EmmanuelDesmond

Copy link
Copy Markdown
Contributor Author

These two pull requests touch the same thing from opposite sides, and merging both would break cargo clippy --locked again.

This pull request adds "reqwest" to the engipay-api entry in backend/Cargo.lock, which is what makes the lock file agree with backend/crates/api/Cargo.toml. That is the right call for a manifest that declares reqwest.

The problem is that the api crate does not use reqwest at all - there is not one reference to it under backend/crates/api, including price_source.rs, which is the code this pull request adds. The dependency is there because the batch payments work left it behind, and the lock file never listed it.

#601 removes the unused reqwest.workspace = true line from backend/crates/api/Cargo.toml instead. Nothing in the lock file has to change, and no unused dependency is carried.

If both are merged, the manifest no longer declares reqwest while the lock file still lists it in engipay-api, and every step that passes --locked stops again with:

error: cannot update the lock file .../backend/Cargo.lock because --locked was passed to prevent this

So the backend/Cargo.lock hunk in this pull request should be dropped - price_source.rs does not need reqwest - or this branch should be rebased on #601 and the hunk resolved that way. Which of the two lands first does not matter, as long as the manifest and the lock file end up agreeing.

I did not mean to conflict; the two pull requests were worked on separately.

@adenueltech
adenueltech merged commit fcd2a57 into The-Engineer-Network:main Oct 22, 2026
1 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants