From 7577f8c2ef37ddd7f4cd299d23ae4d58ec591181 Mon Sep 17 00:00:00 2001 From: tersec Date: Tue, 29 Jun 2021 15:09:29 +0000 Subject: [PATCH] add blockchain_dag altair database reading; add rollback tests (#2683) * add blockchain_dag altair database reading; add rollback tests; fix some unnecessary type conversions * remove debugging scaffolding * proposeSignedBlock() will need to be async for merge; introduce altair types to VC --- AllTests-mainnet.md | 6 ++- beacon_chain/beacon_chain_db.nim | 19 ++++--- .../consensus_object_pools/blockchain_dag.nim | 20 +++++-- beacon_chain/fork_choice/fork_choice.nim | 2 +- beacon_chain/nimbus_validator_client.nim | 5 +- beacon_chain/rpc/beacon_api.nim | 2 +- beacon_chain/rpc/beacon_rest_api.nim | 4 +- .../rpc/eth2_json_rest_serialization.nim | 4 +- beacon_chain/rpc/rpc_utils.nim | 2 +- beacon_chain/rpc/validator_api.nim | 2 +- beacon_chain/spec/state_transition.nim | 10 ++-- beacon_chain/spec/state_transition_epoch.nim | 2 +- beacon_chain/validators/validator_duties.nim | 9 ++-- tests/test_beacon_chain_db.nim | 53 +++++++++++++++++-- 14 files changed, 102 insertions(+), 38 deletions(-) diff --git a/AllTests-mainnet.md b/AllTests-mainnet.md index e96943910..20bc20f27 100644 --- a/AllTests-mainnet.md +++ b/AllTests-mainnet.md @@ -82,16 +82,18 @@ OK: 11/11 Fail: 0/11 Skip: 0/11 ```diff + empty database [Preset: mainnet] OK + find ancestors [Preset: mainnet] OK ++ sanity check Altair and cross-fork getState rollback [Preset: mainnet] OK + sanity check Altair blocks [Preset: mainnet] OK + sanity check Altair states [Preset: mainnet] OK + sanity check Altair states, reusing buffers [Preset: mainnet] OK + sanity check genesis roundtrip [Preset: mainnet] OK + sanity check phase 0 blocks [Preset: mainnet] OK ++ sanity check phase 0 getState rollback [Preset: mainnet] OK + sanity check phase 0 states [Preset: mainnet] OK + sanity check phase 0 states, reusing buffers [Preset: mainnet] OK + sanity check state diff roundtrip [Preset: mainnet] OK ``` -OK: 10/10 Fail: 0/10 Skip: 0/10 +OK: 12/12 Fail: 0/12 Skip: 0/12 ## Beacon state [Preset: mainnet] ```diff + Smoke test initialize_beacon_state_from_eth1 [Preset: mainnet] OK @@ -311,4 +313,4 @@ OK: 3/3 Fail: 0/3 Skip: 0/3 OK: 1/1 Fail: 0/1 Skip: 0/1 ---TOTAL--- -OK: 177/185 Fail: 0/185 Skip: 8/185 +OK: 179/187 Fail: 0/187 Skip: 8/187 diff --git a/beacon_chain/beacon_chain_db.nim b/beacon_chain/beacon_chain_db.nim index d989b0d6c..1d57e5951 100644 --- a/beacon_chain/beacon_chain_db.nim +++ b/beacon_chain/beacon_chain_db.nim @@ -344,7 +344,7 @@ proc new*(T: type BeaconChainDB, altair_blocks: altair_blocks, stateRoots: stateRoots, statesNoVal: statesNoVal, - altairStatesNoVal: statesNoVal, + altairStatesNoVal: altairStatesNoVal, stateDiffs: stateDiffs, summaries: summaries, ) @@ -514,6 +514,13 @@ proc putState*( db: BeaconChainDB, value: phase0.BeaconState | altair.BeaconState) = db.putState(hash_tree_root(value), value) +# For testing rollback +proc putCorruptPhase0State*(db: BeaconChainDB, key: Eth2Digest) = + db.statesNoVal.putSnappySSZ(key.data, Validator()) + +proc putCorruptAltairState*(db: BeaconChainDB, key: Eth2Digest) = + db.altairStatesNoVal.putSnappySSZ(key.data, Validator()) + func stateRootKey(root: Eth2Digest, slot: Slot): array[40, byte] = var ret: array[40, byte] # big endian to get a naturally ascending order on slots in sorted indices @@ -626,13 +633,13 @@ proc getStateOnlyMutableValidators( of GetResult.notFound: false of GetResult.corrupted: - rollback(output) + rollback() false proc getAltairStateOnlyMutableValidators( immutableValidators: openArray[ImmutableValidatorData2], store: KvStoreRef, key: openArray[byte], output: var altair.BeaconState, - rollback: AltairRollbackProc): bool = + rollback: RollbackProc): bool = ## Load state into `output` - BeaconState is large so we want to avoid ## re-allocating it if possible ## Return `true` iff the entry was found in the database and `output` was @@ -668,7 +675,7 @@ proc getAltairStateOnlyMutableValidators( of GetResult.notFound: false of GetResult.corrupted: - rollback(output) + rollback() false proc getState( @@ -697,7 +704,7 @@ proc getState( of GetResult.notFound: false of GetResult.corrupted: - rollback(output) + rollback() false proc getState*( @@ -721,7 +728,7 @@ proc getState*( proc getAltairState*( db: BeaconChainDB, key: Eth2Digest, output: var altair.BeaconState, - rollback: AltairRollbackProc): bool = + rollback: RollbackProc): bool = ## Load state into `output` - BeaconState is large so we want to avoid ## re-allocating it if possible ## Return `true` iff the entry was found in the database and `output` was diff --git a/beacon_chain/consensus_object_pools/blockchain_dag.nim b/beacon_chain/consensus_object_pools/blockchain_dag.nim index f24926c8b..7cdcfe8c2 100644 --- a/beacon_chain/consensus_object_pools/blockchain_dag.nim +++ b/beacon_chain/consensus_object_pools/blockchain_dag.nim @@ -510,11 +510,23 @@ proc getState( else: unsafeAddr dag.headState - func restore(v: var phase0.BeaconState) = - assign(v, restoreAddr[].data.hbsPhase0.data) + let v = addr state.data - if not dag.db.getState(stateRoot, state.data.hbsPhase0.data, restore): - return false + func restore() = + assign(v[], restoreAddr[].data) + + if blck.slot < dag.altairTransitionSlot: + if state.data.beaconStateFork != forkPhase0: + state.data = (ref ForkedHashedBeaconState)(beaconStateFork: forkPhase0)[] + + if not dag.db.getState(stateRoot, state.data.hbsPhase0.data, restore): + return false + else: + if state.data.beaconStateFork != forkAltair: + state.data = (ref ForkedHashedBeaconState)(beaconStateFork: forkAltair)[] + + if not dag.db.getAltairState(stateRoot, state.data.hbsAltair.data, restore): + return false state.blck = blck setStateRoot(state.data, stateRoot) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 0d1977e01..cde88502b 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -157,7 +157,7 @@ proc process_attestation_queue(self: var ForkChoice) = if it.slot < self.checkpoints.time: for validator_index in it.attesting_indices: self.backend.process_attestation( - validator_index.ValidatorIndex, it.block_root, it.slot.epoch()) + validator_index, it.block_root, it.slot.epoch()) false else: true diff --git a/beacon_chain/nimbus_validator_client.nim b/beacon_chain/nimbus_validator_client.nim index af4542040..7b683e6a5 100644 --- a/beacon_chain/nimbus_validator_client.nim +++ b/beacon_chain/nimbus_validator_client.nim @@ -18,7 +18,8 @@ import json_serialization/std/[options, net], # Local modules - ./spec/[datatypes, digest, crypto, helpers, network, signatures], + ./spec/datatypes/[phase0, altair], + ./spec/[digest, crypto, helpers, network, signatures], ./spec/eth2_apis/beacon_rpc_client, ./sync/sync_manager, "."/[conf, beacon_clock, version], @@ -140,7 +141,7 @@ proc onSlotStart(vc: ValidatorClient, lastSlot, scheduledSlot: Slot) {.gcsafe, a let validator = vc.attachedValidators.validators[public_key] let randao_reveal = await validator.genRandaoReveal( vc.fork, vc.beaconGenesis.genesis_validators_root, slot) - var newBlock = SignedBeaconBlock( + var newBlock = phase0.SignedBeaconBlock( message: await vc.client.get_v1_validator_block(slot, vc.graffitiBytes, randao_reveal) ) newBlock.root = hash_tree_root(newBlock.message) diff --git a/beacon_chain/rpc/beacon_api.nim b/beacon_chain/rpc/beacon_api.nim index ef720b2b4..a48afa2b2 100644 --- a/beacon_chain/rpc/beacon_api.nim +++ b/beacon_chain/rpc/beacon_api.nim @@ -409,7 +409,7 @@ proc installBeaconApiHandlers*(rpcServer: RpcServer, node: BeaconNode) {. # It was not integrated into the beacon node's database. return 202 else: - let res = proposeSignedBlock(node, head, AttachedValidator(), blck) + let res = await proposeSignedBlock(node, head, AttachedValidator(), blck) if res == head: node.network.broadcast(getBeaconBlocksTopic(node.forkDigest), blck) # The block failed validation, but was successfully broadcast anyway. diff --git a/beacon_chain/rpc/beacon_rest_api.nim b/beacon_chain/rpc/beacon_rest_api.nim index 0f81cc9cf..0d0d8051f 100644 --- a/beacon_chain/rpc/beacon_rest_api.nim +++ b/beacon_chain/rpc/beacon_rest_api.nim @@ -502,7 +502,7 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = else: let idx = cindex.get() if uint64(idx) < committees_per_slot: - res.add(getCommittee(slot, CommitteeIndex(idx))) + res.add(getCommittee(slot, idx)) var res: seq[RestBeaconStatesCommittees] let qepoch = @@ -629,7 +629,7 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = node.network.broadcast(getBeaconBlocksTopic(node.forkDigest), blck) return RestApiResponse.jsonError(Http202, BlockValidationError) else: - let res = proposeSignedBlock(node, head, AttachedValidator(), blck) + let res = await proposeSignedBlock(node, head, AttachedValidator(), blck) if res == head: node.network.broadcast(getBeaconBlocksTopic(node.forkDigest), blck) return RestApiResponse.jsonError(Http202, BlockValidationError) diff --git a/beacon_chain/rpc/eth2_json_rest_serialization.nim b/beacon_chain/rpc/eth2_json_rest_serialization.nim index 95a518589..19dcd9daf 100644 --- a/beacon_chain/rpc/eth2_json_rest_serialization.nim +++ b/beacon_chain/rpc/eth2_json_rest_serialization.nim @@ -336,9 +336,9 @@ proc decodeBody*[T](t: typedesc[T], let data = try: RestJson.decode(cast[string](body.data), T) - except SerializationError as exc: + except SerializationError: return err("Unable to deserialize data") - except CatchableError as exc: + except CatchableError: return err("Unexpected deserialization error") ok(data) diff --git a/beacon_chain/rpc/rpc_utils.nim b/beacon_chain/rpc/rpc_utils.nim index 95d28f8b7..45a270b26 100644 --- a/beacon_chain/rpc/rpc_utils.nim +++ b/beacon_chain/rpc/rpc_utils.nim @@ -26,7 +26,7 @@ template withStateForStateId*(stateId: string, body: untyped): untyped = if isState(node.dag.headState): withStateVars(node.dag.headState): - var cache {.inject.}: StateCache + var cache {.inject, used.}: StateCache body else: let rpcState = assignClone(node.dag.headState) diff --git a/beacon_chain/rpc/validator_api.nim b/beacon_chain/rpc/validator_api.nim index ae5a93fb5..b914269c9 100644 --- a/beacon_chain/rpc/validator_api.nim +++ b/beacon_chain/rpc/validator_api.nim @@ -56,7 +56,7 @@ proc installValidatorApiHandlers*(rpcServer: RpcServer, node: BeaconNode) {. if head.slot >= body.message.slot: raise newException(CatchableError, "Proposal is for a past slot: " & $body.message.slot) - if head == proposeSignedBlock(node, head, AttachedValidator(), body): + if head == await proposeSignedBlock(node, head, AttachedValidator(), body): raise newException(CatchableError, "Could not propose block") return true diff --git a/beacon_chain/spec/state_transition.nim b/beacon_chain/spec/state_transition.nim index 06b49e178..17717c2d0 100644 --- a/beacon_chain/spec/state_transition.nim +++ b/beacon_chain/spec/state_transition.nim @@ -104,14 +104,10 @@ func verifyStateRoot(state: phase0.BeaconState, blck: altair.TrustedBeaconBlock) true type - RollbackProc* = proc(v: var phase0.BeaconState) {.gcsafe, raises: [Defect].} - AltairRollbackProc* = proc(v: var altair.BeaconState) {.gcsafe, raises: [Defect].} + RollbackProc* = proc() {.gcsafe, raises: [Defect].} -func noRollback*(state: var phase0.BeaconState) = - trace "Skipping rollback of broken phase 0 state" - -func noRollback*(state: var altair.BeaconState) = - trace "Skipping rollback of broken Altair state" +func noRollback*() = + trace "Skipping rollback of broken state" type RollbackHashedProc* = proc(state: var phase0.HashedBeaconState) {.gcsafe, raises: [Defect].} diff --git a/beacon_chain/spec/state_transition_epoch.nim b/beacon_chain/spec/state_transition_epoch.nim index fa43acb27..580266b81 100644 --- a/beacon_chain/spec/state_transition_epoch.nim +++ b/beacon_chain/spec/state_transition_epoch.nim @@ -874,7 +874,7 @@ proc process_epoch*( process_historical_roots_update(state) process_participation_record_updates(state) -# https://github.com/ethereum/eth2.0-specs/blob/v1.1.0-alpha.6/specs/altair/beacon-chain.md#epoch-processing +# https://github.com/ethereum/eth2.0-specs/blob/v1.1.0-alpha.7/specs/altair/beacon-chain.md#epoch-processing proc process_epoch*( state: var altair.BeaconState, flags: UpdateFlags, cache: var StateCache, rewards: var RewardInfo) {.nbench.} = diff --git a/beacon_chain/validators/validator_duties.nim b/beacon_chain/validators/validator_duties.nim index dfd48bf2b..e9a719459 100644 --- a/beacon_chain/validators/validator_duties.nim +++ b/beacon_chain/validators/validator_duties.nim @@ -110,10 +110,10 @@ proc getAttachedValidator*(node: BeaconNode, idx: ValidatorIndex): AttachedValidator = if idx < state_validators.len.ValidatorIndex: let validator = node.getAttachedValidator(state_validators[idx].pubkey) - if validator != nil and validator.index != some(idx.ValidatorIndex): + if validator != nil and validator.index != some(idx): # Update index, in case the validator was activated! notice "Validator activated", pubkey = validator.pubkey, index = idx - validator.index = some(idx.ValidatorIndex) + validator.index = some(idx) validator else: warn "Validator index out of bounds", @@ -340,7 +340,8 @@ proc makeBeaconBlockForHeadAndSlot*(node: BeaconNode, proc proposeSignedBlock*(node: BeaconNode, head: BlockRef, validator: AttachedValidator, - newBlock: SignedBeaconBlock): BlockRef = + newBlock: SignedBeaconBlock): + Future[BlockRef] {.async.} = let newBlockRef = node.dag.addRawBlock(node.quarantine, newBlock) do ( blckRef: BlockRef, trustedBlock: TrustedSignedBeaconBlock, epochRef: EpochRef): @@ -420,7 +421,7 @@ proc proposeBlock(node: BeaconNode, newBlock.signature = await validator.signBlockProposal( fork, genesis_validators_root, slot, newBlock.root) - return node.proposeSignedBlock(head, validator, newBlock) + return await node.proposeSignedBlock(head, validator, newBlock) proc handleAttestations(node: BeaconNode, head: BlockRef, slot: Slot) = ## Perform all attestations that the validators attached to this node should diff --git a/tests/test_beacon_chain_db.nim b/tests/test_beacon_chain_db.nim index 842ff39e0..fcd74a5d5 100644 --- a/tests/test_beacon_chain_db.nim +++ b/tests/test_beacon_chain_db.nim @@ -60,8 +60,7 @@ suite "Beacon chain DB" & preset(): db.getBlock(Eth2Digest()).isNone test "sanity check phase 0 blocks" & preset(): - var - db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true) + var db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true) let signedBlock = withDigest((phase0.TrustedBeaconBlock)()) @@ -90,8 +89,7 @@ suite "Beacon chain DB" & preset(): db.close() test "sanity check Altair blocks" & preset(): - var - db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true) + var db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true) let signedBlock = withDigest((altair.TrustedBeaconBlock)()) @@ -225,6 +223,53 @@ suite "Beacon chain DB" & preset(): db.close() + test "sanity check phase 0 getState rollback" & preset(): + var + db = makeTestDB(SLOTS_PER_EPOCH) + dag = init(ChainDAGRef, defaultRuntimePreset, db) + state = (ref ForkedHashedBeaconState)( + beaconStateFork: forkPhase0, + hbsPhase0: phase0.HashedBeaconState(data: phase0.BeaconState( + slot: 10.Slot))) + root = Eth2Digest() + + db.putCorruptPhase0State(root) + + let restoreAddr = addr dag.headState + + func restore() = + assign(state[], restoreAddr[].data) + + check: + state[].hbsPhase0.data.slot == 10.Slot + not db.getState(root, state[].hbsPhase0.data, restore) + state[].hbsPhase0.data.slot != 10.Slot + + test "sanity check Altair and cross-fork getState rollback" & preset(): + var + db = makeTestDB(SLOTS_PER_EPOCH) + dag = init(ChainDAGRef, defaultRuntimePreset, db) + state = (ref ForkedHashedBeaconState)( + beaconStateFork: forkAltair, + hbsAltair: altair.HashedBeaconState(data: altair.BeaconState( + slot: 10.Slot))) + root = Eth2Digest() + + db.putCorruptAltairState(root) + + let restoreAddr = addr dag.headState + + func restore() = + assign(state[], restoreAddr[].data) + + check: + state[].hbsAltair.data.slot == 10.Slot + not db.getAltairState(root, state[].hbsAltair.data, restore) + + # assign() has switched the case object fork + state[].beaconStateFork == forkPhase0 + state[].hbsPhase0.data.slot != 10.Slot + test "find ancestors" & preset(): var db = BeaconChainDB.new(defaultRuntimePreset, "", inMemory = true)