wallet: increase BnB upper limit #24752

  61 | @@ -62,9 +62,9 @@ struct {
  62 |  
  63 |  static const size_t TOTAL_TRIES = 100000;
  64 |  
  65 | -std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change)
  66 | +std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& upper_bound)
  67 |  {
  68 | -    SelectionResult result(selection_target);
  69 | +    SelectionResult result(selection_target, /* force_no_change=*/ true);

---

 759 | @@ -760,7 +760,8 @@ static bool CreateTransactionInternal(
 760 |  
 761 |      // vouts to the payees
 762 |      if (!coin_selection_params.m_subtract_fee_outputs) {
 763 | -        coin_selection_params.tx_noinputs_size = 11; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 output count, 1 witness overhead (dummy, flag, stack size)
 764 | +        coin_selection_params.tx_noinputs_size = 10; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 witness overhead (dummy, flag, stack size)

---

  26 | @@ -27,25 +27,25 @@ using interfaces::FoundBlock;
  27 |  namespace wallet {
  28 |  static constexpr size_t OUTPUT_GROUP_MAX_ENTRIES{100};
  29 |  
  30 | -int GetTxSpendSize(const CWallet& wallet, const CWalletTx& wtx, unsigned int out, bool use_max_sig)
  31 | +int GetTxSpendSize(const CWallet& wallet, const CWalletTx& wtx, unsigned int out, const CCoinControl* coin_control)

---

 397 | @@ -398,11 +398,8 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
 398 |          results.push_back(*knapsack_result);
 399 |      }
 400 |  
 401 | -    // Include change for SRD as we want to avoid making really small change if the selection just
 402 | -    // barely meets the target. Just use the lower bound change target instead of the randomly
 403 | -    // generated one, since SRD will result in a random change amount anyway; avoid making the
 404 | -    // target needlessly large.
 405 | -    const CAmount srd_target = nTargetValue + coin_selection_params.m_change_fee + CHANGE_LOWER;

---

 397 | -                                            coin_selection_params.m_min_change_target, coin_selection_params.rng_fast)}) {
 398 | +    // So we need to include that for KnapsackSolver and SRD as well, as we are expecting to create a change output.
 399 | +    if (!coin_selection_params.m_subtract_fee_outputs) {
 400 | +        target_with_change += coin_selection_params.m_change_fee;
 401 | +    }
 402 | +    if (auto knapsack_result{KnapsackSolver(all_groups, target_with_change, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast)}) {

---

 257 | @@ -258,15 +258,17 @@ struct SelectionResult
 258 |      bool m_use_effective{false};
 259 |      /** The computed waste */
 260 |      std::optional<CAmount> m_waste;
 261 | +    /** Force no change even if more than enough inputs are selected */
 262 | +    bool m_force_no_change{false};

---

  63 | @@ -64,7 +64,7 @@ static const size_t TOTAL_TRIES = 100000;
  64 |  
  65 |  std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change)
  66 |  {
  67 | -    SelectionResult result(selection_target, SelectionAlgorithm::BNB);
  68 | +    SelectionResult result(selection_target, SelectionAlgorithm::BNB, /*force_no_change=*/true);

---

 268 |      const SelectionAlgorithm m_algo;
 269 |  
 270 | -    explicit SelectionResult(const CAmount target, SelectionAlgorithm algo)
 271 | -        : m_target(target), m_algo(algo) {}
 272 | +    explicit SelectionResult(const CAmount target, SelectionAlgorithm algo, const bool force_no_change = false)
 273 | +        : m_target(target), m_algo(algo), m_force_no_change(force_no_change) {}

---

 449 | @@ -439,7 +450,18 @@ void SelectionResult::Clear()
 450 |  void SelectionResult::AddInput(const OutputGroup& group)
 451 |  {
 452 |      util::insert(m_selected_inputs, group.m_outputs);
 453 | -    m_use_effective = !group.m_subtract_fee_outputs;
 454 | +    m_use_effective = m_use_effective || !group.m_subtract_fee_outputs;

---

 461 | +    m_force_no_change |= other.m_force_no_change;
 462 | +    if (m_algo == SelectionAlgorithm::MANUAL) {
 463 | +        m_algo = other.m_algo;
 464 | +    }
 465 | +    util::insert(m_selected_inputs, other.m_selected_inputs);
 466 |  }

---

 515 | +    if (change <= min_change) { // TODO: less or less_or_equal
 516 | +        change = 0;
 517 | +    }
 518 | +
 519 | +    return change;
 520 | +}

---

 253 | @@ -254,6 +254,10 @@ struct SelectionResult
 254 |  private:
 255 |      /** Set of inputs selected by the algorithm to use in the transaction */
 256 |      std::set<COutput> m_selected_inputs;
 257 | +    /** The target the algorithm selected for. Note that this may not be equal to the recipient amount as it can include non-input fees */
 258 | +    CAmount m_target;

---

 253 | @@ -254,6 +254,10 @@ struct SelectionResult
 254 |  private:
 255 |      /** Set of inputs selected by the algorithm to use in the transaction */
 256 |      std::set<COutput> m_selected_inputs;
 257 | +    /** The target the algorithm selected for. Note that this may not be equal to the recipient amount as it can include non-input fees */
 258 | +    CAmount m_target;
 259 | +    /** The algorithm used to produce this result */
 260 | +    SelectionAlgorithm m_algo;

---

 433 | @@ -434,9 +434,17 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
 434 |               */
 435 |              preset_inputs.Insert(out, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false);
 436 |          }
 437 | +        if (preset_inputs.GetSelectionAmount() < nTargetValue) return std::nullopt;

---

 433 | @@ -434,9 +434,17 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
 434 |               */
 435 |              preset_inputs.Insert(out, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false);
 436 |          }
 437 | +        if (preset_inputs.GetSelectionAmount() < nTargetValue) return std::nullopt;
 438 | +
 439 | +        if (preset_inputs.GetSelectionAmount() > nTargetValue + coin_selection_params.m_cost_of_change) {

---

 584 | @@ -572,8 +585,8 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
 585 |      if (!res) return std::nullopt;
 586 |  
 587 |      // Add preset inputs to result
 588 | -    res->AddInput(preset_inputs);
 589 | -    if (res->m_algo == SelectionAlgorithm::MANUAL) {
 590 | +    res->Merge(preselected);

---

 381 | @@ -382,9 +382,10 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
 382 |      // Vector of results. We will choose the best one based on waste.
 383 |      std::vector<SelectionResult> results;
 384 |  
 385 | +    const auto cost_of_extra_input = coin_selection_params.m_effective_feerate.GetFee(DUMMY_NESTED_P2WPKH_INPUT_SIZE);
 386 |      // Note that unlike KnapsackSolver, we do not include the fee for creating a change output as BnB will not create a change output.
 387 |      std::vector<OutputGroup> positive_groups = GroupOutputs(wallet, coins, coin_selection_params, eligibility_filter, true /* positive_only */);
 388 | -    if (auto bnb_result{SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change)}) {
 389 | +    if (auto bnb_result{SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change + cost_of_extra_input)}) {

---