Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.10.0 test generators use incorrect domain for deposits #1582

Closed
benjaminion opened this issue Jan 21, 2020 · 1 comment
Closed

v0.10.0 test generators use incorrect domain for deposits #1582

benjaminion opened this issue Jan 21, 2020 · 1 comment

Comments

@benjaminion
Copy link
Contributor

Consider test genesis/initialization, although this afflicts other tests as well. This uses minimal config to generate a load of signed deposits.

In minimal config GENESIS_FORK_VERSION is set to 0x00000001. The test generator correctly generates state with this fork information:

fork: {previous_version: '0x00000001', current_version: '0x00000001', epoch: 0}

However, all of the deposits are signed with GENESIS_FORK_VERSION = 0x00000000.

This can be seen by printing some intermediate values in sign_deposit_data():

def sign_deposit_data(spec, deposit_data, privkey):
    deposit_message = spec.DepositMessage(
        pubkey=deposit_data.pubkey,
        withdrawal_credentials=deposit_data.withdrawal_credentials,
        amount=deposit_data.amount)
    domain = spec.compute_domain(spec.DOMAIN_DEPOSIT)
    signing_root = spec.compute_signing_root(deposit_message, domain)
    deposit_data.signature = bls.Sign(privkey, signing_root)
    print("********")
    print(domain.hex())
    print(spec.GENESIS_FORK_VERSION.hex())
    print("********")

This prints

********
0300000000000000
00000001
********

domain ought to be 0x0300000000000001 here. I suspect the default value in compute_domain() is not being correctly handled after the spec constants are overwritten by loading the minimal configuration.

def compute_domain(domain_type: DomainType, fork_version: Version=GENESIS_FORK_VERSION) -> Domain:
    """
    Return the domain for the ``domain_type`` and ``fork_version``.
    """
    return Domain(domain_type + fork_version)
@benjaminion benjaminion mentioned this issue Jan 21, 2020
6 tasks
@djrtwo
Copy link
Contributor

djrtwo commented Jan 21, 2020

Ah, looks like it. Thanks for doing the investigative work.

Will move the setting of fork_version when nothing is passed in into the function

def compute_domain(domain_type: DomainType, fork_version: Version=None) -> Domain:
    """
    Return the domain for the ``domain_type`` and ``fork_version``.
    """
    if fork_version is None:
        fork_version = GENESIS_FORK_VERSION
    return Domain(domain_type + fork_version)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants