Swappable BLS backend

[ChihChengLiang]

What is wrong?

After #708 we have a fast BLS API based on Chia's implementation. But we might want the freedom to switch between different implementation for different use cases.

For example,

• To try out a new verification scheme, we might still want py_ecc.
• To make the testnet fast, we want Chia's implementation.
• Some state test we just want a dummy verification function that returns True, when the correctness of signature is not a concern.

How can it be fixed

1. Defined an interface and create classes for each BLS implementation (Let's call them backends). As the bottommost sample code shows.
2. There are two possible ways to apply these backends:

Option 1: Pass as an argument to functions that need BLS methods

For example,

def foo(..., bls: BaseBLSBackend):
    ...
    bls.sign(...)

This makes inserting the desired backend to production code and testing code very easy. The downside is the diffs will bubble up to lots of functions and we need to reason that ultimately which class should hold these backends as their property (Chain, StateMachine, or StateTransition?).

Option 2: Import and mock in test

For example,

from eth2._utils.bls import (
    PyECCBackend as bls,
)

def foo(...):
    ...
    bls.sign(...)

This is more like the current status. The pros and cons are opposite to option 1. Need inputs from @pipermerriam @cburgdorf @hwwhww

# eth2/_utils/bls.py
from abc import (
    ABC,
    abstractstaticmethod,
)
from typing import (
    Sequence,
)
from eth_typing import (
    BLSPubkey,
    BLSSignature,
    Hash32,
)
from py_ecc import (
    bls as py_ecc_api,
)
from .bls_bindings.chia_network import (
    api as chia_api,
)


class BaseBLSBackend(ABC):

    @abstractstaticmethod
    def privtopub(k: int) -> BLSPubkey:
        pass

    @abstractstaticmethod
    def sign(message_hash: Hash32,
             privkey: int,
             domain: int) -> BLSSignature:
        pass

    @abstractstaticmethod
    def verify(message_hash: Hash32,
               pubkey: BLSPubkey,
               signature: BLSSignature,
               domain: int) -> bool:
        pass

    @abstractstaticmethod
    def aggregate_signatures(signatures: Sequence[BLSSignature]) -> BLSSignature:
        pass

    @abstractstaticmethod
    def aggregate_pubkeys(pubkeys: Sequence[BLSPubkey]) -> BLSPubkey:
        pass

    @abstractstaticmethod
    def verify_multiple(pubkeys: Sequence[BLSPubkey],
                        message_hashes: Sequence[Hash32],
                        signature: BLSSignature,
                        domain: int) -> bool:
        pass


class PyECCBackend(BaseBLSBackend):
    privtopub = py_ecc_api.privtopub
    sign = py_ecc_api.sign
    verify = py_ecc_api.verify
    aggregate_signatures = py_ecc_api.aggregate_signatures
    aggregate_pubkeys = py_ecc_api.aggregate_pubkeys
    verify_multiple = py_ecc_api.verify_multiple


class ChiaBackend(BaseBLSBackend):
    privtopub = chia_api.privtopub
    sign = chia_api.sign
    verify = chia_api.verify
    aggregate_signatures = chia_api.aggregate_signatures
    aggregate_pubkeys = chia_api.aggregate_pubkeys
    verify_multiple = chia_api.verify_multiple


class TrueBackend(BaseBLSBackend):
    """
    Only for test
    """

    def privtopub(k: int) -> BLSPubkey:
        raise NotImplementedError

    def sign(message_hash: Hash32,
             privkey: int,
             domain: int) -> BLSSignature:
        raise NotImplementedError

    def verify(message_hash: Hash32,
               pubkey: BLSPubkey,
               signature: BLSSignature,
               domain: int) -> bool:
        return True

    def aggregate_signatures(signatures: Sequence[BLSSignature]) -> BLSSignature:
        raise NotImplementedError

    def aggregate_pubkeys(pubkeys: Sequence[BLSPubkey]) -> BLSPubkey:
        raise NotImplementedError

    def verify_multiple(pubkeys: Sequence[BLSPubkey],
                        message_hashes: Sequence[Hash32],
                        signature: BLSSignature,
                        domain: int) -> bool:
        return True

[ChihChengLiang]

Inspired by the pattern used in the spec.

In option 2 we can override the global variable bls with a decorator, which works well even the bls is buried in a deep call.

from eth2._utils.bls import (
    TrueBackend as bls,
    ChiaBackend,
)

MSG = b'\x56' * 32
INVALID_SIG = b'\x56' * 96
INVALID_PUB = b'\x56' * 48
DOMAIN = 5


def decorate_chia(fn):
    def entry(*args, **kwargs):
        fn.__globals__['bls'] = ChiaBackend
        output = fn(*args, **kwargs)
        return output
    return entry


def foo(msg, public_key, signature):
    return bls.verify(msg, public_key, signature, DOMAIN)


@decorate_chia
def foo_decorated(msg, public_key, signature):
    return bls.verify(msg, public_key, signature, DOMAIN)


def child_function_needs_bls(msg, public_key, signature):
    return foo(msg, public_key, signature)


@decorate_chia
def bury_bls_so_deep(msg, public_key, signature):
    return child_function_needs_bls(msg, public_key, signature)


if __name__ == '__main__':
    assert foo(MSG, INVALID_PUB, INVALID_SIG)
    assert not foo_decorated(MSG, INVALID_PUB, INVALID_SIG)
    assert not bury_bls_so_deep(MSG, INVALID_PUB, INVALID_SIG)

[cburgdorf]

In general I prefer Option 1 where we properly defer to a passed-in backend.

Btw @abstractstaticmethod is deprecated. The way you'd do this is to use @staticmethod and @abstractmethod together. However, for the example at hand I would probably make these regular instance methods though.

[pipermerriam]

I would recommend looking at eth-keys.

In fact, it's not out of the question to bake this into the eth-keys library under a namespace like eth_keys.bls381 and we could move the current implementations to live under the appropriate namespace for the old curve.

[ChihChengLiang]

Also quoting here: #808 (review)

I think it probably makes sense to do roughly what was done in eth-keys with something like a eth-bls library with the same backend pattern so that codebases like this don't have to actually care about juggling the dependencies. This approach is probably acceptable for now but long term this is something I think should happen at outside of this codebase.

The way we do this for sepk256k1 is py-evm -> eth-keys -> (py_ecc or coincurve) The same rough structure for eth2 stuff would be py-beacon -> eth-bls -> (py_ecc or blspy). (note the idea about the core eth2 stuff eventually getting pulled to it's own library).

[ralexstokes]

can we close this issue? we have moved ahead w/ a "swappable" backend implementation for the BLS routines... it seems like there is some support to (eventually) move the BLS crypto into eth-keys if i'm reading it correctly.... if we want to keep this, let's move to a new, more precise issue just about that

[ChihChengLiang]

For the moving to eth-keys issue, let's open one when we are ready to to do that. Closing this one.