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.

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

[GUI] Remove BIP70 #2286

Merged
merged 10 commits into from
Apr 18, 2021
Merged

[GUI] Remove BIP70 #2286

merged 10 commits into from
Apr 18, 2021

Conversation

Fuzzbawls
Copy link
Collaborator

Following with upstream's bitcoin#17165, this removes the support for the problematic BIP70, which we weren't fully supporting anyways.

This removes the protobuf dependency completely, as well as removes the GUI's dependency on libssl

@Fuzzbawls Fuzzbawls added this to the 6.0.0 milestone Apr 2, 2021
@Fuzzbawls Fuzzbawls self-assigned this Apr 2, 2021
random-zebra
random-zebra previously approved these changes Apr 12, 2021
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

Tested local build / depends / gitian. ACK 84771b0

@random-zebra
Copy link

Needs rebase

Fuzzbawls and others added 10 commits April 16, 2021 16:45
This removes the support for BIP 70 payment
requests. Protobuf is no longer required by any binary target, and
OpenSSL is also no longer needed for the GUI target (when building with
GMP support).
This was originally added in bitcoin#9366 to fix the gui build, as
Protobuf would also define these macros. Now that we're no-longer
using Protobuf, remove the additional check.
in comments, rpc output, and the REST doc
@Fuzzbawls
Copy link
Collaborator Author

rebased

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

utACK 6e25d83 after rebase

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

build tested, ACK 6e25d83 and merging..

@furszy furszy merged commit e980786 into PIVX-Project:master Apr 18, 2021
furszy added a commit that referenced this pull request May 12, 2021
5563331 Snap: remove openssl from nightly snapcraft build requirements (Fuzzbawls)
686bfad doc: Add OpenSSL removal to release notes (Fuzzbawls)
f669248 ci: remove OpenSSL installation (Fuzzbawls)
9660aec doc: remove OpenSSL from build instructions and licensing info (Fuzzbawls)
9b2e35d depends: remove OpenSSL package (Fuzzbawls)
9a81d8e CMake: remove OpenSSL detection and libs (Fuzzbawls)
53576bc build: remove OpenSSL detection and libs (fanquake)
5f30c2b Stop using OpenSSL's sha hashing in bip38 code (Fuzzbawls)
d531bf2 Use our own hmac_sha256 instead of OpenSSL's in scrypt.cpp (Fuzzbawls)
b687f8e Use ctaes instead of OpenSSL's AES in bip38 code (Fuzzbawls)
86c978a Remove unused openssl includes (Fuzzbawls)
ab830e5 remove unused EncodeBase64Secure (Fuzzbawls)
690c938 random: Remove remaining OpenSSL calls and locking infrastructure (fanquake)
602c0b2 random: stop retrieving random bytes from OpenSSL (fanquake)
b1c8396 random: stop feeding RNG output back into OpenSSL (fanquake)

Pull request description:

  The natural follow-up to #2278, #2286, and #2288. With these three PRs merged, there are only a few minor pieces of code that still rely on OpenSSL:

  - a call to `RAND_bytes` during the ::SLOW path of ProcRand
  - feeding output from our RNG back into OpenSSL via `RAND_add` during the ::SLOW and ::SLEEP paths.
  - an unused function in `utilstrencodings.cpp` (`DecodeBase64Secure()`, now removed)
  - some stale (un-needed/un-used) header includes
  - bip38 exclusive usages including the following:
    - using OpenSSL's AES for encryption, now switched to using ctaes
    - using OpenSSL to do HMAC_SHA256 hashing in `crypto/scrypt.cpp`, now switched to using our native HMAC_SHA256 header
    - an unused function in `hash.h` (`std::string Hash(std::string input)`), now removed
    - a SHA256 Hash function to compute a void pointer, switched to using template objects

  Upstream PRs backported: bitcoin#17265, bitcoin#17515, and bitcoin#18825

  The changes to bip38 were tested by doing two-way encryption/decryption between `master` and this PR

ACKs for top commit:
  random-zebra:
    ACK 5563331
  furszy:
    k, ACK 5563331 and merging..

Tree-SHA512: bfa7445d7b153bb5ea04b7b52bbedaa07ad5acd1a56221425fa5fb7c20ecbf90f392c85273734ad2a277d4fffc43b10a7a660924a8a41c175ba2fc68e6cf820f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants