-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Merge #11117: Prepare for non-Base58 addresses #3294
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.
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
Conversation
864cd27 Move CBitcoinAddress to base58.cpp (Pieter Wuille) 5c8ff0d Introduce wrappers around CBitcoinAddress (Pieter Wuille) Pull request description: This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`. As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions. This change is far from complete. In follow-ups I'd like to: * Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so. * Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere). * Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`. However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction. Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e
Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
…sScript()" Signed-off-by: Pasta <pasta@dashboost.org>
…ript()" Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
Signed-off-by: Pasta <pasta@dashboost.org>
Okay, I've resolved all of the errors I've been able to, I'm not sure what to do about |
Pls see 80507fc |
cherry-picked |
Might need to push it again, looks like your previous attempt failed or smth - I don't see 80507fc in your branch. |
I only said cherry-picked, I never said I pushed it 😂 |
FYI: test failure on Gitlab is unrelated and should be fixed by #3296 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍, just nit: 3eb825c
No need to have it in base58.cpp anymore as this is only used in getAddressesFromParams
cherry-picked and pushed this time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
* Merge bitcoin#11117: Prepare for non-Base58 addresses 864cd27 Move CBitcoinAddress to base58.cpp (Pieter Wuille) 5c8ff0d Introduce wrappers around CBitcoinAddress (Pieter Wuille) Pull request description: This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`. As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions. This change is far from complete. In follow-ups I'd like to: * Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so. * Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere). * Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`. However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction. Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e * CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <pasta@dashboost.org> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <pasta@dashboost.org> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <pasta@dashboost.org> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <pasta@dashboost.org> * fix CBitcoinAddress GetKeyID check Signed-off-by: Pasta <pasta@dashboost.org> * fix providertx.cpp Signed-off-by: Pasta <pasta@dashboost.org> * hopefully fix governance-classes.cpp Signed-off-by: Pasta <pasta@dashboost.org> * partially fix governance-validators.cpp, unable to resolve "address.IsScript()" Signed-off-by: Pasta <pasta@dashboost.org> * partially fix governance-classes.cpp, unable to resolve "address.IsScript()" Signed-off-by: Pasta <pasta@dashboost.org> * fix governance-classes.h Signed-off-by: Pasta <pasta@dashboost.org> * DecodeTransaction -> DecodeDestination, fix governance-validators.cpp Signed-off-by: Pasta <pasta@dashboost.org> * More fixes for 3294 * Move GetIndexKey into rpc/misc.cpp near getAddressesFromParams No need to have it in base58.cpp anymore as this is only used in getAddressesFromParams Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com> Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com> Co-authored-by: Alexander Block <ablock84@gmail.com>
* Merge bitcoin#11117: Prepare for non-Base58 addresses 864cd27 Move CBitcoinAddress to base58.cpp (Pieter Wuille) 5c8ff0d Introduce wrappers around CBitcoinAddress (Pieter Wuille) Pull request description: This patch removes the need for the intermediary Base58 type `CBitcoinAddress`, by providing {`Encode`,`Decode`,`IsValid`}`Destination` functions that directly operate on the conversion between `std::string`s and `CTxDestination`. As a side, it also fixes a number of indentation issues, and removes probably several unnecessary implicit `CTxDestination`<->`CBitcoinAddress` conversions. This change is far from complete. In follow-ups I'd like to: * Split off the specific address and key encoding logic from base58.h, and move it to a address.h or so. * Replace `CTxDestination` with a non-`boost::variant` version (which can be more efficient as `boost::variant` allocates everything on the heap, and remove the need for `boost::get<...>` and `IsValidDestination` calls everywhere). * Do the same for `CBitcoinSecret`, `CBitcoinExtKey`, and `CBitcoinExtPubKey`. However, I've tried to keep this patch to be minimally invasive, but still enough to support non-Base58 addresses. Perhaps a smaller patch is possible to hack Bech32 support into `CBitcoinAddress`, but I would consider that a move in the wrong direction. Tree-SHA512: c2c77ffb57caeadf2429b1c2562ce60e8c7be8aa9f8e51b591f354b6b441162625b2efe14c023a1ae485cf2ed417263afa35c892891dfaa7844e7fbabccab85e * CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <pasta@dashboost.org> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <pasta@dashboost.org> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <pasta@dashboost.org> * more CBitcoinAddress -> EncodeDestination in providertx.h Signed-off-by: Pasta <pasta@dashboost.org> * fix CBitcoinAddress GetKeyID check Signed-off-by: Pasta <pasta@dashboost.org> * fix providertx.cpp Signed-off-by: Pasta <pasta@dashboost.org> * hopefully fix governance-classes.cpp Signed-off-by: Pasta <pasta@dashboost.org> * partially fix governance-validators.cpp, unable to resolve "address.IsScript()" Signed-off-by: Pasta <pasta@dashboost.org> * partially fix governance-classes.cpp, unable to resolve "address.IsScript()" Signed-off-by: Pasta <pasta@dashboost.org> * fix governance-classes.h Signed-off-by: Pasta <pasta@dashboost.org> * DecodeTransaction -> DecodeDestination, fix governance-validators.cpp Signed-off-by: Pasta <pasta@dashboost.org> * More fixes for 3294 * Move GetIndexKey into rpc/misc.cpp near getAddressesFromParams No need to have it in base58.cpp anymore as this is only used in getAddressesFromParams Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com> Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com> Co-authored-by: Alexander Block <ablock84@gmail.com>
Lots of conflicts on this one, so likely so problems. I am also expecting tests to fail