We prepare the BDK repo for a major restructuring.
- database modules removed
- blockchain module removed
- minimal API changes.
- Many macros removed.
- no longer applicable examples removed.
- Much conditional compilation removed. Can compile with --all-features.
- delete verify module
Lightning denotes transaction fee rate
sats / 1000 weight units and sats / 1000 vbytes.
Here we add support for creating BDK fee rate from
lightning fee rate. We also move all FeeRate test to
types.rs and rename as_sat_vb to as_sat_per_vb.
Our costant for the P2WPKH satisfaction size was wrong: in
7ac87b8f99fc0897753ce57d48ea24adf495a633 we added 1 WU for the script
sig len - but actually, that's 4WU! This resulted in
P2WPKH_SATISFACTION_SIZE being equal to 109 instead of 112.
This also adds a comment for better readability.
We were wrongly considering the sum of "effective value" (i.e. value -
fee cost) when reporting an early "insufficient funds" error in the
branch and bound coin selection.
This commit fixes essentially two issues:
- Very high fee rates could cause a panic during the i64 -> u64
conversion because we assumed the sum of effective values would never
be negative
- Since we were comparing the sum of effective values of *all* the UTXOs
(even the optional UTXOs with negative effective value) with the target
we'd like to reach, we could in some cases error and tell the user we
don't have enough funds, while in fact we do! Since we are not required
to spend any of the optional UTXOs, so we could just ignore the ones
that *cost us* money to spend and excluding them could potentially
allow us to reach the target.
There's a third issue that was present before and remains even with this
fix: when we report the "available" funds in the error, we are ignoring
UTXOs with negative effective value, so it may look like there are less
funds in the wallet than there actually are.
I don't know how to convey the right message the user: if we actually
consider them we just make the "needed" value larger and larger (which
may be confusing, because if the user asks BDK to send 10k satoshis, why
do we say that we actually need 100k?), while if we don't we could report
an incorrect "available" value.
Before this commit `fee_amount` and `amount_needed` were passed as independent
parameters. From the perspective of coin selection algorithms, they are always
used jointly for the same purpose, to create a coin selection with a total
effective value greater than it's summed values.
This commit removes the abstraction that the use of the two parameter
introduced by consolidating both into a single parameter, `target_amount`, who
carries their values added up.
We would before calculate the TXIN_BASE_WEIGHT as prev_txid (32 bytes) +
prev_vout (4 bytes) + sequence (4 bytes) + script_sig_len (1 bytes), but
that's wrong: the script_sig_len shouldn't be included, as miniscript
already includes it in the `max_satisfaction_size` calculation.
Fixes#160
The former way to compute and create change was inside `create_tx`, just after
performing coin selection.
It blocked the opportunity to have an "ensemble" algorithm to decide between
multiple coin selection algorithms based on a metric, like Waste.
Now, change isn't created inside `coin_select` but the change amount and the
possibility to create change is decided inside the `coin_select` method. In
this way, change is associated with the coin selection algorithm that generated
it, and a method to decide between them can be implemented.
A `is_spent` field is added to LocalUtxo; when a txo is spent we set
this field to true instead of deleting the entire utxo from the
database.
This allows us to create txs double-spending txs already in blockchain.
Listunspent won't return spent utxos, effectively excluding them from the
coin selection and balance calculation
Add a unit test that passes a required utxo to the coin selection
algorithm that is less than the required spend. This tests that we get
that utxo included as well as tests that the rest of the coin selection
algorithm code also executes (i.e., that we do not short circuit
incorrectly).
If the required UTXO set is already bigger (including fees) than the
amount required for the transaction we can return early, no need to go
through the BNB algorithm or random selection.
It was just pointed out that we are calculating the virtual bytes
incorrectly by forgetting to take the ceiling after division by 4 [1]
Add helper functions to encapsulate all weight unit -> virtual byte
calculations including fee to and from fee rate. This makes the code
easier to read, easier to write, and gives us a better chance that bugs
like this will be easier to see.
As an added bonus we can also stop using f32 values for fee amount,
which is by definition an amount in sats so should be a u64. This
removes a bunch of casts and the need for epsilon comparisons and just
deep down feels nice :)
[1] https://github.com/bitcoindevkit/bdk/pull/386#discussion_r670882678
Clippy emits:
warning: struct constructor field order is inconsistent with struct
definition field order
As suggested, re-order the fields to be consistent with the struct
definition.
Adds fix for edge-cases involving small UTXOs (where value < fee) where the coin_select calculation would panic with overflow/underflow errors.
Bitcoin is limited to 21*(10^6), so any Bitcoin amount fits into i64.
To allow adding UTXOs external to the current wallet.
The caller must provide the psbt::Input so we can create a coherent PSBT
at the end and so this is compatible with existing PSBT workflows.
Main changes:
- There are now two types of UTXOs, local and foreign reflected in a
`Utxo` enum.
- `WeightedUtxo` now captures floating `(Utxo, usize)` tuples
- `CoinSelectionResult` now has methods on it for distinguishing between
local amount included vs total.
Fixes#251
TxBuilders are now not created directly but are created through the
wallet with `build_tx` and `build_fee_bump`.
The advantages of this realised in this commit are:
1. Normal tx creation and fee bumping use the code internally. The only
difference between normal tx and fee bump is how the builder is created.
2. The TxBuilder now has a refernce to the wallet and can therefore
lookup things as methods are called on it. `add_utxo` now uses this to
look up UTXO deta when it is called (rather than having to do it and
possibly error later on).
To support these changes `get_utxo` and `get_descriptor_for_keychain`
public methods have been added to Wallet. I could have kept them
pub(crate) but they seem like fine APIs to have publicly.
