🔒 Amend p256 Test Coverage (#256)

### 🕓 Changelog

In the test function `testVerifyWycheproofData` for the `p256` contract,
we flip the `s` parameter if it is higher than the malleability
threshold to validate the correctness of the underlying implementation
(h/t @cairoeth for raising this point). We also `bound` the fuzzed
private key in the `testFuzzVerifyWithValidSignature` test to account
for the secp256r1 curve specifications. Finally, we refactor (i.e.
remove) the `_SIGNATURE_INCREMENT` constant in the `ecdsa` contract by
using the built-in Vyper constant `max_value(int256)` instead. You can
validate the equivalence of the two constants by using
[`titanoboa`](https://github.com/vyperlang/titanoboa):

```console
~$ pip install git+https://github.com/vyperlang/[email protected]
~$ python
>>> import boa
>>> boa.eval("convert(max_value(int256), uint256)") == 57_896_044_618_658_097_711_785_492_504_343_953_926_634_992_332_820_282_019_728_792_003_956_564_819_967
True
```

---------

Signed-off-by: Pascal Marco Caversaccio <[email protected]>

src/snekmate/utils/ecdsa.vy

@@ -20,9 +20,9 @@
 """
 
 
-# @dev Constants used as part of the ECDSA recovery function.
+# @dev The malleability threshold used as part of the ECDSA
+# verification function.
 _MALLEABILITY_THRESHOLD: constant(uint256) = 57_896_044_618_658_097_711_785_492_504_343_953_926_418_782_139_537_452_191_302_581_570_759_080_747_168
-_SIGNATURE_INCREMENT: constant(uint256) = 57_896_044_618_658_097_711_785_492_504_343_953_926_634_992_332_820_282_019_728_792_003_956_564_819_967
 
 
 @deploy
@@ -102,10 +102,9 @@ def _try_recover_r_vs(hash: bytes32, r: uint256, vs: uint256) -> address:
     @param vs The secp256k1 32-byte short signature field of `v` and `s`.
     @return address The recovered 20-byte signer address.
     """
-    s: uint256 = vs & _SIGNATURE_INCREMENT
-    # We do not check for an overflow here since the shift operation
-    # `vs >> 255` results essentially in a `uint8` type (`0` or `1`) and
-    # we use `uint256` as result type.
+    s: uint256 = vs & convert(max_value(int256), uint256)
+    # We do not check for an overflow here, as the shift operation
+    # `vs >> 255` results in `0` or `1`.
     v: uint256 = unsafe_add(vs >> 255, 27)
     return self._try_recover_vrs(hash, v, r, s)
 

src/snekmate/utils/p256.vy

@@ -27,6 +27,10 @@ _C: constant(uint256) = 32
 # curve database: https://neuromancer.sk/std/secg/secp256r1).
 
 
+# @dev The secp256r1 curve order (number of points).
+_N: constant(uint256) = 115_792_089_210_356_248_762_697_446_949_407_573_529_996_955_224_135_760_342_422_259_061_068_512_044_369
+
+
 # @dev The malleability threshold used as part of the ECDSA
 # verification function.
 _MALLEABILITY_THRESHOLD: constant(uint256) = 57_896_044_605_178_124_381_348_723_474_703_786_764_998_477_612_067_880_171_211_129_530_534_256_022_184
@@ -49,10 +53,6 @@ _GX: constant(uint256) = 48_439_561_293_906_451_759_052_585_252_797_914_202_762_
 _GY: constant(uint256) = 36_134_250_956_749_795_798_585_127_919_587_881_956_611_106_672_985_015_071_877_198_253_568_414_405_109
 
 
-# @dev The secp256r1 curve order (number of points).
-_N: constant(uint256) = 115_792_089_210_356_248_762_697_446_949_407_573_529_996_955_224_135_760_342_422_259_061_068_512_044_369
-
-
 # @dev The "-2 mod _P" constant is used to speed up inversion
 # and doubling (avoid negation).
 _MINUS_2MODP: constant(uint256) = 115_792_089_210_356_248_762_697_446_949_407_573_530_086_143_415_290_314_195_533_631_308_867_097_853_949

test/utils/P256.t.sol

@@ -14,6 +14,8 @@ contract P256Test is Test {
     using BytesLib for bytes;
     using stdJson for string;
 
+    uint256 private constant _N =
+        115_792_089_210_356_248_762_697_446_949_407_573_529_996_955_224_135_760_342_422_259_061_068_512_044_369;
     uint256 private constant _MALLEABILITY_THRESHOLD =
         57_896_044_605_178_124_381_348_723_474_703_786_764_998_477_612_067_880_171_211_129_530_534_256_022_184;
     /* solhint-disable const-name-snakecase */
@@ -110,9 +112,18 @@ contract P256Test is Test {
             uint256 y = uint256(vector.readBytes32(".y"));
             bytes32 hash = vector.readBytes32(".hash");
 
-            if (uint256(s) <= _MALLEABILITY_THRESHOLD) {
+            if (s <= _N) {
                 assertEq(
-                    P256.verify_sig(hash, r, s, x, y),
+                    P256.verify_sig(
+                        hash,
+                        r,
+                        /**
+                         * @dev Flip the `s` parameter if it is higher than the malleability threshold.
+                         */
+                        (s > _MALLEABILITY_THRESHOLD) ? (_N - s) : s,
+                        x,
+                        y
+                    ),
                     vector.readBool(".valid")
                 );
             } else {
@@ -133,6 +144,7 @@ contract P256Test is Test {
         string calldata message
     ) public {
         (, uint256 key) = makeAddrAndKey(signer);
+        key = bound(key, 1, _N - 1);
         bytes32 hash = keccak256(abi.encode(message));
         (bytes32 r, bytes32 s) = vm.signP256(key, hash);
         (uint256 qx, uint256 qy) = FCL_ecdsa_utils.ecdsa_derivKpub(key);