-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Open
Description
At the moment, the only test is this one:
Lines 2262 to 2267 in 1605b02
| /* Does check_overflow check catch all ones? */ | |
| static const secp256k1_scalar overflowed = SECP256K1_SCALAR_CONST( | |
| 0xFFFFFFFFUL, 0xFFFFFFFFUL, 0xFFFFFFFFUL, 0xFFFFFFFFUL, | |
| 0xFFFFFFFFUL, 0xFFFFFFFFUL, 0xFFFFFFFFUL, 0xFFFFFFFFUL | |
| ); | |
| CHECK(secp256k1_scalar_check_overflow(&overflowed)); |
That is, even a blank return 1 would pass the tests.
Some calls to secp256k1_scalar_check_overflow in the tests were removed in #1484 because I deemed them redundant. But that's not entirely true. Their primary purpose was to check for overflows in other test code, and this is indeed redundant. But a secondary effect of these calls was to test secp256k1_scalar_check_overflow itself.
We could revert this, but I think it's better to have a bunch of dedicated tests:
- Check a handful of static inputs including the edge cases.
- Check some random inputs. Uniformly random inputs should not overflow. But if we tweak in the right way (e.g., setting top bits), they should overflow.
theStack