[Bitcoin] Set Input Selector to UseAll for Legacy Taproot Building/Signing

[lamafab]

(Context: We were debugging a BRC20 related issue in Wallet Kit)

So, this took us a while to figure out. One single change is required:

    // rust/tw_bitcoin/src/modules/legacy/build_and_sign.rs

    // Normally we only select enough inputs to cover the output balance. However, since
    // some transaction types require precise input ordering (such as BRC20), we
    // do not sort the inputs and use the ordering as provided by the caller.
    let input_selector = UtxoProto::InputSelector::UseAll;  // <-- Used to be `SelectInOrder`

I did some playing around and technically, the existing code should have worked, but the issue is in tw_utxo; it does not consider the fees at all when selecting inputs. It does make a fee estimate after the inputs are selected and the issue eventually gets caught by the code:

    // rust/tw_utxo/src/compiler.rs

    // Calculate the full weight projection (base weight + input & output weight).
    let weight_estimate = tx.weight().to_wu() + input_weight + output_weight;
    let fee_estimate = (weight_estimate + 3) / 4 * proto.weight_base;

    // Check if the fee projection would make the change amount negative
    // (implying insufficient input amount).
    let change_amount_before_fee = total_input - total_output;
    if change_amount_before_fee < fee_estimate {
        return Err(Error::from(Proto::Error::Error_insufficient_inputs));
    }

But of course, the input selector should consider fees in the first place when selecting inputs (@JaimeToca the reason we didn't get this specific error is because we used byte_fee = 0!). In order to implement this correctly, we'd need to iterate over each input and re-calculate the fee (estimation) on each iteration until we cover total_output + fee_estimate (or return an error if otherwise).

This this PR makes sure that @JaimeToca can continue working on BRC20 support in Wallet Kit, but the situation is not ideal given that this is DANGEROUS: if the caller does not use ANY_PLAN first, the call to Rust::tw_bitcoin_legacy_taproot_build_and_sign_transaction will simply use-up ALL inputs WITHOUT generating a return transaction (meaning, the miner gets the full change amount as a reward). However, this might be fine given that Rust::tw_bitcoin_legacy_taproot_build_and_sign_transaction already assumes that the change output is provided upfront by the caller anyway (we set disable_change_output = true). I let you (@satoshiotomakan) decide.

Alternatively, I wrote a PoC implementation of how to do the input selection for covering fees correctly. But given that you're currently restructuring tw_utxo anyway then this might interfere with your plan:

    // rust/tw_utxo/src/compiler.rs

    // Select the inputs accordingly by updating `proto.inputs`.
    let available = std::mem::take(&mut proto.inputs);
    match &proto.input_selector {
        Proto::InputSelector::UseAll => {
            // Simply add all inputs.
            for txin in available {
                let n_txin = convert_proto_to_txin(&txin)?;
                tx.input.push(n_txin);

                // Track selected input
                proto.inputs.push(txin);
            }
        },
        Proto::InputSelector::SelectInOrder | Proto::InputSelector::SelectAscending => {
            let mut total_input_amount = 0;
            let mut total_input_weight = 0;

            // For each iteration, we calculate the full fee estimate and
            // exit when the total amount + fees have been covered.
            for txin in available {
                let n_txin = convert_proto_to_txin(&txin)?;
                tx.input.push(n_txin);

                // Update input amount and weight.
                total_input_amount += txin.value;
                total_input_weight += txin.weight_estimate; // contains scriptSig/Witness weight

                // Track selected input
                proto.inputs.push(txin);

                // Calculate the full weight projection (base weight + input
                // weight + output weight). Note that the change output itself is
                // not included in the transaction yet.
                let weight_estimate =
                    tx.weight().to_wu() + total_input_weight + change_output_weight;
                let fee_estimate = (weight_estimate + 3) / 4 * proto.weight_base;

                if total_input_amount >= total_output_amount + fee_estimate {
                    // Enough inputs to cover the output and fee estimate.
                    break;
                }
            }
        },
    };

See #3667 for the full code. The tests look fine, but need to be adjusted because the fee is calculated slightly differently now. E.g:

---- transaction_plan_compose_brc20 stdout ----
thread 'transaction_plan_compose_brc20' panicked at 'assertion failed: `(left == right)`
  left: `100000671`,
 right: `99992979`', tw_bitcoin/tests/plan_builder.rs:129:9

[lamafab]

Btw, here it should update the proto.inputs based on the selected UTXOs in pre_signed:

wallet-core/rust/tw_bitcoin/src/modules/signer.rs

Line 34 in b65adc4

let pre_signed = BitcoinEntry.preimage_hashes_impl(_coin, proto.clone())?;

before it gets passed on to:

wallet-core/rust/tw_bitcoin/src/modules/signer.rs

Line 50 in b65adc4

BitcoinEntry.compile_impl(_coin, proto, signatures, vec![])

But it does not do that.

[satoshiotomakan]

tw_bitcoin_legacy_taproot_build_and_sign_transaction should not longer be used, but the Bitcoin Planning/Signing 2.0 instead.
It was done in #3665.
@JaimeToca did you have a chance to try the new API?

[lamafab]

Closing in favor of #3667 (Legacy should NOT be used)