LLFourn / bdk_core_staging

Staging area for bdk_core initial development
15 stars 12 forks source link

Reserve UTXOs during transaction building. #200

Open vladimirfomene opened 1 year ago

vladimirfomene commented 1 year ago

This PR solves #186.It add functionality to mark UTXOs as selected/reserved once they have been selected by a transaction during the transaction building process. Therefore the wallet cannot choose this reserved UTXOs for subsequent transaction building.

codecov-commenter commented 1 year ago

Codecov Report

Merging #200 (d0f179e) into master (0fb4e1b) will increase coverage by 2.15%. The diff coverage is 0.00%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #200      +/-   ##
==========================================
+ Coverage   56.74%   58.89%   +2.15%     
==========================================
  Files          10        9       -1     
  Lines         252      236      -16     
==========================================
- Hits          143      139       -4     
+ Misses        109       97      -12     
Impacted Files Coverage Δ
bdk_chain/src/keychain.rs 51.56% <0.00%> (-5.34%) :arrow_down:
bdk_chain/src/file_store.rs

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

LLFourn commented 1 year ago

Just a quick comment these methods and state should be on KeychainTracker not on the indexes. The indexes are monotone and have no way of knowing if something is spent or not in the canonical chain. Another reason they should be there is they affect the output of Balance.

They should also not have a txid argument as mentioned by @evanlinjin. You should be able to reserve a utxo without knowing what txid it's reserved for.

vladimirfomene commented 1 year ago

@LLFourn @evanlinjin updated as requested. Thanks for the quick review!

evanlinjin commented 1 year ago

Another reason they should be there is they affect the output of Balance.

Ahh now I understand why this feature is needed!

evanlinjin commented 1 year ago

@vladimirfomene Are the balances affected when we mark an outpoint? Maybe we can write a test for it.

vladimirfomene commented 1 year ago

@evanlinjin the balances are not yet affected. Let me think of a strategy

vladimirfomene commented 1 year ago

@evanlinjin @LLFourn written test showing how balance are affected when UTXOs are marked as selected. I have added a field for total selected amount to Balance. The idea is to create some form of visibility from a user's perspective on where the UTXOs are if for some reason they request the balance after creating a transaction and not broadcasting it.