BadPractise: assert may bypass function calls when NDEBUG is defined #3168

key.cpp:

151 void SetSecretBytes(const unsigned char vch[32]) { 152 BIGNUM bn; 153 BN_init(&bn); 154 assert(BN_bin2bn(vch, 32, &bn)); 155 assert(EC_KEY_regenerate_key(pkey, &bn)); 156 BN_clear_free(&bn); 157 }

This is a known issue and it exists in multiple places.

For fatal errors that are impossible to handle normally (for example because it implies some internal state was corrupted) we should add a sanity check function that cannot be disabled instead of asserts.

This seems to be rather important then...

We don't support building with NDEBUG at all at the moment.

I think we could use a

bool fAssertsEnabled = false;
assert((fAssertsEnabled = true));
if (!fAssertsEnabled)
   exit in some fatal way;

A quick grep for assert shows that we're no longer doing anything with (obvious) side-effects in an assert anymore. So I'm closing this.

I'd still recommend keeping assertions enabled because a lot of important conditions are checked using assert. You'd reduce security and robustness significantly by disabling them.