Bitcoin V2 TransactionBuilder fails to create valid transaction due to optional changeOutput cost miscalculation.

[paulo-bc]

Describe the bug

Build a transaction with one P2WPKH input that should spend all of it (using both $0.maxAmountOutput = .with { $0.toAddress = p2wpkhReceiveAddress0 } and $0.inputSelector = .useAll). This transaction is correctly build.
Create another transaction, but now instead of using maxAmountOutput property, specify the result amount from the previous test.
We expect this to correctly build and be similar to the previous result, but instead we get a error SigningError.errorNotEnoughUtxos.

To Reproduce

    func test_wc_max_build() throws {
        // Test Data

        // TestNet3 Data
        let p2wpkhReceiveAddress0 = "tb1qal5xl77a7d9vtaafda960lcxexn3uw9fqpdds5"
        let p2wpkhTxId = "902f0828ffcd8f35fe3f4903bd0a871c429cb5685b7a74cf262287f590ef40d2"
        let p2wpkhValue: Int64 = 2_000
        let p2wpkhIndex: UInt32 = 2
        let p2wpkh_pk_str = "1ec2a0f68e84edf0a6811dc2d35cf366bb495cf2475dbdf7a0545aab54864b22"

        // P2WPKH UTXO

        let p2wpkhPrivateKeyData = Data(hexString: p2wpkh_pk_str)!
        let p2wpkhPrivateKey = PrivateKey(data: p2wpkhPrivateKeyData)!
        let p2wpkhPublicKey = p2wpkhPrivateKey.getPublicKeySecp256k1(compressed: false)
        let p2wpkhUtxo = BitcoinV2Input.with {
            $0.outPoint = BitcoinV2OutPoint.with {
                $0.hash = Data.reverse(hexString: p2wpkhTxId)
                $0.vout = p2wpkhIndex
            }
            $0.value = p2wpkhValue
            $0.sighashType = BitcoinSigHashType.all.rawValue
            $0.scriptBuilder = .with { $0.p2Wpkh = .with { $0.pubkey = p2wpkhPublicKey.data } }
        }


        // Test `maxAmountOutput`

        let output_max: BitcoinSigningOutput = AnySigner.sign(
            input: BitcoinSigningInput.with {
                $0.signingV2 = .with {
                    $0.privateKeys = [p2wpkhPrivateKeyData]
                    $0.builder = .with {
                        $0.version = .v2
                        $0.inputs = [p2wpkhUtxo]
                        $0.maxAmountOutput = .with { $0.toAddress = p2wpkhReceiveAddress0 }
                        $0.inputSelector = .useAll
                        $0.feePerVb = 2
                        $0.fixedDustThreshold = 546
                    }
                }
            },
            coin: .bitcoin
        )
        let result_max = output_max.signingResultV2
        let expectedOutputValue: Int64 = 1780
        XCTAssertEqual(result_max.error, .ok)
        XCTAssertEqual(result_max.transaction.inputs.count, 1)
        XCTAssertEqual(result_max.transaction.outputs.count, 1)
        XCTAssertEqual(result_max.transaction.outputs[0].value, expectedOutputValue)
        XCTAssertEqual(result_max.fee, p2wpkhValue - expectedOutputValue)


        // Test creation of a transaction with `expectedOutputValue`.

        let out0 = BitcoinV2Output.with {
            $0.value = expectedOutputValue
            $0.toAddress = p2wpkhReceiveAddress0
        }

        let output_select: BitcoinSigningOutput = AnySigner.sign(
            input: BitcoinSigningInput.with {
                $0.signingV2 = .with {
                    $0.privateKeys = [p2wpkhPrivateKeyData]
                    $0.builder = .with {
                        $0.version = .v2
                        $0.inputs = [p2wpkhUtxo]
                        $0.outputs = [out0]
                        // Commenting out the next line will make tests pass.
                        $0.changeOutput = .with { $0.toAddress = p2wpkhReceiveAddress0 }
                        $0.inputSelector = .selectInOrder
                        $0.feePerVb = 2
                        $0.fixedDustThreshold = 546
                    }
                }
            },
            coin: .bitcoin
        )
        let result_select = output_select.signingResultV2
        XCTAssertEqual(result_select.error, .ok)
        XCTAssertEqual(result_select.transaction.inputs.count, 1)
        XCTAssertEqual(result_select.transaction.outputs.count, 1)
        XCTAssertEqual(result_select.transaction.outputs.first?.value, expectedOutputValue)
        XCTAssertEqual(result_select.fee, p2wpkhValue - expectedOutputValue)
    }

Expected behavior

We expect result_select to correctly build and be similar to result_max. We also expect that result_select won't produce a change output given it's inputs covers outputs+fee exactly.
But instead we get an error Not enough non-dust input UTXOs to cover requested amount (dust UTXOs are filtered out)\nContext:\n0. Insufficient funds to generate a transaction. Available '2000', required '1780' + fee '282'\n1. Error selecting UTXOs

Additional context
Commenting out result_select's $0.changeOutput = .with { $0.toAddress = p2wpkhReceiveAddress0 } will allow the test to pass.

[paulo-bc]

Seems like after ExactInputSelector checks that !total_covered, we should recalculate without the change_output cost.

[satoshiotomakan]

Hi @paulo-bc, good note! I think I can run ExactInputSelector::select_inputs twice with and without change output, and see if it doesn't fail at least in one case 👍
I'll work on it next week most likely