Skip to content

Commit

Permalink
Merge #1378: ellswift: fix probabilistic test failure when swapping s…
Browse files Browse the repository at this point in the history
…ides

c424e2f ellswift: fix probabilistic test failure when swapping sides (Jonas Nick)

Pull request description:

  Reported by jonatack in bitcoin/bitcoin#28079.

  When configured with `--disable-module-ecdh --enable-module-recovery`, then `./tests  64 81af32fd7ab8c9cbc2e62a689f642106` fails with
  ```
  src/modules/ellswift/tests_impl.h:396: test condition failed: secp256k1_memcmp_var(share32_bad, share32a, 32) != 0
  ```

  This tests verifies that changing the `party` bit of the `secp256k1_ellswift_xdh` function results in a different share. However, that's not the case when the secret keys of both parties are the same and this is actually what happens in the observed test failure. The keys can be equal in this test case because they are created by the `random_scalar_order_test` function whose output is not uniformly random (it's biased towards 0).

  This commit restores the assumption that the secret keys differ.

ACKs for top commit:
  sipa:
    utACK c424e2f
  real-or-random:
    utACK c424e2f

Tree-SHA512: d1ab61473a77478f9aeffb21ad73e0bba478c90d8573c72ec89d2e0140434cc65c9d5f4d56e5f259931dc68fc1800695c6cd5d63d9cfce4c1c4d6744eeaa2028
  • Loading branch information
real-or-random committed Jul 17, 2023
2 parents 907a672 + c424e2f commit b40e2d3
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/modules/ellswift/tests_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,9 @@ void run_ellswift_tests(void) {
secp256k1_testrand256_test(auxrnd32a);
secp256k1_testrand256_test(auxrnd32b);
random_scalar_order_test(&seca);
random_scalar_order_test(&secb);
/* Draw secb uniformly at random to make sure that the secret keys
* differ */
random_scalar_order(&secb);
secp256k1_scalar_get_b32(sec32a, &seca);
secp256k1_scalar_get_b32(sec32b, &secb);

Expand Down

0 comments on commit b40e2d3

Please sign in to comment.