This adds a test for and fixes issue #2063.
The changes do the following:
1. Build a preliminary list of candidate UTXOs.
2. Use the preliminary list to fix unconnected/valueless inputs before
calculating `valueNeeded`.
3. Create a new candidate list without the UTXOs that were already present
in inputs and use it as input for generating the `CoinSelection`.
Test changes:
1. Update existing test to show the improvement (i.e. no wasted fee).
2. Add a new test (based on PR #3055) that shows no duplicate inputs.
This work is based on changes by @michael-swiggs in PR #3055.
Note that this changes behavior slightly. We are now warning on inputs with null value
and skipping inputs with no value in the sum. Previously we were checking for non-null
getConnectedOutput(). I reviewed the code and whenever getConnectedOutput() is non-null,
getValue() should also be non-null. There might be cases where we have a value and
no connected output, but in those cases I think we should use the value.
Without this fix (and code cleanup) the `OP_RETURN` limit is not
enforced when `ensureMinRequiredFee` is false or `emptyWallet` is true.
Parameterize `twoOpReturnsPerTransactionTest()` to test all combinations
of `ensureMinRequiredFee` and `emptyWallet`.
The current implementation is based on plain P2SH. I assume as of
today one would prefer an implementation based on taproot or at least
segwit (P2WSH).
Note the `isFollowing` and `sigsRequiredToSpend` fields in the
`DeterministicKey` message of the wallet protobuf are preserved for
now, because they might come in handy for a future implementation and
in general it's hard to remove fields from protobufs.
Also removes the `marry` action from `WalletTool`.
* The check against `maxMoney` is removed. There BIP21 spec does not restrict
the amount towards positive infinity. The new maximum is now dictated by
`Coin.parseCoin()`: `Long.MAX_VALUE` satoshis
* The check for negative amounts now throws `OptionalFieldValidationException`
directly, rather than re-throwing via `ArithmeticException`.
* Exception messages now use string concatenation like everywhere else.
* Tests around the new max amount are added/changed.
* Adds new methods taking `Network` rather than `NetworkParameters`
* Deprecates all converted methods
* Updates tests, examples, and tools that use these calls
* A static constructur `read()` that deserializes from a payload.
* A `write()` helper to write to a buffer.
* A `serialize()` helper to get the serialized bytes.
* A `BYTES` constant for the number of serialized bytes.
Includes a test.
Rather than log an error at runtime, the compiler should fail with an error
when subclasses of `BaseMessage` don't implement `bitcoinSerializeToStream()`.