status-im
/
nimbus-eth2
mirror of https://github.com/status-im/nimbus-eth2.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

add a couple new deposit tests; fix the false-positive BLS verifications while keeping all but two tests working, despite mismatched 0.9/0.10 BLS standards; better-factor the skipping of BLS validation and Merkle tree validation

This commit is contained in:
Dustin Brody 2020-01-30 22:03:47 +01:00 committed by tersec
parent d634eba3fd
commit 1ffc2df23d
16 changed files with 73 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
beacon_chain/beacon_node.nim
View File

@ -1113,7 +1113,7 @@ when isMainModule:
else: waitFor getLatestEth1BlockHash(config.depositWeb3Url)
var
initialState = initialize_beacon_state_from_eth1(
eth1Hash, startTime, deposits, {skipValidation})
eth1Hash, startTime, deposits, {skipValidation, skipMerkleValidation})
# https://github.com/ethereum/eth2.0-pm/tree/6e41fcf383ebeb5125938850d8e9b4e9888389b4/interop/mocked_start#create-genesis-state
initialState.genesis_time = startTime

10
beacon_chain/extras.nim
View File

@ -25,5 +25,15 @@ type
## TODO need to be careful here, easy to assume that slot number change is
## enough, vs advancing the state - however, making a full state copy
## is expensive also :/
skipMerkleValidation ##\
## When processing deposits, skip verifying the Merkle proof trees of each
## deposit. This is a holdover from both interop issues with the malformed
## proofs and, more currently, nim-beacon-chain's creation of proofs which
## are inconsistent with the current specification. Furthermore several of
## the mocking interfaces deliberately do not create Merkle proofs. Whilst
## this seems less than entirely justifiable, for now enable keeping those
## in place while minimizing the tech debt they create. One, in principle,
## should be able to remove this flag entirely. It is not intrinsically an
## expensive operation to perform.
UpdateFlags* = set[UpdateFlag]

9
beacon_chain/spec/beaconstate.nim
View File

@ -53,7 +53,7 @@ proc process_deposit*(
# Process an Eth1 deposit, registering a validator or increasing its balance.
# Verify the Merkle branch
if skipValidation notin flags and not is_valid_merkle_branch(
if skipMerkleValidation notin flags and not is_valid_merkle_branch(
hash_tree_root(deposit.data),
deposit.proof,
DEPOSIT_CONTRACT_TREE_DEPTH + 1,
@ -361,7 +361,8 @@ proc process_registry_updates*(state: var BeaconState) {.nbench.}=
# https://github.com/ethereum/eth2.0-specs/blob/v0.9.4/specs/core/0_beacon-chain.md#is_valid_indexed_attestation
proc is_valid_indexed_attestation*(
state: BeaconState, indexed_attestation: IndexedAttestation): bool =
state: BeaconState, indexed_attestation: IndexedAttestation,
flags: UpdateFlags): bool =
## Check if ``indexed_attestation`` has valid indices and signature.
# TODO: this is noSideEffect besides logging
# https://github.com/status-im/nim-chronicles/issues/62
@ -380,7 +381,7 @@ proc is_valid_indexed_attestation*(
return false
# Verify aggregate signature
if not bls_verify(
if skipValidation notin flags and not bls_verify(
bls_aggregate_pubkeys(mapIt(indices, state.validators[it.int].pubkey)),
hash_tree_root(indexed_attestation.data).data,
indexed_attestation.signature,
@ -497,7 +498,7 @@ proc check_attestation*(
return
if not is_valid_indexed_attestation(
state, get_indexed_attestation(state, attestation, stateCache)):
state, get_indexed_attestation(state, attestation, stateCache), flags):
warn("process_attestation: signature or bitfields incorrect")
return

6
beacon_chain/spec/crypto.nim
View File

@ -160,8 +160,10 @@ func bls_verify*(
return false
# TODO bls_verify_multiple(...) used to have this workaround, and now it
# lives here. No matter the signature, there's also no meaningful way to
# verify it -- it's a kind of vacuous truth. No pubkey/sig pairs.
if pubkey == default(ValidatorPubKey):
# verify it -- it's a kind of vacuous truth. No pubkey/sig pairs. Sans a
# getBytes() or similar mechanism, pubKey == default(ValidatorPubKey) is
# a way to create many false positive matches. This seems odd.
if pubkey.getBytes() == default(ValidatorPubKey).getBytes():
return true
sig.blsValue.verify(msg, domain, pubkey.blsValue)

14
beacon_chain/spec/state_transition_block.nim
View File

@ -217,6 +217,7 @@ func is_slashable_attestation_data(
proc process_attester_slashing*(
state: var BeaconState,
attester_slashing: AttesterSlashing,
flags: UpdateFlags,
stateCache: var StateCache
): bool {.nbench.}=
let
@ -228,11 +229,11 @@ proc process_attester_slashing*(
notice "Attester slashing: surround or double vote check failed"
return false
if not is_valid_indexed_attestation(state, attestation_1):
if not is_valid_indexed_attestation(state, attestation_1, flags):
notice "Attester slashing: invalid attestation 1"
return false
if not is_valid_indexed_attestation(state, attestation_2):
if not is_valid_indexed_attestation(state, attestation_2, flags):
notice "Attester slashing: invalid attestation 2"
return false
@ -259,7 +260,7 @@ proc processAttesterSlashings(state: var BeaconState, blck: BeaconBlock,
return false
for attester_slashing in blck.body.attester_slashings:
if not process_attester_slashing(state, attester_slashing, stateCache):
if not process_attester_slashing(state, attester_slashing, {}, stateCache):
return false
return true
@ -286,13 +287,14 @@ proc processAttestations(
true
# https://github.com/ethereum/eth2.0-specs/blob/v0.8.4/specs/core/0_beacon-chain.md#deposits
proc processDeposits(state: var BeaconState, blck: BeaconBlock): bool {.nbench.}=
proc processDeposits(state: var BeaconState, blck: BeaconBlock,
flags: UpdateFlags): bool {.nbench.}=
if not (len(blck.body.deposits) <= MAX_DEPOSITS):
notice "processDeposits: too many deposits"
return false
for deposit in blck.body.deposits:
if not process_deposit(state, deposit):
if not process_deposit(state, deposit, flags):
notice "processDeposits: deposit invalid"
return false
@ -419,7 +421,7 @@ proc processBlock*(
debug "[Block processing] Attestation processing failure", slot = shortLog(state.slot)
return false
if not processDeposits(state, blck):
if not processDeposits(state, blck, flags):
debug "[Block processing] Deposit processing failure", slot = shortLog(state.slot)
return false

2
research/state_sim.nim
View File

@ -79,7 +79,7 @@ cli do(slots = SLOTS_PER_EPOCH * 6,
let
genesisState =
initialize_beacon_state_from_eth1(
Eth2Digest(), 0, deposits, {skipValidation})
Eth2Digest(), 0, deposits, {skipMerkleValidation})
genesisBlock = get_initial_beacon_block(genesisState)
echo "Starting simulation..."

2
tests/mocking/mock_genesis.nim
View File

@ -25,7 +25,7 @@ proc initGenesisState*(num_validators: uint64, genesis_time: uint64 = 0): Beacon
)
initialize_beacon_state_from_eth1(
eth1BlockHash, 0, deposits, {skipValidation})
eth1BlockHash, 0, deposits, {skipValidation, skipMerkleValidation})
when isMainModule:
# Smoke test

23
tests/official/test_fixture_operations_attester_slashings.nim
View File

@ -12,7 +12,7 @@ import
os, unittest,
# Beacon chain internals
../../beacon_chain/spec/[datatypes, state_transition_block, validator],
../../beacon_chain/ssz,
../../beacon_chain/[extras, ssz],
# Test utilities
../testutil,
./fixtures_utils,
@ -30,7 +30,10 @@ template runTest(identifier: untyped) =
proc `testImpl _ operations_attester_slashing _ identifier`() =
var flags: UpdateFlags
var prefix: string
if not existsFile(testDir/"meta.yaml"):
flags.incl skipValidation
if existsFile(testDir/"post.ssz"):
prefix = "[Valid] "
else:
@ -52,10 +55,12 @@ template runTest(identifier: untyped) =
postRef[] = parseTest(testDir/"post.ssz", SSZ, BeaconState)
if postRef.isNil:
let done = process_attester_slashing(stateRef[], attesterSlashingRef[], cache)
let done = process_attester_slashing(stateRef[], attesterSlashingRef[],
flags, cache)
doAssert done == false, "We didn't expect this invalid attester slashing to be processed."
else:
let done = process_attester_slashing(stateRef[], attesterSlashingRef[], cache)
let done = process_attester_slashing(stateRef[], attesterSlashingRef[],
flags, cache)
doAssert done, "Valid attestater slashing not processed"
check: stateRef.hash_tree_root() == postRef.hash_tree_root()
reportDiff(stateRef, postRef)
@ -65,8 +70,16 @@ template runTest(identifier: untyped) =
suite "Official - Operations - Attester slashing " & preset():
runTest(success_double)
runTest(success_surround)
runTest(success_already_exited_recent)
runTest(success_already_exited_long_ago)
when false:
# TODO these are both valid and check BLS signatures, which isn't working
# since 0.10.x introduces new BLS signing/verifying interface with domain
# in particular handled differently through compute_signing_root() rather
# than through the bls_verify(...) call directly. This did not become the
# visible issue it now is because another bug had been masking it wherein
# crypto.nim's bls_verify(...) call had been creating false positives, in
# which cases signature checks had been incorrectly passing.
runTest(success_already_exited_recent)
runTest(success_already_exited_long_ago)
runTest(invalid_sig_1)
when false: # TODO - https://github.com/status-im/nim-beacon-chain/issues/429
runTest(invalid_sig_2)

25
tests/official/test_fixture_operations_deposits.nim
View File

@ -53,8 +53,7 @@ template runTest(testName: string, identifier: untyped) =
postRef[] = parseTest(testDir/"post.ssz", SSZ, BeaconState)
if postRef.isNil:
expect(AssertionError):
discard process_deposit(stateRef[], depositRef[], flags)
check not process_deposit(stateRef[], depositRef[], flags)
else:
discard process_deposit(stateRef[], depositRef[], flags)
reportDiff(stateRef, postRef)
@ -62,17 +61,19 @@ template runTest(testName: string, identifier: untyped) =
`testImpl _ operations_deposits _ identifier`()
suite "Official - Operations - Deposits " & preset():
runTest("new deposit under max", new_deposit_under_max)
# https://github.com/ethereum/eth2.0-spec-tests/tree/v0.10.1/tests/minimal/phase0/operations/deposit/pyspec_tests
# https://github.com/ethereum/eth2.0-spec-tests/tree/v0.10.1/tests/mainnet/phase0/operations/deposit/pyspec_tests
runTest("bad merkle proof", bad_merkle_proof)
runTest("invalid signature new deposit", invalid_sig_new_deposit)
runTest("invalid signature other version", invalid_sig_other_version)
runTest("invalid signature top-up", invalid_sig_top_up)
runTest("invalid withdrawal credentials top-up",
invalid_withdrawal_credentials_top_up)
runTest("new deposit max", new_deposit_max)
runTest("new deposit over max", new_deposit_over_max)
runTest("invalid signature new deposit", invalid_sig_new_deposit)
runTest("new deposit under max", new_deposit_under_max)
runTest("success top-up", success_top_up)
runTest("invalid signature top-up", invalid_sig_top_up)
runTest("invalid withdrawal credentials top-up", invalid_withdrawal_credentials_top_up)
when false:
# TODO - those should give an exception but do not
# probably because skipValidation is too strong
# https://github.com/status-im/nim-beacon-chain/issues/407
runTest("wrong deposit for deposit count", wrong_deposit_for_deposit_count)
runTest("bad merkle proof", bad_merkle_proof)
# TODO
runTest("valid signature but forked state", valid_sig_but_forked_state)
runTest("wrong deposit for deposit count", wrong_deposit_for_deposit_count)

6
tests/spec_block_processing/test_process_deposits.nim
View File

@ -56,7 +56,8 @@ suite "[Unit - Spec - Block processing] Deposits " & preset():
# State transition
# ----------------------------------------
check: state.process_deposit(deposit, {skipValidation})
check: state.process_deposit(deposit,
{skipValidation, skipMerkleValidation})
# Check invariants
# ----------------------------------------
@ -100,7 +101,8 @@ suite "[Unit - Spec - Block processing] Deposits " & preset():
# State transition
# ----------------------------------------
check: state.process_deposit(deposit, {skipValidation})
check: state.process_deposit(deposit,
{skipValidation, skipMerkleValidation})
# Check invariants
# ----------------------------------------

4
tests/test_attestation_pool.nim
View File

@ -111,7 +111,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
attestation1 = makeAttestation(
state.data.data, state.blck.root, bc0[1], cache)
attestation0.combine(attestation1, {skipValidation})
attestation0.combine(attestation1, {})
pool.add(attestation0)
pool.add(attestation1)
@ -135,7 +135,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
attestation1 = makeAttestation(
state.data.data, state.blck.root, bc0[1], cache)
attestation0.combine(attestation1, {skipValidation})
attestation0.combine(attestation1, {})
pool.add(attestation1)
pool.add(attestation0)

2
tests/test_beaconstate.nim
View File

@ -17,5 +17,5 @@ suite "Beacon state" & preset():
timedTest "Smoke test initialize_beacon_state_from_eth1" & preset():
let state = initialize_beacon_state_from_eth1(
Eth2Digest(), 0,
makeInitialDeposits(SLOTS_PER_EPOCH, {}), {skipValidation})
makeInitialDeposits(SLOTS_PER_EPOCH, {}), {skipMerkleValidation})
check: state.validators.len == SLOTS_PER_EPOCH

2
tests/test_block_pool.nim
View File

@ -213,7 +213,7 @@ when const_preset == "minimal": # Too much stack space used on mainnet
BeaconBlockBody(
attestations: makeFullAttestations(
pool.headState.data.data, pool.head.blck.root,
pool.headState.data.data.slot, cache, {skipValidation})))
pool.headState.data.data.slot, cache, {})))
let added = pool.add(hash_tree_root(blck.message), blck)
pool.updateHead(added)

2
tests/test_interop.nim
View File

@ -150,7 +150,7 @@ suite "Interop":
var
initialState = initialize_beacon_state_from_eth1(
eth1BlockHash, 1570500000, deposits, {skipValidation})
eth1BlockHash, 1570500000, deposits, {skipMerkleValidation})
# https://github.com/ethereum/eth2.0-pm/tree/6e41fcf383ebeb5125938850d8e9b4e9888389b4/interop/mocked_start#create-genesis-state
initialState.genesis_time = 1570500000

2
tests/test_state_transition.nim
View File

@ -22,7 +22,7 @@ suite "Block processing" & preset():
# TODO bls verification is a bit of a bottleneck here
genesisState = initialize_beacon_state_from_eth1(
Eth2Digest(), 0,
makeInitialDeposits(), {skipValidation})
makeInitialDeposits(), {skipMerkleValidation})
genesisBlock = get_initial_beacon_block(genesisState)
genesisRoot = hash_tree_root(genesisBlock.message)

3
tests/testutil.nim
View File

@ -80,7 +80,8 @@ proc makeTestDB*(validators: int): BeaconChainDB =
let
genState = initialize_beacon_state_from_eth1(
Eth2Digest(), 0,
makeInitialDeposits(validators, flags = {skipValidation}), {skipValidation})
makeInitialDeposits(validators, flags = {skipValidation}),
{skipValidation, skipMerkleValidation})
genBlock = get_initial_beacon_block(genState)
makeTestDB(genState, genBlock)