Zebra: Check that C++ script implementation is threadsafe.

Created on 11 Aug 2020  路  4Comments  路  Source: ZcashFoundation/zebra

We should check that the C++ script implementation (appears to be) threadsafe, in order to be able to do parallel script verification (#868).

All 4 comments

starting this now. cc @str4d @gtank incase either of you have any suggestions for things to double check.

So far I've checked the main logic, mostly in interpreter.cpp. That code seems to have no shared data beyond static consts which should be safe to share. Same goes for the calling logic in zcashconsensus.cpp/h. I'm going to go ahead and double check all of the .cpp files that are needed by the build.rs file in zcash_script.

pubkey.cpp looks like its likely not threadsafe.

namespace
{
/* Global secp256k1_context object used for verification. */
secp256k1_context* secp256k1_context_verify = NULL;
}

Shared global verification context that is mutated as far as I can tell X_X

Okay so upon further review it looks like it is thread safe, here's how:

So the secp context pointer is managed by this ECCVerifyHandle type that makes it act like a lazy_static:

/* static */ int ECCVerifyHandle::refcount = 0;

ECCVerifyHandle::ECCVerifyHandle()
{
    if (refcount == 0) {
        assert(secp256k1_context_verify == NULL);
        secp256k1_context_verify = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
        assert(secp256k1_context_verify != NULL);
    }
    refcount++;
}

ECCVerifyHandle::~ECCVerifyHandle()
{
    refcount--;
    if (refcount == 0) {
        assert(secp256k1_context_verify != NULL);
        secp256k1_context_destroy(secp256k1_context_verify);
        secp256k1_context_verify = NULL;
    }
}

This handle type is only ever constructed in one place, within zcash_script.cpp

struct ECCryptoClosure
{
    ECCVerifyHandle handle;
};

ECCryptoClosure instance_of_eccryptoclosure;

This is a global variable that creates one of those EccVerifyHandles upon startup of the binary, which initializes the secp context pointer. This is the last time that global pointer is ever mutated, all usages of this context by zcash_script_verify are read only, which should be safe from multiple threads concurrently.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dconnolly picture dconnolly  路  3Comments

nathan-at-least picture nathan-at-least  路  4Comments

yaahc picture yaahc  路  3Comments

dconnolly picture dconnolly  路  5Comments

teor2345 picture teor2345  路  4Comments