From ccbbce79a96afbf13eb9d286ced69a68d01a9ba9 Mon Sep 17 00:00:00 2001 From: tersec Date: Fri, 10 Apr 2020 13:59:17 +0000 Subject: [PATCH] Mostly remove skipMerkleValidation by fixing Merkle proof construction/usage (#879) * refactor and fix merkle proof construction in test suite and thereby remove most remaining skipMerkleValidation flags, now unnecessary * a few non-semantic comment update/removals --- beacon_chain/attestation_aggregation.nim | 7 ---- beacon_chain/spec/presets/mainnet.nim | 2 +- beacon_chain/spec/presets/minimal.nim | 2 +- beacon_chain/state_transition.nim | 2 +- research/state_sim.nim | 3 +- tests/mocking/mock_deposits.nim | 41 ++++++------------- tests/mocking/mock_genesis.nim | 4 +- .../test_process_deposits.nim | 8 ++-- tests/test_beacon_chain_db.nim | 2 +- tests/test_beaconstate.nim | 3 +- tests/test_state_transition.nim | 3 +- tests/testblockutil.nim | 32 ++++++++++++++- tests/testutil.nim | 2 +- 13 files changed, 55 insertions(+), 56 deletions(-) diff --git a/beacon_chain/attestation_aggregation.nim b/beacon_chain/attestation_aggregation.nim index 584e47e9e..623ef9ded 100644 --- a/beacon_chain/attestation_aggregation.nim +++ b/beacon_chain/attestation_aggregation.nim @@ -5,13 +5,6 @@ # * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -# The other part is arguably part of attestation pool -- the validation's -# something that should be happing on receipt, not aggregation per se. In -# that part, check that messages conform -- so, check for each type -# https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/p2p-interface.md#topics-and-messages -# specifies. So by the time this calls attestation pool, all validation's -# already done. - import options, ./spec/[beaconstate, datatypes, crypto, digest, helpers, validator, diff --git a/beacon_chain/spec/presets/mainnet.nim b/beacon_chain/spec/presets/mainnet.nim index 33dfeb0d8..708b3ab52 100644 --- a/beacon_chain/spec/presets/mainnet.nim +++ b/beacon_chain/spec/presets/mainnet.nim @@ -80,7 +80,7 @@ const # Time parameters # --------------------------------------------------------------- - # https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/configs/mainnet.yaml#L71 + # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/configs/mainnet.yaml#L77 MIN_GENESIS_DELAY* = 86400 # 86400 seconds (1 day) SECONDS_PER_SLOT*{.intdefine.} = 12'u64 # Compile with -d:SECONDS_PER_SLOT=1 for 12x faster slots diff --git a/beacon_chain/spec/presets/minimal.nim b/beacon_chain/spec/presets/minimal.nim index aa474a512..cb5ab92ab 100644 --- a/beacon_chain/spec/presets/minimal.nim +++ b/beacon_chain/spec/presets/minimal.nim @@ -62,7 +62,7 @@ const # Initial values # --------------------------------------------------------------- - # https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/configs/minimal.yaml#L64 + # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/configs/minimal.yaml#L70 # Unchanged GENESIS_SLOT* = 0.Slot diff --git a/beacon_chain/state_transition.nim b/beacon_chain/state_transition.nim index 1f2362333..540f232da 100644 --- a/beacon_chain/state_transition.nim +++ b/beacon_chain/state_transition.nim @@ -31,7 +31,7 @@ # now. import - collections/sets, chronicles, sets, options, + collections/sets, chronicles, sets, ./extras, ./ssz, metrics, ./spec/[datatypes, crypto, digest, helpers, validator], ./spec/[state_transition_block, state_transition_epoch], diff --git a/research/state_sim.nim b/research/state_sim.nim index 0b6f7bb4e..27d089c74 100644 --- a/research/state_sim.nim +++ b/research/state_sim.nim @@ -79,8 +79,7 @@ cli do(slots = SLOTS_PER_EPOCH * 6, let genesisState = - initialize_beacon_state_from_eth1( - Eth2Digest(), 0, deposits, flags + {skipMerkleValidation}) + initialize_beacon_state_from_eth1(Eth2Digest(), 0, deposits, flags) genesisBlock = get_initial_beacon_block(genesisState) echo "Starting simulation..." diff --git a/tests/mocking/mock_deposits.nim b/tests/mocking/mock_deposits.nim index 8d53bda15..bbb71d9c1 100644 --- a/tests/mocking/mock_deposits.nim +++ b/tests/mocking/mock_deposits.nim @@ -1,5 +1,5 @@ # beacon_chain -# Copyright (c) 2018-2019 Status Research & Development GmbH +# Copyright (c) 2018-2020 Status Research & Development GmbH # Licensed and distributed under either of # * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). @@ -12,11 +12,13 @@ import # Standard library math, random, # Specs - ../../beacon_chain/spec/[datatypes, crypto, helpers, digest, beaconstate], + ../../beacon_chain/spec/[datatypes, crypto, helpers, digest], # Internals ../../beacon_chain/[ssz, extras], # Mocking procs - ./merkle_minimal, ./mock_validator_keys + ./mock_validator_keys, + # Other test utilities, for attachMerkleProofs() + ../testblockutil func signMockDepositData( deposit_data: var DepositData, @@ -143,26 +145,6 @@ template mockGenesisDepositsImpl( depositsData.add result[valIdx].data depositsDataHash.add hash_tree_root(result[valIdx].data) - # 2nd & 3rd loops - build hashes and proofs - let root = hash_tree_root(depositsData) - let tree = merkleTreeFromLeaves(depositsDataHash) - - # 4th loop - append proof - for valIdx in 0 ..< validatorCount.int: - # TODO ensure genesis & deposit process tests suffice to catch whether - # changes here break things; ensure that this matches the merkle proof - # sequence is_valid_merkle_branch(...) now looks for - result[valIdx].proof[0..31] = tree.getMerkleProof(valIdx) - result[valIdx].proof[32] = - Eth2Digest(data: int_to_bytes32((valIdx + 1).uint64)) - doAssert is_valid_merkle_branch( - depositsDataHash[valIdx], - result[valIdx].proof, - DEPOSIT_CONTRACT_TREE_DEPTH, - valIdx.uint64, - root - ) - proc mockGenesisBalancedDeposits*( validatorCount: uint64, amountInEth: Positive, @@ -179,6 +161,7 @@ proc mockGenesisBalancedDeposits*( let amount = amountInEth.uint64 * 10'u64^9 mockGenesisDepositsImpl(result, validatorCount,amount,flags): discard + attachMerkleProofs(result) proc mockGenesisUnBalancedDeposits*( validatorCount: uint64, @@ -199,6 +182,7 @@ proc mockGenesisUnBalancedDeposits*( mockGenesisDepositsImpl(result, validatorCount, amount, flags): amount = rng.rand(amountRangeInEth).uint64 * 10'u64^9 + attachMerkleProofs(result) proc mockUpdateStateForNewDeposit*( state: var BeaconState, @@ -220,14 +204,13 @@ proc mockUpdateStateForNewDeposit*( flags ) - when false: # TODO - let tree = merkleTreeFromLeaves([hash_tree_root(result.data)]) - result[valIdx].proof[0..31] = tree.getMerkleProof(0) - result[valIdx].proof[32] = int_to_bytes32(0 + 1) - # doAssert is_valid_merkle_branch(...) + var result_seq = @[result] + attachMerkleProofs(result_seq) + result.proof = result_seq[0].proof # TODO: this logic from the eth2.0-specs test suite seems strange # but confirmed by running it state.eth1_deposit_index = 0 - state.eth1_data.deposit_root = hash_tree_root(result.data) + state.eth1_data.deposit_root = + hash_tree_root(sszList(@[result.data], 2'i64^DEPOSIT_CONTRACT_TREE_DEPTH)) state.eth1_data.deposit_count = 1 diff --git a/tests/mocking/mock_genesis.nim b/tests/mocking/mock_genesis.nim index e23fb62c1..14913ef7b 100644 --- a/tests/mocking/mock_genesis.nim +++ b/tests/mocking/mock_genesis.nim @@ -1,5 +1,5 @@ # beacon_chain -# Copyright (c) 2018-2019 Status Research & Development GmbH +# Copyright (c) 2018-2020 Status Research & Development GmbH # Licensed and distributed under either of # * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). @@ -25,7 +25,7 @@ proc initGenesisState*(num_validators: uint64, genesis_time: uint64 = 0): Beacon ) initialize_beacon_state_from_eth1( - eth1BlockHash, 0, deposits, {skipBlsValidation, skipMerkleValidation}) + eth1BlockHash, 0, deposits, {skipBlsValidation}) when isMainModule: # Smoke test diff --git a/tests/spec_block_processing/test_process_deposits.nim b/tests/spec_block_processing/test_process_deposits.nim index 765e0aa8d..713e815ca 100644 --- a/tests/spec_block_processing/test_process_deposits.nim +++ b/tests/spec_block_processing/test_process_deposits.nim @@ -1,5 +1,5 @@ # beacon_chain -# Copyright (c) 2018 Status Research & Development GmbH +# Copyright (c) 2018-2020 Status Research & Development GmbH # Licensed and distributed under either of # * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). @@ -56,8 +56,7 @@ suiteReport "[Unit - Spec - Block processing] Deposits " & preset(): # State transition # ---------------------------------------- - check: state.process_deposit(deposit, - {skipBlsValidation, skipMerkleValidation}) + check: state.process_deposit(deposit, {skipBlsValidation}) # Check invariants # ---------------------------------------- @@ -101,8 +100,7 @@ suiteReport "[Unit - Spec - Block processing] Deposits " & preset(): # State transition # ---------------------------------------- - check: state.process_deposit(deposit, - {skipBlsValidation, skipMerkleValidation}) + check: state.process_deposit(deposit, {skipBlsValidation}) # Check invariants # ---------------------------------------- diff --git a/tests/test_beacon_chain_db.nim b/tests/test_beacon_chain_db.nim index 28bf53ee4..8f8dc9de5 100644 --- a/tests/test_beacon_chain_db.nim +++ b/tests/test_beacon_chain_db.nim @@ -99,7 +99,7 @@ suiteReport "Beacon chain DB" & preset(): let state = initialize_beacon_state_from_eth1( - eth1BlockHash, 0, makeInitialDeposits(SLOTS_PER_EPOCH), {skipBlsValidation, skipMerkleValidation}) + eth1BlockHash, 0, makeInitialDeposits(SLOTS_PER_EPOCH), {skipBlsValidation}) root = hash_tree_root(state) db.putState(state) diff --git a/tests/test_beaconstate.nim b/tests/test_beaconstate.nim index b539ce9a8..365bdebc7 100644 --- a/tests/test_beaconstate.nim +++ b/tests/test_beaconstate.nim @@ -16,6 +16,5 @@ import suiteReport "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, {}), {skipMerkleValidation}) + Eth2Digest(), 0, makeInitialDeposits(SLOTS_PER_EPOCH, {}), {}) check: state.validators.len == SLOTS_PER_EPOCH diff --git a/tests/test_state_transition.nim b/tests/test_state_transition.nim index a48254300..1a2493893 100644 --- a/tests/test_state_transition.nim +++ b/tests/test_state_transition.nim @@ -21,8 +21,7 @@ suiteReport "Block processing" & preset(): # Genesis state with minimal number of deposits # TODO bls verification is a bit of a bottleneck here genesisState = initialize_beacon_state_from_eth1( - Eth2Digest(), 0, - makeInitialDeposits(), {skipMerkleValidation}) + Eth2Digest(), 0, makeInitialDeposits(), {}) genesisBlock = get_initial_beacon_block(genesisState) genesisRoot = hash_tree_root(genesisBlock.message) diff --git a/tests/testblockutil.nim b/tests/testblockutil.nim index 45c30bed6..9c8959585 100644 --- a/tests/testblockutil.nim +++ b/tests/testblockutil.nim @@ -6,8 +6,9 @@ # at your option. This file may not be copied, modified, or distributed except according to those terms. import - options, stew/endians2, + options, sequtils, stew/endians2, chronicles, eth/trie/[db], + ./mocking/merkle_minimal, ../beacon_chain/[beacon_chain_db, block_pool, extras, ssz, state_transition, validator_pool], ../beacon_chain/spec/[beaconstate, crypto, datatypes, digest, @@ -57,11 +58,38 @@ func makeDeposit(i: int, flags: UpdateFlags): Deposit = let signing_root = compute_signing_root(result.getDepositMessage, domain) result.data.signature = bls_sign(privkey, signing_root.data) -func makeInitialDeposits*( +proc attachMerkleProofs*(deposits: var seq[Deposit]) = + let deposit_data_roots = mapIt(deposits, it.data.hash_tree_root) + var + deposit_data_sums: seq[Eth2Digest] + for prefix_root in hash_tree_roots_prefix( + deposit_data_roots, 1'i64 shl DEPOSIT_CONTRACT_TREE_DEPTH): + deposit_data_sums.add prefix_root + + for val_idx in 0 ..< deposits.len: + let merkle_tree = merkleTreeFromLeaves(deposit_data_roots[0..val_idx]) + deposits[val_idx].proof[0..31] = merkle_tree.getMerkleProof(val_idx) + deposits[val_idx].proof[32].data[0..7] = int_to_bytes8((val_idx + 1).uint64) + + doAssert is_valid_merkle_branch( + deposit_data_roots[val_idx], deposits[val_idx].proof, + DEPOSIT_CONTRACT_TREE_DEPTH + 1, val_idx.uint64, + deposit_data_sums[val_idx]) + +proc makeInitialDeposits*( n = SLOTS_PER_EPOCH, flags: UpdateFlags = {}): seq[Deposit] = for i in 0..