Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove duplicate destination decoding #11259

Merged
merged 2 commits into from Sep 6, 2017
Merged

Remove duplicate destination decoding #11259

merged 2 commits into from Sep 6, 2017

Conversation

promag
Copy link
Member

@promag promag commented Sep 6, 2017

Follow up of #11117, this patch removes an extra unnecessary destination decoding that was identified while reviewing #11117. It is also the only case where IsValidDestinationString is called before DecodeDestination.

For reference see comment.

@laanwj
Copy link
Member

laanwj commented Sep 6, 2017

utACK 6bc6e24

@sipa
Copy link
Member

sipa commented Sep 6, 2017

utACK 6bc6e241176b4a3acd1be2c12b7ffa7f1f65bd56

@meshcollider
Copy link
Contributor

utACK 6bc6e24

@promag promag force-pushed the 2017-09-remove-duplicate-destination-decoding branch from 6bc6e24 to 86e6dd4 Compare September 6, 2017 22:48
@promag
Copy link
Member Author

promag commented Sep 6, 2017 •
edited

As per @laanwj suggestion on IRC, added a commit to remove unused methods that caused compile warnings:

base58.cpp:308:6: warning: ‘bool {anonymous}::CBitcoinAddress::IsScript() const’ defined but not used [-Wunused-function]
base58.cpp:298:6: warning: ‘bool {anonymous}::CBitcoinAddress::GetKeyID(CKeyID&) const’ defined but not used [-Wunused-function]

Also rebased.

@sipa
Copy link
Member

sipa commented Sep 6, 2017

utACK 8d0041e

@laanwj
Copy link
Member

laanwj commented Sep 6, 2017

Thanks. re-utACK 86e6dd4, builds without warnings (except in leveldb, which we can't do anything about here) now.

@laanwj laanwj merged commit 86e6dd4 into bitcoin:master Sep 6, 2017
laanwj added a commit that referenced this pull request Sep 6, 2017
@laanwj
Merge #11259: Remove duplicate destination decoding
089b742 
86e6dd4 Remove duplicate destination decoding (João Barbosa)
8d0041e Remove unused GetKeyID and IsScript methods from CBitcoinAddress (João Barbosa)

Pull request description:

  Follow up of #11117, this patch removes an extra unnecessary destination decoding that was identified while reviewing #11117. It is also the only case where `IsValidDestinationString` is called before `DecodeDestination`.

  For reference see [comment](#11117 (comment)).

Tree-SHA512: f5ff5cb28a576ccd819a058f102188bde55654f30618520cc680c91d2f6e536fe038fc7220dd2d2dd64c6175fcb23f89b94b48444521e11ddec0b2f8ef2c05dd
cculianu pushed a commit to cculianu/bitcoin-abc that referenced this pull request Sep 27, 2017
@promag @dagurval
Remove duplicate destination decoding
3648e1b 
Summary:
Port of bitcoin/bitcoin#11259

Depends on D540

Test Plan: None

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D541
@dagurval dagurval mentioned this pull request Jan 19, 2018
Merged
This was referenced Apr 20, 2018
Closed
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Apr 23, 2018
@zkbot
Auto merge of #3202 - str4d:3058-sapling-bech32, r=<try>
2c1f65d 
Bech32 encoding support and t-addr encoding refactor

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`
- bitcoin/bitcoin#11117
- bitcoin/bitcoin#11259
- bitcoin/bitcoin#11167
  - Only the first three commits (the fourth commit depends on #2390, later ones are SegWit-specific).

Part of #3058.
@str4d str4d mentioned this pull request May 1, 2018
Merged
zkbot added a commit to zcash/zcash that referenced this pull request May 4, 2018
@zkbot
Auto merge of #3228 - str4d:3058-taddr-encoding-refactor, r=str4d
d97bfb7 
Refactor t-address encoding

Includes code cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#11117
- bitcoin/bitcoin#11259
  - Only the second commit (first is for QT code)
- bitcoin/bitcoin#11167
  - Only the first commit (the rest are not part of the t-address encoding refactor).

Part of #3058. Precursor to #3202.
mkjekk pushed a commit to mkjekk/zcash that referenced this pull request Aug 12, 2018
@zkbot @mkjekk
Auto merge of zcash#3228 - str4d:3058-taddr-encoding-refactor, r=str4d
78344bb 
Refactor t-address encoding

Includes code cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#11117
- bitcoin/bitcoin#11259
  - Only the second commit (first is for QT code)
- bitcoin/bitcoin#11167
  - Only the first commit (the rest are not part of the t-address
encoding refactor).

Part of zcash#3058. Precursor to zcash#3202.
mkjekk added a commit to mkjekk/zcash that referenced this pull request Aug 12, 2018
@mkjekk
Auto merge of zcash#3228 - str4d:3058-taddr-encoding-refactor, r=str4d
cadb67b 
Refactor t-address encoding

Includes code cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#11117
- bitcoin/bitcoin#11259
  - Only the second commit (first is for QT code)
- bitcoin/bitcoin#11167
  - Only the first commit (the rest are not part of the t-address
encoding refactor).

Part of zcash#3058. Precursor to zcash#3202.
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 12, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11259: Remove duplicate destination decoding
1e7d9e7 
86e6dd4 Remove duplicate destination decoding (João Barbosa)
8d0041e Remove unused GetKeyID and IsScript methods from CBitcoinAddress (João Barbosa)

Pull request description:

  Follow up of bitcoin#11117, this patch removes an extra unnecessary destination decoding that was identified while reviewing bitcoin#11117. It is also the only case where `IsValidDestinationString` is called before `DecodeDestination`.

  For reference see [comment](bitcoin#11117 (comment)).

Tree-SHA512: f5ff5cb28a576ccd819a058f102188bde55654f30618520cc680c91d2f6e536fe038fc7220dd2d2dd64c6175fcb23f89b94b48444521e11ddec0b2f8ef2c05dd
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11259: Remove duplicate destination decoding
bf71677 
86e6dd4 Remove duplicate destination decoding (João Barbosa)
8d0041e Remove unused GetKeyID and IsScript methods from CBitcoinAddress (João Barbosa)

Pull request description:

  Follow up of bitcoin#11117, this patch removes an extra unnecessary destination decoding that was identified while reviewing bitcoin#11117. It is also the only case where `IsValidDestinationString` is called before `DecodeDestination`.

  For reference see [comment](bitcoin#11117 (comment)).

Tree-SHA512: f5ff5cb28a576ccd819a058f102188bde55654f30618520cc680c91d2f6e536fe038fc7220dd2d2dd64c6175fcb23f89b94b48444521e11ddec0b2f8ef2c05dd
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 29, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11259: Remove duplicate destination decoding
a321e61 
86e6dd4 Remove duplicate destination decoding (João Barbosa)
8d0041e Remove unused GetKeyID and IsScript methods from CBitcoinAddress (João Barbosa)

Pull request description:

  Follow up of bitcoin#11117, this patch removes an extra unnecessary destination decoding that was identified while reviewing bitcoin#11117. It is also the only case where `IsValidDestinationString` is called before `DecodeDestination`.

  For reference see [comment](bitcoin#11117 (comment)).

Tree-SHA512: f5ff5cb28a576ccd819a058f102188bde55654f30618520cc680c91d2f6e536fe038fc7220dd2d2dd64c6175fcb23f89b94b48444521e11ddec0b2f8ef2c05dd
@furszy furszy mentioned this pull request Jun 12, 2020
Merged
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Jul 5, 2020
@random-zebra
Merge #1679: Prepare for non-Base58 addresses [Step 3]
c157deb 
e07286d Remove possibly confusing IsValidDestinationString(str) (furszy)
a7bafa1 Implement {Encode,Decode}Destination without CBitcoinAddress (furszy)
7d3ba5d Remove unused GetKeyID and IsScript methods from CBitcoinAddress (furszy)
e7c5d9a Move CBitcoinAddress to base58.cpp (furszy)
548f828 Final migration from CBitcoinAddress to the destionation wrapper (furszy)

Pull request description:

  Finished the complete removal of the base58 address class from the sources, migrated to the destination wrapper.

  Plus includes bitcoin#11117 - last commit - and bitcoin#11259 as an extra.

ACKs for top commit:
  random-zebra:
    utACK e07286d
  Fuzzbawls:
    utACK e07286d

Tree-SHA512: b2bf4c5ed10f863f0de7ab57a87610fddab1bbe21a1f585006e5c370ba3c04635fb1314a9d9a80b2c15a66d5bfe34fd9590dc113f0d91d93d33eb37344c7f8fe
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@promag @laanwj @sipa @meshcollider