feat(wallet): WalletTx now supports non-canonical txs#480
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #480 +/- ##
==========================================
+ Coverage 80.30% 80.52% +0.21%
==========================================
Files 24 24
Lines 5417 5478 +61
Branches 245 246 +1
==========================================
+ Hits 4350 4411 +61
Misses 989 989
Partials 78 78
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bc1de69 to
2bf915d
Compare
| .filter(|t| is_canonical(t)) | ||
| .collect(); | ||
|
|
||
| blockers.remove(&txid); |
There was a problem hiding this comment.
nit: the is_canonical(t) filter above only allows canonical txids into blockers, and txid is non-canonical here, so this line is unreachable. So, I think it's safe to remove.
|
Concept ACK |
2bf915d to
5968606
Compare
|
After some ideas by ValuedMammal I simplified the model a bit:
PR description updated accordingly. |
|
|
Description
WalletTxwas an alias forCanonicalTx, sowallet.transactions()andwallet.get_tx(txid)only returned canonical wallet-relevant txs. Evicted/replaced txs disappeared tho still wallet relevant.This PR changes
WalletTxtoTransactionInfoand makes it a struct that represents any wallet-relevant tx:Canonical status lives in
details.canonicality.Fixes #295
Supported use cases
Surface now including non-canonical txs
TransactionInfo,TxCanonicality,TxDetails,transactions(),get_tx()Notes to the reviewers
transactions()takes a small hit. Worth confirming whether it's an issue in practice.Changelog notice
WalletTx→TransactionInfostruct withdetails,tx_node,last_evicted,conflicts;WalletTxStatus→TxCanonicality.transactions()/get_tx()also return non-canonical wallet-relevant txs.TxDetails::chain_position→canonicality.Wallet::tx_details(txid), usewallet.get_tx(txid).details.Checklists
All Submissions:
just pbefore pushingNew Features: