wallet: Fix sendall with watchonly wallets and specified inputs #26344
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sendall should be using a bitwise AND for sendall's IsMine check rather than an equality as IsMine will never return ISMINE_ALL.
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Member
|
Does this need backporting? |
Member
Author
Yes |
furszy
approved these changes
Oct 20, 2022
Member
furszy
left a comment
furszy
left a comment
There was a problem hiding this comment.
Code review ACK d442101d
With an extra crash finding that doesn't necessarily need to be included here.
achow101
force-pushed
the
fix-sendall-watchonly
branch
from
d442101 to
315fd4d
Compare
Member
Author
I prefer to keep implementation separate from test so that tests can be easily cherry-picked onto master to verify that they fail. |
furszy
approved these changes
Oct 20, 2022
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Oct 23, 2022
…pecified inputs 315fd4d test: Test for out of bounds vout in sendall (Andrew Chow) b132c85 wallet: Check utxo prevout index out of bounds in sendall (Andrew Chow) 708b72b test: Test that sendall works with watchonly spending specific utxos (Andrew Chow) 6bcd7e2 wallet: Correctly check ismine for sendall (Andrew Chow) Pull request description: The `sendall` RPC would previously fail when used with a watchonly wallet and specified inputs. This failure was caused by checking isminetype equality with ISMINE_ALL rather than a bitwise AND as IsMine can never return ISMINE_ALL. Also added a test. ACKs for top commit: w0xlt: ACK bitcoin@315fd4d furszy: ACK 315fd4d Tree-SHA512: fb55cf6524e789964770b803f401027319f0351433ea084ffa7c5e6f1797567a608c956b7f7c5bd542aa172c4b7b38b07d0976f5ec587569efead27266e8664c
fanquake
pushed a commit
to fanquake/bitcoin
that referenced
this pull request
Oct 28, 2022
sendall should be using a bitwise AND for sendall's IsMine check rather than an equality as IsMine will never return ISMINE_ALL. Github-Pull: bitcoin#26344 Rebased-From: 6bcd7e2
fanquake
pushed a commit
to fanquake/bitcoin
that referenced
this pull request
Oct 28, 2022
Github-Pull: bitcoin#26344 Rebased-From: 708b72b
fanquake
pushed a commit
to fanquake/bitcoin
that referenced
this pull request
Oct 28, 2022
Github-Pull: bitcoin#26344 Rebased-From: b132c85
fanquake
pushed a commit
to fanquake/bitcoin
that referenced
this pull request
Oct 28, 2022
Github-Pull: bitcoin#26344 Rebased-From: 315fd4d
Merged
Member
|
Backported in #26410. |
fanquake
added a commit
that referenced
this pull request
Oct 31, 2022
d570190 rpc: make `address` field optional (w0xlt) e4b8c9b rpc: add non-regression test about deriveaddresses crash when index is 2147483647 (muxator) bf2bf73 rpc: fix crash in deriveaddresses when derivation index is 2147483647 (muxator) b04f5f9 test: Test for out of bounds vout in sendall (Andrew Chow) dedee6a wallet: Check utxo prevout index out of bounds in sendall (Andrew Chow) 931db78 test: Test that sendall works with watchonly spending specific utxos (Andrew Chow) bbe864a wallet: Correctly check ismine for sendall (Andrew Chow) 4b7d30d Adjust `.tx/config` for new Transifex CLI (Hennadii Stepanov) Pull request description: Backports: * #26321 * #26344 * #26275 * #26349 ACKs for top commit: instagibbs: ACK d570190 hebasto: ACK d570190, I've cherry-picked commits manually and got zero diff with this PR branch. Tree-SHA512: dad64f4074b4f06d666c0f2d804eda92df241bcce0a49c28486311a151f2e9d46b75e1bce02de570dcc85957c9ce936debb2a4faa045800c9757c6c495115d7c
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
sendallRPC would previously fail when used with a watchonly wallet and specified inputs. This failure was caused by checking isminetype equality with ISMINE_ALL rather than a bitwise AND as IsMine can never return ISMINE_ALL.Also added a test.