configure: Allow users to explicitly enable libsecp256k1's GMP bignum support #20121
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
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. |
|
Can you clarify why this needs to be moved? Using master (661fe5d) I can do: ./autogen.sh
./configure --with-bignum=gmp
....
asm = x86_64
bignum = gmp
ecmult window size = 15and secp256k1 is configured with bignum support using gmp. I guess there's somewhere this doesn't currently work? @sipa also mentioned on IRC:
|
|
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). |
|
@fanquake No idea how you're getting that... |
|
Rebased |
289f56e to
a26496f
Compare
I think that alone is enough reason to not do this, let alone the other reasons you mention. |
By moving the
--with-bignum=nooption to the start of the configure args, users can explicitly override it when configuring Bitcoin Core.When this option was originally added in 2015 (7fd5b80 #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?