Add documentation fixes
This commit is contained in:
@@ -4,7 +4,7 @@ use super::*;
|
||||
pub enum BranchStrategy {
|
||||
/// We continue exploring subtrees of this node, starting with the inclusion branch.
|
||||
Continue,
|
||||
/// We continue exploring ONY the omission branch of this node, skipping the inclusion branch.
|
||||
/// We continue exploring ONLY the omission branch of this node, skipping the inclusion branch.
|
||||
SkipInclusion,
|
||||
/// We skip both the inclusion and omission branches of this node.
|
||||
SkipBoth,
|
||||
@@ -54,7 +54,7 @@ impl<'c, S: Ord> Bnb<'c, S> {
|
||||
/// Turns our [`Bnb`] state into an iterator.
|
||||
///
|
||||
/// `strategy` should assess our current selection/node and determine the branching strategy and
|
||||
/// whether this selection is a candidate solution (if so, return the score of the selection).
|
||||
/// whether this selection is a candidate solution (if so, return the selection score).
|
||||
pub fn into_iter<'f>(self, strategy: &'f DecideStrategy<'c, S>) -> BnbIter<'c, 'f, S> {
|
||||
BnbIter {
|
||||
state: self,
|
||||
@@ -70,7 +70,7 @@ impl<'c, S: Ord> Bnb<'c, S> {
|
||||
let (index, candidate) = self.pool[pos];
|
||||
|
||||
if self.selection.is_selected(index) {
|
||||
// deselect last `pos`, so next round will check omission branch
|
||||
// deselect the last `pos`, so the next round will check the omission branch
|
||||
self.pool_pos = pos;
|
||||
self.selection.deselect(index);
|
||||
true
|
||||
@@ -82,7 +82,7 @@ impl<'c, S: Ord> Bnb<'c, S> {
|
||||
})
|
||||
}
|
||||
|
||||
/// Continue down this branch, skip inclusion branch if specified.
|
||||
/// Continue down this branch and skip the inclusion branch if specified.
|
||||
pub fn forward(&mut self, skip: bool) {
|
||||
let (index, candidate) = self.pool[self.pool_pos];
|
||||
self.rem_abs -= candidate.value;
|
||||
@@ -93,7 +93,7 @@ impl<'c, S: Ord> Bnb<'c, S> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Compare advertised score with current best. New best will be the smaller value. Return true
|
||||
/// Compare the advertised score with the current best. The new best will be the smaller value. Return true
|
||||
/// if best is replaced.
|
||||
pub fn advertise_new_score(&mut self, score: S) -> bool {
|
||||
if score <= self.best_score {
|
||||
@@ -108,7 +108,7 @@ pub struct BnbIter<'c, 'f, S> {
|
||||
state: Bnb<'c, S>,
|
||||
done: bool,
|
||||
|
||||
/// Check our current selection (node), and returns the branching strategy, alongside a score
|
||||
/// Check our current selection (node) and returns the branching strategy alongside a score
|
||||
/// (if the current selection is a candidate solution).
|
||||
strategy: &'f DecideStrategy<'c, S>,
|
||||
}
|
||||
@@ -133,7 +133,7 @@ impl<'c, 'f, S: Ord + Copy + Display> Iterator for BnbIter<'c, 'f, S> {
|
||||
|
||||
debug_assert!(
|
||||
!strategy.will_continue() || self.state.pool_pos < self.state.pool.len(),
|
||||
"Faulty strategy implementation! Strategy suggested that we continue traversing, however we have already reached the end of the candidates pool! pool_len={}, pool_pos={}",
|
||||
"Faulty strategy implementation! Strategy suggested that we continue traversing, however, we have already reached the end of the candidates pool! pool_len={}, pool_pos={}",
|
||||
self.state.pool.len(), self.state.pool_pos,
|
||||
);
|
||||
|
||||
@@ -187,15 +187,15 @@ impl From<core::time::Duration> for BnbLimit {
|
||||
/// in Bitcoin Core).
|
||||
///
|
||||
/// The differences are as follows:
|
||||
/// * In additional to working with effective values, we also work with absolute values.
|
||||
/// This way, we can use bounds of absolute values to enforce `min_absolute_fee` (which is used by
|
||||
/// * In addition to working with effective values, we also work with absolute values.
|
||||
/// This way, we can use bounds of the absolute values to enforce `min_absolute_fee` (which is used by
|
||||
/// RBF), and `max_extra_target` (which can be used to increase the possible solution set, given
|
||||
/// that the sender is okay with sending extra to the receiver).
|
||||
///
|
||||
/// Murch's Master Thesis: <https://murch.one/wp-content/uploads/2016/11/erhardt2016coinselection.pdf>
|
||||
/// Bitcoin Core Implementation: <https://github.com/bitcoin/bitcoin/blob/23.x/src/wallet/coinselection.cpp#L65>
|
||||
///
|
||||
/// TODO: Another optimization we could do is figure out candidate with smallest waste, and
|
||||
/// TODO: Another optimization we could do is figure out candidates with the smallest waste, and
|
||||
/// if we find a result with waste equal to this, we can just break.
|
||||
pub fn coin_select_bnb<L>(limit: L, selector: CoinSelector) -> Option<CoinSelector>
|
||||
where
|
||||
@@ -203,7 +203,7 @@ where
|
||||
{
|
||||
let opts = selector.opts;
|
||||
|
||||
// prepare pool of candidates to select from:
|
||||
// prepare the pool of candidates to select from:
|
||||
// * filter out candidates with negative/zero effective values
|
||||
// * sort candidates by descending effective value
|
||||
let pool = {
|
||||
@@ -231,12 +231,12 @@ where
|
||||
let selected_abs = bnb.selection.selected_absolute_value();
|
||||
let selected_eff = bnb.selection.selected_effective_value();
|
||||
|
||||
// backtrack if remaining value is not enough to reach target
|
||||
// backtrack if the remaining value is not enough to reach the target
|
||||
if selected_abs + bnb.rem_abs < target_abs || selected_eff + bnb.rem_eff < target_eff {
|
||||
return (BranchStrategy::SkipBoth, None);
|
||||
}
|
||||
|
||||
// backtrack if selected value already surpassed upper bounds
|
||||
// backtrack if the selected value has already surpassed upper bounds
|
||||
if selected_abs > upper_bound_abs && selected_eff > upper_bound_eff {
|
||||
return (BranchStrategy::SkipBoth, None);
|
||||
}
|
||||
@@ -244,7 +244,7 @@ where
|
||||
let selected_waste = bnb.selection.selected_waste();
|
||||
|
||||
// when feerate decreases, waste without excess is guaranteed to increase with each
|
||||
// selection. So if we have already surpassed best score, we can backtrack.
|
||||
// selection. So if we have already surpassed the best score, we can backtrack.
|
||||
if feerate_decreases && selected_waste > bnb.best_score {
|
||||
return (BranchStrategy::SkipBoth, None);
|
||||
}
|
||||
@@ -270,11 +270,11 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
// check out inclusion branch first
|
||||
// check out the inclusion branch first
|
||||
(BranchStrategy::Continue, None)
|
||||
};
|
||||
|
||||
// determine sum of absolute and effective values for current selection
|
||||
// determine the sum of absolute and effective values for the current selection
|
||||
let (selected_abs, selected_eff) = selector.selected().fold((0, 0), |(abs, eff), (_, c)| {
|
||||
(
|
||||
abs + c.value,
|
||||
@@ -376,7 +376,7 @@ mod test {
|
||||
);
|
||||
}
|
||||
|
||||
/// `cost_of_change` acts as the upper-bound in Bnb, we check whether these boundaries are
|
||||
/// `cost_of_change` acts as the upper-bound in Bnb; we check whether these boundaries are
|
||||
/// enforced in code
|
||||
#[test]
|
||||
fn cost_of_change() {
|
||||
@@ -412,7 +412,7 @@ mod test {
|
||||
(lowest_opts, highest_opts)
|
||||
};
|
||||
|
||||
// test lowest possible target we are able to select
|
||||
// test lowest possible target we can select
|
||||
let lowest_eval = evaluate_bnb(CoinSelector::new(&candidates, &lowest_opts), 10_000);
|
||||
assert!(lowest_eval.is_ok());
|
||||
let lowest_eval = lowest_eval.unwrap();
|
||||
@@ -426,7 +426,7 @@ mod test {
|
||||
0.0
|
||||
);
|
||||
|
||||
// test highest possible target we are able to select
|
||||
// test the highest possible target we can select
|
||||
let highest_eval = evaluate_bnb(CoinSelector::new(&candidates, &highest_opts), 10_000);
|
||||
assert!(highest_eval.is_ok());
|
||||
let highest_eval = highest_eval.unwrap();
|
||||
@@ -587,8 +587,8 @@ mod test {
|
||||
});
|
||||
}
|
||||
|
||||
/// For a decreasing feerate (longterm feerate is lower than effective feerate), we should
|
||||
/// select less. For increasing feerate (longterm feerate is higher than effective feerate), we
|
||||
/// For a decreasing feerate (long-term feerate is lower than effective feerate), we should
|
||||
/// select less. For increasing feerate (long-term feerate is higher than effective feerate), we
|
||||
/// should select more.
|
||||
#[test]
|
||||
fn feerate_difference() {
|
||||
@@ -639,7 +639,7 @@ mod test {
|
||||
/// * We should only have `ExcessStrategy::ToDrain` when `drain_value >= min_drain_value`.
|
||||
/// * Fuzz
|
||||
/// * Solution feerate should never be lower than target feerate
|
||||
/// * Solution fee should never be lower than `min_absolute_fee`
|
||||
/// * Solution fee should never be lower than `min_absolute_fee`.
|
||||
/// * Preselected should always remain selected
|
||||
fn _todo() {}
|
||||
}
|
||||
|
||||
@@ -10,7 +10,7 @@ pub struct WeightedValue {
|
||||
/// `txin` fields: `prevout`, `nSequence`, `scriptSigLen`, `scriptSig`, `scriptWitnessLen`,
|
||||
/// `scriptWitness` should all be included.
|
||||
pub weight: u32,
|
||||
/// Total number of inputs; so we can calculate extra `varint` weight due to `vin` len changes.
|
||||
/// The total number of inputs; so we can calculate extra `varint` weight due to `vin` length changes.
|
||||
pub input_count: usize,
|
||||
/// Whether this [`WeightedValue`] contains at least one segwit spend.
|
||||
pub is_segwit: bool,
|
||||
@@ -33,7 +33,7 @@ impl WeightedValue {
|
||||
|
||||
/// Effective value of this input candidate: `actual_value - input_weight * feerate (sats/wu)`.
|
||||
pub fn effective_value(&self, effective_feerate: f32) -> i64 {
|
||||
// We prefer undershooting the candidate's effective value (so we over estimate the fee of a
|
||||
// We prefer undershooting the candidate's effective value (so we over-estimate the fee of a
|
||||
// candidate). If we overshoot the candidate's effective value, it may be possible to find a
|
||||
// solution which does not meet the target feerate.
|
||||
self.value as i64 - (self.weight as f32 * effective_feerate).ceil() as i64
|
||||
@@ -43,8 +43,8 @@ impl WeightedValue {
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub struct CoinSelectorOpt {
|
||||
/// The value we need to select.
|
||||
/// If the value is `None` then the selection will be complete if it can pay for the drain
|
||||
/// output and satisfy the other constraints (e.g. minimum fees).
|
||||
/// If the value is `None`, then the selection will be complete if it can pay for the drain
|
||||
/// output and satisfy the other constraints (e.g., minimum fees).
|
||||
pub target_value: Option<u64>,
|
||||
/// Additional leeway for the target value.
|
||||
pub max_extra_target: u64, // TODO: Maybe out of scope here?
|
||||
@@ -53,10 +53,10 @@ pub struct CoinSelectorOpt {
|
||||
pub target_feerate: f32,
|
||||
/// The feerate
|
||||
pub long_term_feerate: Option<f32>, // TODO: Maybe out of scope? (waste)
|
||||
/// The minimum absolute fee. I.e. needed for RBF.
|
||||
/// The minimum absolute fee. I.e., needed for RBF.
|
||||
pub min_absolute_fee: u64,
|
||||
|
||||
/// The weight of the template transaction including fixed fields and outputs.
|
||||
/// The weight of the template transaction, including fixed fields and outputs.
|
||||
pub base_weight: u32,
|
||||
/// Additional weight if we include the drain (change) output.
|
||||
pub drain_weight: u32,
|
||||
@@ -130,7 +130,7 @@ impl CoinSelectorOpt {
|
||||
}
|
||||
}
|
||||
|
||||
/// [`CoinSelector`] is responsible for selecting and deselecting from a set of canididates.
|
||||
/// [`CoinSelector`] selects and deselects from a set of candidates.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct CoinSelector<'a> {
|
||||
pub opts: &'a CoinSelectorOpt,
|
||||
@@ -303,7 +303,7 @@ impl<'a> CoinSelector<'a> {
|
||||
let target_value = self.opts.target_value.unwrap_or(0);
|
||||
let selected = self.selected_absolute_value();
|
||||
|
||||
// find the largest unsatisfied constraint (if any), and return error of that constraint
|
||||
// find the largest unsatisfied constraint (if any), and return the error of that constraint
|
||||
// "selected" should always be greater than or equal to these selected values
|
||||
[
|
||||
(
|
||||
@@ -321,8 +321,7 @@ impl<'a> CoinSelector<'a> {
|
||||
(
|
||||
SelectionConstraint::MinDrainValue,
|
||||
// when we have no target value (hence no recipient txouts), we need to ensure
|
||||
// the selected amount can satisfy requirements for a drain output (so we at
|
||||
// least have one txout)
|
||||
// the selected amount can satisfy requirements for a drain output (so we at least have one txout)
|
||||
if self.opts.target_value.is_none() {
|
||||
(fee_with_drain + self.opts.min_drain_value).saturating_sub(selected)
|
||||
} else {
|
||||
@@ -354,8 +353,8 @@ impl<'a> CoinSelector<'a> {
|
||||
let mut excess_strategies = HashMap::new();
|
||||
|
||||
// only allow `ToFee` and `ToRecipient` excess strategies when we have a `target_value`,
|
||||
// otherwise we will result in a result with no txouts, or attempt to add value to an output
|
||||
// that does not exist
|
||||
// otherwise, we will result in a result with no txouts, or attempt to add value to an output
|
||||
// that does not exist.
|
||||
if self.opts.target_value.is_some() {
|
||||
// no drain, excess to fee
|
||||
excess_strategies.insert(
|
||||
@@ -369,7 +368,7 @@ impl<'a> CoinSelector<'a> {
|
||||
},
|
||||
);
|
||||
|
||||
// no drain, excess to recipient
|
||||
// no drain, send the excess to the recipient
|
||||
// if `excess == 0`, this result will be the same as the previous, so don't consider it
|
||||
// if `max_extra_target == 0`, there is no leeway for this strategy
|
||||
if excess_without_drain > 0 && self.opts.max_extra_target > 0 {
|
||||
@@ -407,7 +406,7 @@ impl<'a> CoinSelector<'a> {
|
||||
|
||||
debug_assert!(
|
||||
!excess_strategies.is_empty(),
|
||||
"should have at least one excess strategy"
|
||||
"should have at least one excess strategy."
|
||||
);
|
||||
|
||||
Ok(Selection {
|
||||
@@ -529,7 +528,7 @@ mod test {
|
||||
|
||||
use super::{CoinSelector, CoinSelectorOpt, WeightedValue};
|
||||
|
||||
/// Ensure `target_value` is respected. Can't have no disrespect.
|
||||
/// Ensure `target_value` is respected. Can't have any disrespect.
|
||||
#[test]
|
||||
fn target_value_respected() {
|
||||
let target_value = 1000_u64;
|
||||
@@ -611,6 +610,6 @@ mod test {
|
||||
/// TODO: Tests to add:
|
||||
/// * `finish` should ensure at least `target_value` is selected.
|
||||
/// * actual feerate should be equal or higher than `target_feerate`.
|
||||
/// * actual drain value should be equal or higher than `min_drain_value` (or else no drain).
|
||||
/// * actual drain value should be equal to or higher than `min_drain_value` (or else no drain).
|
||||
fn _todo() {}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user