importmulti: Don't add internal addresses to address book #14679
Conversation
This file contains 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
|
This seems to be a bug that has existed since I guess few people import change addresses. |
|
Yes, it's been here since the introduction I believe. With my hww setup I use PSBTs which have no notion of |
sipa
reviewed
Nov 7, 2018
src/wallet/rpcdump.cpp
Outdated
| // add to address book or update label | ||
| if (IsValidDestination(scriptpubkey_dest)) { | ||
| // if not internal add to address book or update label | ||
| if (!internal && IsValidDestination(scriptpubkey_dest)) { |
There was a problem hiding this comment.
I think this could just be:
if (!internal) {
assert(IsValidDestination(scriptpubkey_dest));
...
}
instagibbs
force-pushed
the
importmulti_internal_fix
branch
from
92a8469
to
7afddfa
Compare
|
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. |
|
Good catch, utACK 7afddfa |
|
utACK 7afddfa |
1 similar comment
|
utACK 7afddfa |
laanwj
added a commit
that referenced
this pull request
Nov 13, 2018
7afddfa importmulti: Don't add internal addresses to address book (Gregory Sanders) Pull request description: Currently anything imported with `internal` will not be treated as change since checking the address book is a primary test of this. Added basic tests of all combinations of arguments and change identification. Resolves #14662 Tree-SHA512: a1f08dc624a3fadee93cc5392d50c4796b0c5eedf38e295382f71570f2066d9e978ed6e3962084b902989863fe1273a8642d8fdb094a266d69de10622a4176b0
@fanquake Backport to what branch? |
|
Assigned 0.17.2, because it is not a new bug |
fanquake
pushed a commit
to fanquake/bitcoin
that referenced
this pull request
Dec 9, 2018
Github-Pull: bitcoin#14679 Rebased-From: 7afddfa
|
Removed "Needs backport", being done in #14900. |
meshcollider
added a commit
that referenced
this pull request
Dec 24, 2018
…sses to address book ae1b675 importmulti: Don't add internal addresses to address book (Gregory Sanders) Pull request description: Tree-SHA512: 749e4864862bc3402a0fad200578804ce6577b4d954fd3e741191b4c941df28d54f98422f90e7fdf9bdf9c01e53ec36cf970e61d22ccc0478771285dd8cef6a0
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Feb 14, 2020
Summary: 7afddfa8cefd01249ad59cf2370e7cec90b34f6f importmulti: Don't add internal addresses to address book (Gregory Sanders) Pull request description: Currently anything imported with `internal` will not be treated as change since checking the address book is a primary test of this. Added basic tests of all combinations of arguments and change identification. Resolves bitcoin/bitcoin#14662 Tree-SHA512: a1f08dc624a3fadee93cc5392d50c4796b0c5eedf38e295382f71570f2066d9e978ed6e3962084b902989863fe1273a8642d8fdb094a266d69de10622a4176b0 Backport of Core [[bitcoin/bitcoin#14679 | PR14679]] bitcoin/bitcoin#14679 Test Plan: ninja check ninja check-functional ./bitcoind ./bitcoin-cli help importmulti Verify changes to help text. Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D5288
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Currently anything imported with
internalwill not be treated as change since checking the address book is a primary test of this.Added basic tests of all combinations of arguments and change identification.
Resolves #14662