TxOrdering::Untouched no longer ensures the order of tx input

Describe the bug
Since add_utxo of build_tx in 1.2.0 uses hashmap to contain input, TxOrdrering cannot ensure that the order is not changed.

To Reproduce

    let utxos = wallet1
        .list_unspent()
        .map(|o| o.outpoint)
        .collect::<Vec<_>>();
    let builder = wallet1.build_tx();
    builder
        .ordering(bdk_wallet::TxOrdering::Untouched)
        .fee_rate(FeeRate::from_sat_per_vb(5).unwrap())
        .add_utxo(utxos[0])
        .unwrap()
        .add_utxo(utxos[1])
        .unwrap()
        .add_recipient(
            wallet1.peek_address(External, 0).address.script_pubkey(),
            bitcoin::Amount::from_sat(1000),
        );

    let inputs = builder.finish().unwrap().unsigned_tx.input;
    // can't assert
    assert!(inputs[0].previous_output == utxos[0]);
    assert!(inputs[1].previous_output == utxos[1]);

Expected behavior

    assert!(inputs[0].previous_output == utxos[0]);
    assert!(inputs[1].previous_output == utxos[1]);

Build environment

BDK tag/commit: 1.2.0
OS+version:
Rust/Cargo version: *
Rust/Cargo target: *

Additional context

bdk_wallet/crates/wallet/src/wallet/tx_builder.rs

Line 128 in 5817ed0

pub(crate) utxos: HashMap<OutPoint, WeightedUtxo>,

@nymius

@nymius

Sorry for the late reply. You're right, I over sighted the importance of the insertion order. However, I would like to explore a solution that does not include an extra dependency just for this case. I will try to think in something else.

fix(tx_builder): preserve insertion order with TxOrdering::Untouched

Also, coin selection has no guarantee of maintaining the ordering. There are two solutions to this:

the coin selection should only be allowed to return "additional" UTXOs so that the downstream code can guarantee the order is maintained
the coin selection trait should require implementations to maintain the order of required UTXOs to be at the start.

Currently this line in the default bnb coin selection algorithm is violating that (but it's also not part of the CoinSelection contract):

bdk_wallet/wallet/src/wallet/coin_selection.rs

Line 711 in 4d3116a

selected_utxos.append(&mut required_utxos);

I think opting for the first option would be best.

fix(tx_builder): preserve insertion order with TxOrdering::Untouched

Also, coin selection has no guarantee of maintaining the ordering. There are two solutions to this:
* the coin selection should only be allowed to return "additional" utxos so that the downstream code can guarantee the order is maintained

* the coin selection trait should require implementations to maintain the order of required vtxos to be at the start.
I think opting for the first option would be best.

I agree coin selection should only return additional UTxOs, and is something which has been already proposed before (look at the third point of the changelog of this old PR).

TxBuilder is expected to be replaced by bdk-tx, so its going to only receive fixes from now on, mainly.

As bdk-tx is leaning towards using bdk-coin-select. As long as I can tell, is also providing the required UTxOs to the selection algorithms.

If bdk-coin-select is not already doing what you propose, I think the change should be there.

fix(tx_builder): preserve insertion order with TxOrdering::Untouched

Currently this line in the default bnb coin selection algorithm is violating that (but it's also not part of the CoinSelection contract):

Still it clearly looks like a bug and should probably be fixed. It would make sense to have a test for every coin-select algo that manually selected inputs come first as long as we specify a tx ordering of Untouched.

An easy "fix" for the default algorithm is to swap the order in that line I linked. Then at least the default algo does what is expected.

…

On 6/24/25 6:27 PM, ValuedMammal ***@***.***> wrote: *ValuedMammal* left a comment (bitcoindevkit/bdk_wallet#244) <https:// github.com/bitcoindevkit/bdk_wallet/issues/244#issuecomment-3001297252> Currently this line in the default bnb coin selection algorithm is violating that (but it's also not part of the |CoinSelection| contract): Still it clearly looks like a bug and should probably be fixed. It would make sense to have a test for every coin-select algo that manually selected inputs come first as long as we specify a tx ordering of | Untouched|. — Reply to this email directly, view it on GitHub <https://github.com/ bitcoindevkit/bdk_wallet#244#issuecomment-3001297252>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ AAGQLXFIHXYXQIBAJE45E7T3FGC7FAVCNFSM6AAAAAB55ULKQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAMBRGI4TOMRVGI>. You are receiving this because you commented.Message ID: <bitcoindevkit/ ***@***.***>

fix(tx_builder): preserve insertion order with TxOrdering::Untouched

Merge #262: Preserve insertion order of manually selected utxos if `T…

Merge #390: fix(coin_selection): `calculate_cs_result` returns the re…

Navigation Menu

TxOrdering::Untouched no longer ensures the order of tx input #244

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

TxOrdering::Untouched no longer ensures the order of tx input #244

Description

Activity

eauxxs commented on May 28, 2025

nymius commented on Jun 9, 2025

stevenroose commented on Jun 23, 2025

nymius commented on Jun 23, 2025

ValuedMammal commented on Jun 24, 2025

stevenroose commented on Jun 24, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions