Implement chain::Access trait
#21
Conversation
tnull
force-pushed
the
2022-09-chain-access-trait
branch
3 times, most recently
from
3cc9695
to
dd97b78
Compare
|
Blocked on bitcoindevkit/rust-esplora-client#8, so currently based on my |
tnull
force-pushed
the
2022-09-chain-access-trait
branch
from
dd97b78
to
30ed951
Compare
The change has been merged now, so not blocked anymore. |
tnull
force-pushed
the
2022-09-chain-access-trait
branch
5 times, most recently
from
f242dff
to
aec3a52
Compare
| // position, and finally feed them to the interface in order. | ||
| confirmed_txs.sort_unstable_by( | ||
| |(_, block_height1, _, pos1), (_, block_height2, _, pos2)| { | ||
| block_height1.cmp(&block_height2).then_with(|| pos1.cmp(&pos2)) |
There was a problem hiding this comment.
Could also be implemented using sort_unstable_by_key: getlipa/lipa-lightning-lib@b2f521d
This reverts commit fdef833.
Here we migrate from wrapping all upstream errors to exposing only select error variants to the downstream user. Also, we clean up and unify logging outputs.
tnull
force-pushed
the
2022-09-chain-access-trait
branch
from
aec3a52
to
c384381
Compare
|
Rebased on #26. |
We migrate BDK and other remaining blocking parts to async.
tnull
force-pushed
the
2022-09-chain-access-trait
branch
from
c384381
to
5420711
Compare
| } | ||
| } | ||
|
|
||
| impl<D> Access for ChainAccess<D> |
There was a problem hiding this comment.
I don't think we want to do this. Access doesn't really make sense to implement unless we're querying a local store, so implementing it for something other than local RPC/REST seems like we're sending the wrong signal.
There was a problem hiding this comment.
Hm, I agree this is kind of counter-intuitive in the mobile case when we aim to default to the RGS + Electrum/Esplora model. However, as we probably soon add a server mode where we will default to P2P + Bitcoind', it may be awkward not to cover all options, i.e., allow to switch to P2P + Electrum/Esplora, if the user really wanted that for some reason?
There was a problem hiding this comment.
I don't think "cover all the bases" is a goal? Like, I guess the question is do we expect someone to be running on a "server" without a local bitcoind? Or, I suppose, do we want to support that - I'd strongly suggest a hard no. If you're running a large forwarding node you must run your own bitcoind, and we shouldn't support anything else.
There was a problem hiding this comment.
Alright, fair enough. Will close this for now and just implement Access vs bitcoind in the future 'server mode' feature.
Based on
#8, #9, #10, #11.Funnily enough I so far had overlooked implementing
chain::Access. This PR fills this gap.