api: fix empty request_id in trace spans (#790), .env.example token fallback (#811), coinbase prod-URL test coverage (#706), single source of truth for price scale (#709) - #857
Merged
Conversation
…ple token fallback (SO4-Markets#811); coinbase prod-URL test coverage (SO4-Markets#706); single source of truth for price scale (SO4-Markets#709) - SO4-Markets#790: on a Router, the last .layer() added is outermost / runs first, so SetRequestIdLayer (added after trace_layer) ran too late — trace_layer's make_span_with read the RequestId extension before it was set and every span got request_id = "". Swapped the two so SetRequestIdLayer is outermost. Test asserts the span carries the real id. - SO4-Markets#811: .env.example set PRICE_FEED_CONFIG=[] — a present, empty value that configures zero tokens (GET /prices always 503s), never the include_str! fallback which only fires when the var is unset. Left it unset with a comment explaining the difference and a populated override example. - SO4-Markets#706: every coinbase HTTP test built base_url as "{uri}/", exercising only the use_query=false branch; production's ".../exchange-rates?currency=" shape always takes use_query=true (the reqwest .query() path), which had zero coverage. Added a wiremock test in that shape with a query_param matcher. - SO4-Markets#709: the "prices are scaled to 30 decimals" invariant was four disconnected literals across binance.rs and pyth.rs (two FLOAT_PRECISION copies, two bare 30s). Introduced crate::SCALE_DIGITS (and a derived crate::FLOAT_PRECISION); binance/pyth now re-export FLOAT_PRECISION and derive their scale/exponent bounds from SCALE_DIGITS. Also fixes a pre-existing broken lib unit test in price_loop.rs (build_cached_price gained a 5th arg) so cargo test / clippy --all-targets compile again. Verified locally: cargo check --lib --tests and cargo clippy --lib --tests are clean.
|
@thefadah Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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.
#790 — trace spans always had an empty
request_idOn a
Router,.layer(a).layer(b)makes the last layer outermost — it sees the request first. SoSetRequestIdLayer(added aftertrace_layer) ran after it, andtrace_layer'smake_span_withreadrequest.extensions().get::<RequestId>()while it was stillNone→ every span gotrequest_id = "", defeating the request-id-in-logs feature from #657.middleware_integration.rs::test_request_id_and_completion_logsdidn't catch it because it only checks the response header (produced byPropagateRequestIdLayer, which works regardless).Fix:
SetRequestIdLayeris now added last (outermost), so it populates the extension beforetrace_layerreads it:trace_span_carries_the_request_id_not_an_empty_stringdrives a request with an explicitx-request-idthrough the real router with a capturing subscriber and asserts that id appears in the span's structured output (and that"request_id":""does not).#811 —
.env.exampleproduced a zero-token oraclePRICE_FEED_CONFIG=[]is a present value that parses to zero configured tokens — it never triggers theinclude_str!("../../config/tokens.json")fallback, which only fires when the var is entirely unset. Following the example verbatim gave a service that starts cleanly but never fetches a price (GET /pricesalways 503). The line is now left unset with a comment spelling out the difference and a populated override example.#706 — coinbase's production request-building branch had zero coverage
fetch_spot_price_with_urlpicksuse_queryfrom a substring check ofbase_url. The realCOINBASE_EXCHANGE_RATES_URL(.../v2/exchange-rates?currency=) always takesuse_query = true(thereqwest .query()path). Every existing wiremock test buildsbase_urlas"{uri}/"— neither substring — so all of them exercise theuse_query = falseformat!-concatenation branch that production never runs. Addedfetch_spot_price_production_url_shape_uses_query_param_branch: a wiremock test in the real URL shape with aquery_param("currency", "BTC")matcher, asserting the currency arrives as a query parameter.#709 — the "30-decimal scale" invariant was four ungoverned magic numbers
binance.rsandpyth.rseach hardcoded30/10^30independently (twoFLOAT_PRECISIONcopies,let scale_digits = 30usize,-30..=0,30 + exponent) with no shared source. Introducedcrate::SCALE_DIGITS: u32 = 30and a derivedcrate::FLOAT_PRECISION = 10i128.pow(SCALE_DIGITS).binance::FLOAT_PRECISION/pyth::FLOAT_PRECISIONare nowpub usere-exports (callers unchanged), and every scale/exponent bound is derived fromSCALE_DIGITS.Incidental
oracle/src/price_loop.rs: a pre-existing broken lib unit test (build_cached_pricegained a 5th parampyth_batch_failed: boolbutfixed_source_builds_signed_cached_pricestill passed 4 args) madecargo test/cargo clippy --all-targetsfail to compile onmain. One-arg fix included.Verification
cargo check -p oracle --lib --testsandcargo clippy -p oracle --lib --testsare clean locally.Closes #790
Closes #811
Closes #706
Closes #709