configure: Allow users to explicitly enable libsecp256k1's GMP bignum support #20121

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:secp256k1_allow_bignum changing 1 files +1 −1
  1. luke-jr commented at 2:42 PM on October 11, 2020: member

    By moving the --with-bignum=no option to the start of the configure args, users can explicitly override it when configuring Bitcoin Core.

    When this option was originally added in 2015 (7fd5b801ff16d748b5ca13ded09ed5da8eacf7e7 #6210), there was mention of a runtime crash reported by gmaxwell. Do details of that incident exist anywhere? Was it ever resolved? Did it crash at startup? If not, could it be triggered remotely?

  2. DrahtBot added the label Build system on Oct 11, 2020
  3. DrahtBot commented at 8:25 PM on October 11, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

  4. DrahtBot cross-referenced this on Oct 11, 2020 from issue History for Taproot PR #19953 by sipa
  5. DrahtBot cross-referenced this on Oct 11, 2020 from issue Implement BIP 340-342 validation (Schnorr/taproot/tapscript) by sipa
  6. laanwj commented at 8:55 AM on October 14, 2020: member

    The reason for disabling gmp by default is that the releases should not have a dependency on gmp. There are some good reasons to have the default configure options equivalent (or as close as possible) to the release options. Especially for something that affects consensus so directly if there was a bug in this less-tested path.

    Concept ACK on being able to override it on demand though.

  7. fanquake commented at 1:59 AM on October 15, 2020: member

    Can you clarify why this needs to be moved? Using master (661fe5d65cc6516439f9d6e0f1a5e2db0e129059) I can do:

    ./autogen.sh
    ./configure --with-bignum=gmp
    ....
      asm                     = x86_64
      bignum                  = gmp
      ecmult window size      = 15
    

    and secp256k1 is configured with bignum support using gmp. I guess there's somewhere this doesn't currently work? @sipa also mentioned on IRC:

    2020-10-11T16:16:27 <sipa> luke-jr: fwiw, after secp256k1 PR 767 we'll probably remove bignum support entirely

  8. gmaxwell commented at 9:21 AM on October 15, 2020: contributor

    This is going to be removed, so I don't think it makes sense to worry about it.

    I don't know if it would be correct to say GMP to be maintained in a way suitable for consensus consistency: Although GMP testing is extremely good, they would-- for example, quickly fix a bug that slipped through in a minor release where consensus code might want to be handled in a more coordinated manner. (Although the secp256k1 GMP actually double-checks the GMP computation and will runtime assert for an incorrect result with 100% confidence, though if a GMP bug causes memory corruption all bets are off).

    Use of GMP also potentially has licensing implications for users/redistributors as GMP is LGPLv3/GPLv2.

    As far as the crash-- I don't recall and don't have access to old IRC logs, it's extremely likely that it would have been discussed in public. It would have been debian on PPC64 (big endian).

  9. DrahtBot added the label Needs rebase on Oct 15, 2020
  10. luke-jr commented at 6:38 PM on October 24, 2020: member

    @fanquake No idea how you're getting that...

    $ ./configure --with-bignum=gmp
    ...
      bignum                  = no
    
  11. configure: Allow users to explicitly enable libsecp256k1's GMP bignum support a26496f6a7
  12. luke-jr commented at 6:40 PM on October 24, 2020: member

    Rebased

  13. luke-jr force-pushed on Oct 24, 2020
  14. DrahtBot removed the label Needs rebase on Oct 24, 2020
  15. DrahtBot cross-referenced this on Nov 16, 2020 from issue Erlay: bandwidth-efficient transaction relay protocol by naumenkogs
  16. laanwj commented at 10:15 AM on November 19, 2020: member

    This is going to be removed, so I don't think it makes sense to worry about it.

    I think that alone is enough reason to not do this, let alone the other reasons you mention. NACK

  17. luke-jr closed this on Nov 19, 2020

  18. bitcoin locked this on Feb 15, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC