bdk
bdk copied to clipboard
Fix `last_seen` in Esplora and Electrum chains [WIP]
Description
The last seen unix time should be set for mempool transactions (#1336). I started working on this in the blocking Esplora client.
Notes to the reviewers
I decided to check-in with a WIP to get some clarifications from maintainers. There is a method on tx_graph
that allows an insertion into the map for storing last_seen
. This method is insert_seen_at
, which I think could be called insert_last_seen
without any ambiguity. When a transaction has been confirmed, there is no method to remove the Txid
from the map so we don't continually store last_seen
when a TX has been confirmed.
Should I add a remove_seen_at
in tx_graph
? Or if my naming alternative makes sense, add a remove_last_seen
?
Thanks (:
Checklists
All Submissions:
- [x] I've signed all my commits
- [x] I followed the contribution guidelines
- [x] I ran
cargo fmt
andcargo clippy
before committing
Bugfixes:
- [ ] This pull request breaks the existing API
- [ ] I've added tests to reproduce the issue which are now passing
- [ ] I'm linking the issue being fixed by this PR
Concept ACK, approach NACK.
This will be a great feature to have! However, I think we are limited by the Esplora/Electrum APIs (let me know if I have missed something).
Note that your approach here is incorrect. Please check the Esplora API docs: https://github.com/Blockstream/esplora/blob/master/API.md#transaction-format
status.block_time
is only available for confirmed transactions (so will always be None
).
Oh got it! That is trickier to implement than I thought. Am I correct in that the last_seen
field is only intended for mempool transactions, and would be appropriate to remove once a transaction has been confirmed? In that case a remove_seen_at
on tx_graph
could be beneficial later.
Oh got it! That is trickier to implement than I thought. Am I correct in that the
last_seen
field is only intended for mempool transactions, and would be appropriate to remove once a transaction has been confirmed? In that case aremove_seen_at
ontx_graph
could be beneficial later.
last_seen
just means "last seen in the mempool". It is used to resolve conflicts between unconfirmed transactions (not necessarily mempool transactions - BDK would not know, it just approximates). The current assumption is that for two conflicting unconfirmed transactions, the transaction with the more recent last_seen takes precedence (however, this approximation can be improved - see #1321).
Currently, the nature of TxGraph
is fully monotone. Nothing can be added or removed. TxGraph
is combined with a ChainOracle
implementation to know whether a transaction is confirmed or not. When a transaction gets reorged out (the block which the tx is anchored to is no longer in the best chain), BDK considered it unconfirmed. If a conflicting transaction with a more recent last_seen
gets introduced, the prior transaction is considered evicted.
Currently, the nature of
TxGraph
is fully monotone. Nothing can be added or removed.
I think things can be added!
Currently, the nature of
TxGraph
is fully monotone. Nothing can be added or removed.I think things can be added!
Sorry *nothing can be removed 😅