99bd335599 Make int128 overflow test use secp256k1_[ui]128_mul (Pieter Wuille)
3afce0af7c Avoid signed overflow in MSVC AMR64 secp256k1_mul128 (Pieter Wuille)
9b5f589d30 Heuristically decide whether to use int128_struct (Pieter Wuille)
63ff064d2f int128: Add test override for testing __(u)mulh on MSVC X64 (Tim Ruffing)
f2b7e88768 Add int128 randomized tests (Pieter Wuille)
Pull request description:
This is a follow-up to #1000:
* Add randomized unit tests for int128 logic.
* Add CI for the `_(u)mulh` code path (on non-ARM64 MSVC).
* Add heuristic logic to enable int128_struct based arithmetic on 64-bit MSVC, or systems with pointers wider than 32 bits.
* Fix signed overflow in ARM64 MSVC code.
ACKs for top commit:
roconnor-blockstream:
utACK 99bd335
real-or-random:
ACK 99bd335599 tested this also on MSVC locally with the override, including all the benchmark binaries
jonasnick:
utACK 99bd335599
Tree-SHA512: 5ea897362293b45a86650593e1fdc8c4004a1d9452eed2fa070d22dffc7ed7ca1ec50a4df61e3a33dbe35e08132ad9686286ac44af6742b32b82f11c9d3341c6
a340d9500a ci: add int128_struct tests (Jonas Nick)
dceaa1f579 int128: Tidy #includes of int128.h and int128_impl.h (Tim Ruffing)
2914bccbc0 Simulated int128 type. (Russell O'Connor)
Pull request description:
Abstracts the int128 type and provides an native version, if available, or a implements it using a pair of int64_t's.
This is activated by setting the configuration flag `--with-test-override-wide-multiply=int128_struct`.
The primary purpose of this PR is to take advantage of MSVC's [umulh](https://docs.microsoft.com/en-us/cpp/intrinsics/umulh?view=msvc-170) intrinsic that we can use to simulate an int128 type which MSVC does not have (AFAIU). This PR lays out the groundwork for this level of MSVC support, but doesn't include the configuration logic to enable it yet.
For completeness, and implementation of `umulh` and `mulh` are also provided for compilers that support neither the intrinsic nor the int128 type (such as CompCert?). This also opens up the possibility of removing the 32-bit field and scalar implementations should that ever be desired.
ACKs for top commit:
sipa:
ACK a340d9500a
jonasnick:
ACK a340d9500a
Tree-SHA512: b4f2853fa3ab60ce9d77b4eaee1fd20c4b612850e19fcb3179d7e36986f420c6c4589ff72f0cf844f989584ace49a1cd23cca3f4e405dabefc8da647a0df679d
6a965b6b98 Remove usage of CHECK from non-test file (Tobin C. Harding)
Pull request description:
Currently CHECK is used only in test and bench mark files except for one usage in `ecmult_impl.h`.
We would like to move the definition of CHECK out of `util.h` so that `util.h` no longer has a hard dependency on `stdio.h`.
Done as part of an effort to allow secp256k1 to be compiled to WASM as part of `rust-secp256k1`.
### Note to reviewers
Please review carefully, I don't actually know if this patch is correct. Done while working on #1095. I'm happy to make any changes both in concept and execution - I'm super rusty at C programming.
cc real-or-random
ACKs for top commit:
sipa:
utACK 6a965b6b98
real-or-random:
utACK 6a965b6b98
Tree-SHA512: 6bfb456bdb92a831acd3bc202607e80f6d0a194d6b2cf745c8eceb12ba675d03a319d6d105332b0cbca474e443969295e5a8e938635453e21e057d0ee597440b
After this commit, int128.h and int128_impl.h are included as follows:
- .c files which use int128 include int128_impl.h (after util.h)
- .h files which use int128 include int128.h (after util.h)
This list is exhaustive. util.h needs to included first because it sets
up necessary #defines.
Currently CHECK is used only in test and bench mark files except for one
usage in `ecmult_impl.h`.
We would like to move the definition of CHECK out of `util.h` so that
`util.h` no longer has a hard dependency on `stdio.h`.
Done in preparation for moving the definition of `CHECK` as part of an
effort to allow secp256k1 to be compiled to WASM as part of
`rust-secp256k1`.
Provides a method that will give an upper bound on the size of a rangeproof,
given an upper bound on the value to be passed in and an upper bound on the
min_bits parameter.
There is a lot of design freedom here since the actual size of the rangeproof
depends on every parameter passed to rangeproof_sign, including the value to
be proven, often in quite intricate ways. For the sake of simplicity we assume
a nonzero `min_value` and that `exp` will be 0 (the default, and size-maximizing,
choice), and provide an exact value for a proof of the given value and min_bits.
5ac8fb035e surjectionproof: make sure that n_used_pubkeys > 0 in generate (Jonas Nick)
Pull request description:
ACKs for top commit:
apoelstra:
utACK 5ac8fb035e
Tree-SHA512: 915f7181e69e2c4e1f830d6c2620a2d9b0af4d2ae8a63709b489b01ed9e13ccfeeaedebd4680cf2d927cd473a6ae88602cf29e2fdd116cb597fba6c0ab77720d
If the proof was generated with surjectionproof_initialize (as mandated by the
API docs), then n_used_pubkeys can never be 0. Without this commit, compilers
will (rightfully) warn that borromean_s[ring_input_index] is not initialized in
surjectionproof_generate. Therefore, this commit makes sure that n_used_pubkeys
is greater than 0 which ensures that the array is initialized at
ring_input_index.
5a40f3d99b replace memcmp with secp256k1_memcmp_var throughout the codebase (Andrew Poelstra)
92820d944b rangeproof: add a test for all-zero blinding factors (Andrew Poelstra)
Pull request description:
I was curious about under what conditions you can create a rangeproof on an "unblinded" commitment which has a zero blinding factor. Apparently the answer is "when you are proving at least 3-bits". In this case rewinding words and you can encode 32 bytes of data. (In fact I believe you can encode up to 128 but I haven't tested that.)
ACKs for top commit:
real-or-random:
utACK 5a40f3d99b
Tree-SHA512: bed7f9362d082d2b56668809077d5ddde52280109c992a290d87b55cb70138a08799fcca18cafbb3b3e9efed4349418bf9bb2c0ccedacdce0567e841e6d21e13
347f96d94a fix include paths in all the -zkp modules (Andrew Poelstra)
Pull request description:
This is causing out-of-tree build failures in Elements.
ACKs for top commit:
real-or-random:
utACK 347f96d94a
Tree-SHA512: 7d6211f3b8d5612f95bcb3085c22458e7ceaa79f1ee74e37404cc6d1fdf0fbc02b4443b02623b9b6c1225437c1a1954b6d36a953d52b020ac7913326404894e0
We don't enable the ECDSA recovery module, because we don't recommend
ECDSA recovery for new protocols. In particular, the recovery API is
prone to misuse: It invites the caller to forget to check the public
key (and the verification function always returns 1).
In general, we also don't recommend ordinary ECDSA for new protocols.
But disabling the ECDSA functions is not possible because they're not
in a module, and let's be honest: disabling ECDSA would mean to ignore
reality blatantly.
