From c723b852852b0daca2c4045fb01a9c5457bc2aa7 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 4 Aug 2023 01:16:58 +0200 Subject: [PATCH] redefine VERIFY_CHECK to empty in production (non-VERIFY) mode As suggested in issue #1381, this will make things simpler and improve code readability, as we don't need to force omitting of evaluations on a case-by-case basis anymore and hence can remove lots of `#ifdef VERIFY`/`#endif` lines (see next commit). Plus, VERIFY_CHECK behaves now identical in both non-VERIFY and coverage mode, making the latter not special anymore and hopefully decreasing maintenance burden. The idea of "side-effect safety" is given up. Note that at two places in the ellswift module void-casts of return values have to be inserted for non-VERIFY builds, in order to avoid "variable ... set but not used [-Wunused-but-set-variable]" warnings. --- src/modules/ellswift/main_impl.h | 8 ++++++++ src/util.h | 9 +++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/modules/ellswift/main_impl.h b/src/modules/ellswift/main_impl.h index 00bb8a3da5..d7bcf8577d 100644 --- a/src/modules/ellswift/main_impl.h +++ b/src/modules/ellswift/main_impl.h @@ -272,7 +272,11 @@ static int secp256k1_ellswift_xswiftec_inv_var(secp256k1_fe *t, const secp256k1_ secp256k1_fe_negate(&q, &q, 1); /* q = -s*(4*(u^3+7)+3*u^2*s) */ if (!secp256k1_fe_is_square_var(&q)) return 0; ret = secp256k1_fe_sqrt(&r, &q); /* r = sqrt(-s*(4*(u^3+7)+3*u^2*s)) */ +#ifdef VERIFY VERIFY_CHECK(ret); +#else + (void)ret; +#endif /* If (c & 1) = 1 and r = 0, fail. */ if (EXPECT((c & 1) && secp256k1_fe_normalizes_to_zero_var(&r), 0)) return 0; @@ -417,7 +421,11 @@ int secp256k1_ellswift_encode(const secp256k1_context *ctx, unsigned char *ell64 * BIP340 tagged hash with tag "secp256k1_ellswift_encode". */ secp256k1_ellswift_sha256_init_encode(&hash); ser_ret = secp256k1_eckey_pubkey_serialize(&p, p64, &ser_size, 1); +#ifdef VERIFY VERIFY_CHECK(ser_ret && ser_size == 33); +#else + (void)ser_ret; +#endif secp256k1_sha256_write(&hash, p64, sizeof(p64)); secp256k1_sha256_write(&hash, rnd32, 32); diff --git a/src/util.h b/src/util.h index cf7e5d1af5..1f1f92c99b 100644 --- a/src/util.h +++ b/src/util.h @@ -132,15 +132,12 @@ static const secp256k1_callback default_error_callback = { } while(0) #endif -/* Like assert(), but when VERIFY is defined, and side-effect safe. */ -#if defined(COVERAGE) -#define VERIFY_CHECK(check) -#define VERIFY_SETUP(stmt) -#elif defined(VERIFY) +/* Like assert(), but when VERIFY is defined. */ +#if defined(VERIFY) #define VERIFY_CHECK CHECK #define VERIFY_SETUP(stmt) do { stmt; } while(0) #else -#define VERIFY_CHECK(cond) do { (void)(cond); } while(0) +#define VERIFY_CHECK(cond) #define VERIFY_SETUP(stmt) #endif