pubkey: Assert CPubKey's ECCVerifyHandle precondition #17275

Assert CPubKey's ECCVerifyHandle precondition.

This makes it more clear for fuzzing harness writers and others that ECCVerifyHandle is expected to be held when interacting with CPubKey.

Related PR #17274.

Concept ACK, I think the "Users of this module must hold an ECCVerifyHandle." message is unclear, doesn't provide context. what about a more direct "secp256k1_context_verify must be initialized to use CPubKey"

@laanwj

I've now changed to the suggested wording.

The old wording was taken from here:

https://github.com/bitcoin/bitcoin/blob/773026044f9d87462de4809fbb339e42c52d0a76/src/pubkey.h#L227-L227

Please re-review :)

@fanquake No longer waiting for author? :)

@laanwj Would you mind reviewing? :)

I think it would be good to have these assertions in.

Without these it is possible for someone to mess up and have a NULL secp256k1_context_verify passed to the secp256k1-functions (by forgetting the ECCVerifyHandle pre-conditiion).

Consider the following example:

bool CPubKey::IsFullyValid() const {
    if (!IsValid())
        return false;
    secp256k1_pubkey pubkey;
    return secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, vch, size());
}

That function will return also in cases where secp256k1_context_verify == nullptr.

I think it would be good to guard against those sharp edges by adding the suggested assertions :)

In this case:

bool CPubKey::IsFullyValid() const {
    if (!IsValid())
        return false;
    secp256k1_pubkey pubkey;
    assert(secp256k1_context_verify && "secp256k1_context_verify must be initialized to use CPubKey.");
    return secp256k1_ec_pubkey_parse(secp256k1_context_verify, &pubkey, vch, size());
}

ACK d8daa8f3711909223b117b8faa82daca87fc942d