zcash_client_backend: Add a Tor client that can fetch USD/ZEC exchange rates #1422
Conversation
| # Tor | ||
| # - `arti-client` depends on `rusqlite`, and a version mismatch there causes a compilation | ||
| # failure due to incompatible `libsqlite3-sys` versions. | ||
| arti-client = { version = "0.11", default-features = false, features = ["compression", "rustls", "tokio"] } |
There was a problem hiding this comment.
After #1421 we are no longer constrained by ring, but we are still constrained here by rusqlite which is why I only bumped this from 0.9 to 0.11, not to the latest 0.19. We should separately look at bumping rusqlite, subject to the CocoaPods issue above.
There was a problem hiding this comment.
Ugh, we can't actually avoid cross-version ring dependencies, because tor-rtcompat depends on both a rustls version that uses ring 0.17, and the x509-signature crate that uses ring 0.16. We'd need to use the older arti-client version etc. that uses the older rustls. Given that the most recent release of Arti is also still in this state, I'm inclined to just leave it as-is (we're at least down to a single version of rustls) and see how it goes.
There was a problem hiding this comment.
I confirmed that this still happens to work on iOS, so I think we should just leave it as-is for now, and if I have time I'll try to contribute a change to Arti that migrates away from x509-signature.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1422 +/- ##
==========================================
- Coverage 64.19% 63.90% -0.30%
==========================================
Files 130 132 +2
Lines 15423 15494 +71
==========================================
Hits 9901 9901
- Misses 5522 5593 +71 ☔ View full report in Codecov by Sentry. |
str4d
force-pushed
the
zcb-tor
branch
2 times, most recently
from
bb54d4e to
e5ff573
Compare
str4d
force-pushed
the
zcb-tor
branch
2 times, most recently
from
746e168 to
b5a3c2c
Compare
str4d
force-pushed
the
zcb-tor
branch
5 times, most recently
from
89fe473 to
f618ca3
Compare
str4d
force-pushed
the
zcb-tor
branch
2 times, most recently
from
4d4575a to
361653b
Compare
|
Rebased on |
|
Force-pushed to remove unnecessary |
|
Force-pushed to not pass Instead, we will always query for |
|
Rebased on |
There was a problem hiding this comment.
I think that the implementation of get_exchange_rate is excessively restrictive - it does not permit either the exclusion of data providers or the addition of providers without forking the crate. If the API is released as-is, it will require a breaking API change to remedy that, and I don't see a good reason for not adding the necessary abstraction layer when introducing the API to begin with.
str4d
force-pushed
the
zcb-tor
branch
2 times, most recently
from
0ccfdb2 to
675490e
Compare
|
Force-pushed to rework the exchange rate API to allow callers to customise the exchanges (or define their own data sources). |
This fetches price data from multiple exchanges over Tor, and takes the median of the successful responses. Closes #1416.
Currently only unauthenticated connections are supported (i.e. no API keys can be configured). However, AFAICT none of these exchanges provide non-IP-based rate limits for authenticated connections to public APIs, so authentication wouldn't help to make connections over Tor more reliable.
|
Force-pushed to address review comments. |
Closes #1416.