Repository navigation
feat(api): aggregate quotes across multiple price sources, and fix the stale Cargo.lock - #594
Conversation
|
@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.
c181234 to
7d61956
Compare
|
These two pull requests touch the same thing from opposite sides, and merging both would break This pull request adds The problem is that the api crate does not use #601 removes the unused If both are merged, the manifest no longer declares So the I did not mean to conflict; the two pull requests were worked on separately. |
What this changes
Two commits.
1.
fix(backend): add the reqwest entry Cargo.lock is missingcrates/api/Cargo.tomldeclaresreqwest.workspace = true, butCargo.lockdoes not list it underengipay-api. The manifest and the lock file therefore disagree, and the "Backend (Rust)" job stops at its second step:cargo clippy --lockedandcargo test --lockedcannot 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 sourcesA conversion quote no longer rests on a single provider. A new
AggregatedPriceSourcecombines 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 isStalePrice.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
SourcesDisagreeif 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.
ConversionPricenow 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 asprice_sources, and it is stored with the quote so a price can be traced back to the providers that produced it. Migration0021_quote_price_sources.sqladds the column toconversionsandbuy_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
AppStateyet: there is no second real provider to aggregate, soAppState::with_fakesstill 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 --checkinbackend: 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-apiandcargo metadata --locked --format-version 1inbackend: pass. Both fail onmainwith "cannot update the lock file ... because --locked was passed to prevent this".cargo clippygets 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.rsandcrates/api/src/routes/convert.rscome back clean, and the two errors inroutes/buy.rsare onmain's own lines 21 and 188, not on any line this pull request adds.price_source.rscover 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 warningsandcargo test --workspace --lockedcould not be run locally: this machine has no Windows SDK, solink.execannot findkernel32.liband every build script fails to link. Thex86_64-pc-windows-gnutarget was tried as well, but there is no 64-bit MinGW toolchain here. Both need CI.Pre-existing failures this pull request does not fix
These are already broken on
mainand are listed so they are not mistaken for regressions here.The api crate does not compile on
main.routes/buy.rsimportscrate::services::receipt, butservices/mod.rsdeclares the module asreceipts;routes/buy.rsandroutes/convert.rscallAsset::from_symbol, whichcrates/core/src/asset.rsdoes not define;routes/auth/stellar.rsusesstellar_xdr::int64/uint32/uint64and aTransactionEnvelope::TxV1variant that the pinnedstellar-xdrdoes not have;routes/transfers.rsimportscrate::state;routes/batch_payments.rsimportsaxum::extract::Multipart;routes/receipts.rsimportscrate::services::receipts::ReceiptKindand readsReceiptfields that the struct does not have; andcreate_tokenis called with the wrong number of arguments in the api tests. This is what the "Backend (Rust)" job now fails on, after the--lockederror in front of it is cleared."Ledger guarantees (Postgres)" fails on
maininscripts/verify-migrations.sh, comparing the two schema dumps it takes:The only difference is the random
\restrictand\unrestricttokens thatpg_dumpwrites on recent PostgreSQL.capture_schemafilters 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 postgrescreates theengipaydatabase empty, and bothverify-migrations.shandtest-db-guarantees.shapply the migrations to throwaway databases that they drop again. The DB-backed tests connect toengipayand 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.sqland0002_fee_schedules.sqlare both version 2, the three0018_*.sqlfiles are all version 18, and0020_idempotency_keys.sqland0020_receipts.sqlare both version 20._sqlx_migrations.versionis a primary key,Migrate::applyinserts into it, andMigrator::runtakes 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
mainatnpm audit --audit-level=high:tailwindcss3.4.x pulls in vulnerablebraces,chokidar,micromatchandfast-glob. Clearing that needs a Tailwind v4 migration, which is a separate piece of work.Checklist
npm run typecheck,npm testandnpm run buildpass (web app changes) - no web app changescargo fmt,cargo clippy -- -D warningsandcargo testpass (backend changes) -cargo fmt --all --checkpasses 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..envfiles are includedCargo.lockgains thereqwestentry thatcrates/api/Cargo.tomlalready declared and the lock file was missing