test: cover all foreign provider auth kinds in verify-foreign-tx e2e - #3759
Conversation
1a01e6e to
706ab7d
Compare
Pull request overviewExtends the existing Changes:
Reviewed changesPer-file summary
FindingsNo blocking issues — the wiring between config and mocks is consistent for all three auth kinds (Path substitution → Non-blocking (nits, follow-ups, suggestions):
✅ Approved |
There was a problem hiding this comment.
Pull request overview
This PR strengthens the verify_foreign_transaction__should_sign_all_supported_chains E2E test to exercise all credential-carrying AuthConfig variants end-to-end (path/header/query), ensuring the node actually injects configured credentials during foreign provider calls (including the path placeholder substitution regression from #2637 / #2762).
Changes:
- Extend the foreign tx validation E2E test to configure Bitcoin/Base/BNB providers with
AuthConfig::{Path,Header,Query}and require matching credentials in mocks. - Add mock-side authentication expectations plus a 401 unauthorized catch-all to ensure unauthenticated requests fail in a realistic way.
- Add assertions that the authenticated mocks were actually hit (preventing accidental mock loosening).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/e2e-tests/tests/foreign_chain_tx_validation.rs | Configures one chain per auth kind and asserts authenticated mocks were queried. |
| crates/e2e-tests/src/foreign_chain_mock.rs | Adds MockAuthExpectation and an unauthorized catch-all to enforce credential presence in requests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| impl MockAuthExpectation { | ||
| fn apply(&self, when: When) -> When { | ||
| match self { | ||
| Self::None => when.path("/"), | ||
| Self::ApiKeyInPath { key } => when.path(format!("/{key}")), | ||
| Self::Header { name, value } => when.path("/").header(name, value), | ||
| Self::QueryParam { name, value } => when.path("/").query_param(name, value), | ||
| } | ||
| } | ||
| } |
| bitcoin: bitcoin_server.url("/"), | ||
| // The configured URL carries the literal placeholder; the node must | ||
| // substitute the API key into it before any request can match the mock. | ||
| bitcoin: bitcoin_server.url(format!("/{PATH_AUTH_PLACEHOLDER}")), |
Bitcoin, Base and BNB mock providers now require path, header and query credentials respectively; requests without them get the 401 real providers return instead of a valid RPC response. Verification can only succeed if the node injects each configured credential, covering the path-auth API key substitution end to end. Closes #2786
10ea069 to
a0a32cf
Compare
74dce1a
Closes #2786
Summary
Extends
verify_foreign_transaction__should_sign_all_supported_chainsso one chain exercises each credential-carryingAuthConfigkind: Bitcoinpath, Baseheader, BNBquery(the rest keepnone). Each authenticated mock only answers requests carrying the exact configured credential values, so verification can only succeed if the node injected them — covering the path-auth API key substitution from #2637 (fixed in #2762 with unit tests only) end to end, through real node binaries and config loading.Why mocks answer bad auth with 401
Requests without the expected credentials fall through to a catch-all returning HTTP 401 with a JSON-RPC error body, mirroring the providers on the testnet on-chain whitelist: Alchemy returns exactly this body (probed live — including for a literal unsubstituted
{API_KEY}path, the #2637 scenario; their error reference lists no 404), Geomi returns 401 for an invalid Bearer token, and QuickNode documents 401"error": "UNAUTHORIZED". A regression therefore produces failure logs matching a real credential misconfiguration.Testing
verify_foreign_transaction__should_sign_all_supported_chainspasses locally in an isolated run (~21s against prebuilt binaries).