Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Nov 15, 2021

This is for consistency with the send_self_transfer method.

Also, remove the feature that the change of the last transfer can be retrieved via get_utxo. This can trivially and clearer be achieved by simply passing the txid of the transfer.

Also, this fixes the bug in feature_txindex_compatibility in current master after a silent merge conflict.

Fixes #23514

@maflcko
Copy link
Member Author

maflcko commented Nov 15, 2021

Going to merge this, but maybe someone donates a post-merge ack some day?

@Sjors
Copy link
Member

Sjors commented Nov 15, 2021

Looks like @theStack and @mjdietzx last touched this, so perhaps they can check this change. See also #20385

@maflcko maflcko deleted the 2111-testMiniwalletLargeUtxo branch November 15, 2021 16:30
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post-merge code-review ACK fa62207 🪐

Note that this changes the functionality of create_self_transfer slightly: before it would always sort the internal UTXOs first, now it would only do that if no utxo_to_spend is passed. I don't think this is a problem though -- actually it seems to be more logical to only sort if needed.

Also checked that only one (out of 7) instance of .get_utxo() calls without parameters are expecting to get the change output, which is tackled in rpc_txoutproof.py by passing the txid.

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 16, 2021
…utxo

fa62207 test: Return the largest utxo in MiniWallet.get_utxo (MarcoFalke)

Pull request description:

  This is for consistency with the `send_self_transfer` method.

  Also, remove the feature that the change of the last transfer can be retrieved via `get_utxo`. This can trivially and clearer be achieved by simply passing the txid of the transfer.

  Also, this fixes the bug in `feature_txindex_compatibility` in current master after a silent merge conflict.

  Fixes bitcoin#23514

Top commit has no ACKs.

Tree-SHA512: edd066d372aaa72b4e0fc7526f84931c8d1f6d14f53678cb7832bc8e3d211f44b90ec9c59b7d915ef24acc63a36e7d66c8d3b7598355bd490ac637ed3bcc3dff
@bitcoin bitcoin locked and limited conversation to collaborators Nov 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

qa: Failure in feature_txindex_compatibility.py on master
4 participants