From e4afc36d71dff000c90e87a6520dd5f140addbc0 Mon Sep 17 00:00:00 2001 From: tersec Date: Wed, 14 Jul 2021 12:18:52 +0000 Subject: [PATCH] use ForkedTrustedSignedBeaconBlock (#2720) * use ForkedTrustedSignedBeaconBlock * remove --subscribe-all-subnets * https://ethereum.github.io/eth2.0-APIs/#/Beacon/getBlock implementation was passing through forked beaconblocks --- beacon_chain/beacon_node_common.nim | 2 - .../attestation_pool.nim | 11 ++-- .../block_pools_types.nim | 2 +- .../consensus_object_pools/blockchain_dag.nim | 30 +++++---- .../gossip_processing/gossip_validation.nim | 8 +-- beacon_chain/nimbus_beacon_node.nim | 12 ++-- beacon_chain/rpc/beacon_api.nim | 17 +++-- beacon_chain/rpc/beacon_rest_api.nim | 45 +++++++------ beacon_chain/rpc/validator_api.nim | 6 +- beacon_chain/rpc/validator_rest_api.nim | 3 +- .../spec/forkedbeaconstate_helpers.nim | 63 +++++++++++++++---- beacon_chain/spec/state_transition.nim | 8 ++- beacon_chain/sync/sync_protocol.nim | 6 +- beacon_chain/validators/validator_duties.nim | 6 +- tests/test_block_pool.nim | 2 +- 15 files changed, 142 insertions(+), 79 deletions(-) diff --git a/beacon_chain/beacon_node_common.nim b/beacon_chain/beacon_node_common.nim index 7e81e8698..c611dfe53 100644 --- a/beacon_chain/beacon_node_common.nim +++ b/beacon_chain/beacon_node_common.nim @@ -50,8 +50,6 @@ type vcProcess*: Process requestManager*: RequestManager syncManager*: SyncManager[Peer, PeerID] - topicBeaconBlocks*: string - topicAggregateAndProofs*: string genesisSnapshotContent*: string attestationSubnets*: AttestationSubnets processor*: ref Eth2Processor diff --git a/beacon_chain/consensus_object_pools/attestation_pool.nim b/beacon_chain/consensus_object_pools/attestation_pool.nim index 6b3aa7e12..e956711e0 100644 --- a/beacon_chain/consensus_object_pools/attestation_pool.nim +++ b/beacon_chain/consensus_object_pools/attestation_pool.nim @@ -64,7 +64,7 @@ proc init*(T: type AttestationPool, dag: ChainDAGRef, quarantine: QuarantineRef) var epochRef = finalizedEpochRef for i in 0.. tuple[canonical: bool, header: SignedBeaconBlockHeader]: let bd = node.getBlockDataFromBlockId(blockId) - let tsbb = bd.data + # TODO check for Altair blocks and fail, because /v1/ + let tsbb = bd.data.phase0Block static: doAssert tsbb.signature is TrustedSig and sizeof(ValidatorSig) == sizeof(tsbb.signature) result.header.signature = cast[ValidatorSig](tsbb.signature) @@ -404,7 +405,8 @@ proc installBeaconApiHandlers*(rpcServer: RpcServer, node: BeaconNode) {. "Beacon node is currently syncing, try again later.") let head = node.dag.head if head.slot >= blck.message.slot: - # TODO altair-transition + # TODO altair-transition, but not immediate testnet-priority to detect + # Altair and fail, since /v1/ doesn't support Altair let blocksTopic = getBeaconBlocksTopic(node.dag.forkDigests.phase0) node.network.broadcast(blocksTopic, blck) # The block failed validation, but was successfully broadcast anyway. @@ -413,7 +415,7 @@ proc installBeaconApiHandlers*(rpcServer: RpcServer, node: BeaconNode) {. else: let res = await proposeSignedBlock(node, head, AttachedValidator(), blck) if res == head: - # TODO altair-transition + # TODO altair-transition, but not immediate testnet-priority let blocksTopic = getBeaconBlocksTopic(node.dag.forkDigests.phase0) node.network.broadcast(blocksTopic, blck) # The block failed validation, but was successfully broadcast anyway. @@ -426,15 +428,18 @@ proc installBeaconApiHandlers*(rpcServer: RpcServer, node: BeaconNode) {. rpcServer.rpc("get_v1_beacon_blocks_blockId") do ( blockId: string) -> TrustedSignedBeaconBlock: - return node.getBlockDataFromBlockId(blockId).data + # TODO detect Altair and fail: /v1/ APIs don't support Altair + return node.getBlockDataFromBlockId(blockId).data.phase0Block rpcServer.rpc("get_v1_beacon_blocks_blockId_root") do ( blockId: string) -> Eth2Digest: - return node.getBlockDataFromBlockId(blockId).data.message.state_root + # TODO detect Altair and fail: /v1/ APIs don't support Altair + return node.getBlockDataFromBlockId(blockId).data.phase0Block.message.state_root rpcServer.rpc("get_v1_beacon_blocks_blockId_attestations") do ( blockId: string) -> seq[TrustedAttestation]: - return node.getBlockDataFromBlockId(blockId).data.message.body.attestations.asSeq + # TODO detect Altair and fail: /v1/ APIs don't support Altair + return node.getBlockDataFromBlockId(blockId).data.phase0Block.message.body.attestations.asSeq rpcServer.rpc("get_v1_beacon_pool_attestations") do ( slot: Option[uint64], committee_index: Option[uint64]) -> diff --git a/beacon_chain/rpc/beacon_rest_api.nim b/beacon_chain/rpc/beacon_rest_api.nim index aeb4c3c64..778d8e8dd 100644 --- a/beacon_chain/rpc/beacon_rest_api.nim +++ b/beacon_chain/rpc/beacon_rest_api.nim @@ -539,17 +539,19 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = return RestApiResponse.jsonResponse( ( - root: bdata.data.root, + # TODO Altair insofar as it should detect the error condition rather + # than crashing. This API is only specified for phase 0 + root: bdata.data.phase0Block.root, canonical: bdata.refs.isAncestorOf(node.dag.head), header: ( message: ( - slot: bdata.data.message.slot, - proposer_index: bdata.data.message.proposer_index, - parent_root: bdata.data.message.parent_root, - state_root: bdata.data.message.state_root, - body_root: bdata.data.message.body.hash_tree_root() + slot: bdata.data.phase0Block.message.slot, + proposer_index: bdata.data.phase0Block.message.proposer_index, + parent_root: bdata.data.phase0Block.message.parent_root, + state_root: bdata.data.phase0Block.message.state_root, + body_root: bdata.data.phase0Block.message.body.hash_tree_root() ), - signature: bdata.data.signature + signature: bdata.data.phase0Block.signature ) ) ) @@ -569,17 +571,19 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = return RestApiResponse.jsonResponse( ( - root: bdata.data.root, + # TODO for Altair, check that it's a phase 0 block and return error if + # not, since /v1/ APIs don't support Altair + root: bdata.data.phase0Block.root, canonical: bdata.refs.isAncestorOf(node.dag.head), header: ( message: ( - slot: bdata.data.message.slot, - proposer_index: bdata.data.message.proposer_index, - parent_root: bdata.data.message.parent_root, - state_root: bdata.data.message.state_root, - body_root: bdata.data.message.body.hash_tree_root() + slot: bdata.data.phase0Block.message.slot, + proposer_index: bdata.data.phase0Block.message.proposer_index, + parent_root: bdata.data.phase0Block.message.parent_root, + state_root: bdata.data.phase0Block.message.state_root, + body_root: bdata.data.phase0Block.message.body.hash_tree_root() ), - signature: bdata.data.signature + signature: bdata.data.phase0Block.signature ) ) ) @@ -606,14 +610,14 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = return RestApiResponse.jsonError(Http503, BeaconNodeInSyncError) if head.slot >= blck.message.slot: - # TODO altair-transition + # TODO altair-transition, but not for immediate testnet-priority let blocksTopic = getBeaconBlocksTopic(node.dag.forkDigests.phase0) node.network.broadcast(blocksTopic, blck) return RestApiResponse.jsonError(Http202, BlockValidationError) else: let res = await proposeSignedBlock(node, head, AttachedValidator(), blck) if res == head: - # TODO altair-transition + # TODO altair-transition, but not for immediate testnet-priority let blocksTopic = getBeaconBlocksTopic(node.dag.forkDigests.phase0) node.network.broadcast(blocksTopic, blck) return RestApiResponse.jsonError(Http202, BlockValidationError) @@ -632,7 +636,8 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = if res.isErr(): return RestApiResponse.jsonError(Http404, BlockNotFoundError) res.get() - return RestApiResponse.jsonResponse(bdata.data) + static: doAssert bdata.data.phase0Block is TrustedSignedBeaconBlock + return RestApiResponse.jsonResponse(bdata.data.phase0Block) # https://ethereum.github.io/eth2.0-APIs/#/Beacon/getBlockRoot router.api(MethodGet, "/api/eth/v1/beacon/blocks/{block_id}/root") do ( @@ -646,7 +651,8 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = if res.isErr(): return RestApiResponse.jsonError(Http404, BlockNotFoundError) res.get() - return RestApiResponse.jsonResponse((root: bdata.data.root)) + # TODO check whether block is altair, and if so, return error + return RestApiResponse.jsonResponse((root: bdata.data.phase0Block.root)) # https://ethereum.github.io/eth2.0-APIs/#/Beacon/getBlockAttestations router.api(MethodGet, @@ -661,8 +667,9 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = if res.isErr(): return RestApiResponse.jsonError(Http404, BlockNotFoundError) res.get() + # TODO check whether block is altair, and if so, return error return RestApiResponse.jsonResponse( - bdata.data.message.body.attestations.asSeq() + bdata.data.phase0Block.message.body.attestations.asSeq() ) # https://ethereum.github.io/eth2.0-APIs/#/Beacon/getPoolAttestations diff --git a/beacon_chain/rpc/validator_api.nim b/beacon_chain/rpc/validator_api.nim index b914269c9..3df6f6d0a 100644 --- a/beacon_chain/rpc/validator_api.nim +++ b/beacon_chain/rpc/validator_api.nim @@ -56,7 +56,8 @@ 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 == await proposeSignedBlock(node, head, AttachedValidator(), body): + if head == await proposeSignedBlock( + node, head, AttachedValidator(), body): raise newException(CatchableError, "Could not propose block") return true @@ -79,7 +80,8 @@ proc installValidatorApiHandlers*(rpcServer: RpcServer, node: BeaconNode) {. rpcServer.rpc("post_v1_validator_aggregate_and_proofs") do ( payload: SignedAggregateAndProof) -> bool: debug "post_v1_validator_aggregate_and_proofs" - node.network.broadcast(node.topicAggregateAndProofs, payload) + node.network.broadcast( + getAggregateAndProofsTopic(node.dag.forkDigests.phase0), payload) notice "Aggregated attestation sent", attestation = shortLog(payload.message.aggregate) diff --git a/beacon_chain/rpc/validator_rest_api.nim b/beacon_chain/rpc/validator_rest_api.nim index 37da6a319..53778f424 100644 --- a/beacon_chain/rpc/validator_rest_api.nim +++ b/beacon_chain/rpc/validator_rest_api.nim @@ -288,7 +288,8 @@ proc installValidatorApiHandlers*(router: var RestRouter, node: BeaconNode) = return RestApiResponse.jsonError(Http400, AggregateAndProofValidationError, $res.error()) - node.network.broadcast(node.topicAggregateAndProofs, item) + node.network.broadcast( + getAggregateAndProofsTopic(node.dag.forkDigests.phase0), item) return RestApiResponse.jsonMsgResponse(AggregateAndProofValidationSuccess) diff --git a/beacon_chain/spec/forkedbeaconstate_helpers.nim b/beacon_chain/spec/forkedbeaconstate_helpers.nim index 75c668208..33154c5ba 100644 --- a/beacon_chain/spec/forkedbeaconstate_helpers.nim +++ b/beacon_chain/spec/forkedbeaconstate_helpers.nim @@ -51,6 +51,16 @@ type ForkDigestsRef* = ref ForkDigests +template init*(T: type ForkedSignedBeaconBlock, blck: phase0.SignedBeaconBlock): T = + T(kind: BeaconBlockFork.Phase0, phase0Block: blck) +template init*(T: type ForkedSignedBeaconBlock, blck: altair.SignedBeaconBlock): T = + T(kind: BeaconBlockFork.Altair, altairBlock: blck) + +template init*(T: type ForkedTrustedSignedBeaconBlock, blck: phase0.TrustedSignedBeaconBlock): T = + T(kind: BeaconBlockFork.Phase0, phase0Block: blck) +template init*(T: type ForkedTrustedSignedBeaconBlock, blck: altair.TrustedSignedBeaconBlock): T = + T(kind: BeaconBlockFork.Altair, altairBlock: blck) + # State-related functionality based on ForkedHashedBeaconState instead of BeaconState # Dispatch functions @@ -219,21 +229,52 @@ func init*(T: type ForkDigests, template asSigned*(x: phase0.TrustedSignedBeaconBlock or phase0.SigVerifiedBeaconBlock): phase0.SignedBeaconBlock = - static: # TODO See isomorphicCast - doAssert sizeof(x) == sizeof(phase0.SignedBeaconBlock) - - cast[ptr phase0.SignedBeaconBlock](x.unsafeAddr)[] + isomorphicCast[phase0.SignedBeaconBlock](x) template asSigned*(x: altair.TrustedSignedBeaconBlock or altair.SigVerifiedBeaconBlock): altair.SignedBeaconBlock = - static: # TODO See isomorphicCast - doAssert sizeof(x) == sizeof(altair.SignedBeaconBlock) - - cast[ptr altair.SignedBeaconBlock](x.unsafeAddr)[] + isomorphicCast[altair.SignedBeaconBlock](x) template asSigned*(x: ForkedTrustedSignedBeaconBlock): ForkedSignedBeaconBlock = - static: # TODO See isomorphicCast - doAssert sizeof(x) == sizeof(ForkedSignedBeaconBlock) + isomorphicCast[ForkedSignedBeaconBlock](x) - cast[ptr ForkedSignedBeaconBlock](x.unsafeAddr)[] +template asTrusted*(x: phase0.SignedBeaconBlock or phase0.SigVerifiedBeaconBlock): + phase0.TrustedSignedBeaconBlock = + isomorphicCast[phase0.TrustedSignedBeaconBlock](x) +template asTrusted*(x: altair.SignedBeaconBlock or altair.SigVerifiedBeaconBlock): + altair.TrustedSignedBeaconBlock = + isomorphicCast[altair.TrustedSignedBeaconBlock](x) + +template asTrusted*(x: ForkedSignedBeaconBlock): ForkedSignedBeaconBlock = + isomorphicCast[ForkedTrustedSignedBeaconBlock](x) + +template withBlck*(x: ForkedSignedBeaconBlock | ForkedTrustedSignedBeaconBlock, body: untyped): untyped = + case x.kind + of BeaconBlockFork.Phase0: + template blck: untyped = x.phase0Block + body + of BeaconBlockFork.Altair: + template blck: untyped = x.altairBlock + body + +template getForkedBlockField*(x: ForkedSignedBeaconBlock | ForkedTrustedSignedBeaconBlock, y: untyped): untyped = + withBlck(x): blck.message.y + +template signature*(x: ForkedSignedBeaconBlock): ValidatorSig = + withBlck(x): blck.signature + +template signature*(x: ForkedTrustedSignedBeaconBlock): TrustedSig = + withBlck(x): blck.signature + +template root*(x: ForkedSignedBeaconBlock | ForkedTrustedSignedBeaconBlock): Eth2Digest = + withBlck(x): blck.root + +template slot*(x: ForkedSignedBeaconBlock | ForkedTrustedSignedBeaconBlock): Slot = + getForkedBlockField(x, slot) + +func shortLog*(x: ForkedSignedBeaconBlock | ForkedTrustedSignedBeaconBlock): auto = + withBlck(x): shortLog(blck.message) + +chronicles.formatIt ForkedSignedBeaconBlock: it.shortLog +chronicles.formatIt ForkedTrustedSignedBeaconBlock: it.shortLog diff --git a/beacon_chain/spec/state_transition.nim b/beacon_chain/spec/state_transition.nim index 80977b634..59d31a292 100644 --- a/beacon_chain/spec/state_transition.nim +++ b/beacon_chain/spec/state_transition.nim @@ -225,7 +225,7 @@ proc state_transition_block_aux( state: var SomeHashedBeaconState, signedBlock: phase0.SignedBeaconBlock | phase0.SigVerifiedSignedBeaconBlock | phase0.TrustedSignedBeaconBlock | altair.SignedBeaconBlock | - altair.SigVerifiedSignedBeaconBlock, + altair.SigVerifiedSignedBeaconBlock | altair.TrustedSignedBeaconBlock, cache: var StateCache, flags: UpdateFlags): bool {.nbench.} = # Block updates - these happen when there's a new block being suggested # by the block proposer. Every actor in the network will update its state @@ -274,7 +274,8 @@ proc state_transition_block*( state: var ForkedHashedBeaconState, signedBlock: phase0.SignedBeaconBlock | phase0.SigVerifiedSignedBeaconBlock | phase0.TrustedSignedBeaconBlock | - altair.SignedBeaconBlock | altair.SigVerifiedSignedBeaconBlock, + altair.SignedBeaconBlock | altair.SigVerifiedSignedBeaconBlock | + altair.TrustedSignedBeaconBlock, cache: var StateCache, flags: UpdateFlags, rollback: RollbackForkedHashedProc): bool {.nbench.} = ## `rollback` is called if the transition fails and the given state has been @@ -303,7 +304,8 @@ proc state_transition*( cfg: RuntimeConfig, state: var ForkedHashedBeaconState, signedBlock: phase0.SignedBeaconBlock | phase0.SigVerifiedSignedBeaconBlock | - phase0.TrustedSignedBeaconBlock | altair.SignedBeaconBlock, + phase0.TrustedSignedBeaconBlock | altair.SignedBeaconBlock | + altair.TrustedSignedBeaconBlock, cache: var StateCache, rewards: var RewardInfo, flags: UpdateFlags, rollback: RollbackForkedHashedProc): bool {.nbench.} = ## Apply a block to the state, advancing the slot counter as necessary. The diff --git a/beacon_chain/sync/sync_protocol.nim b/beacon_chain/sync/sync_protocol.nim index 5ef3f5fdf..ba14fc3ab 100644 --- a/beacon_chain/sync/sync_protocol.nim +++ b/beacon_chain/sync/sync_protocol.nim @@ -230,7 +230,8 @@ p2pProtocol BeaconSync(version = 1, trace "wrote response block", slot = blocks[i].slot, roor = shortLog(blocks[i].root) let blk = dag.get(blocks[i]).data - await response.write(blk.asSigned) + # TODO Altair + await response.write(blk.phase0Block.asSigned) debug "Block range request done", peer, startSlot, count, reqStep, found = count - startIndex @@ -259,7 +260,8 @@ p2pProtocol BeaconSync(version = 1, let blockRef = dag.getRef(blockRoots[i]) if not isNil(blockRef): let blk = dag.get(blockRef).data - await response.write(blk.asSigned) + # TODO Altair + await response.write(blk.phase0Block.asSigned) inc found peer.updateRequestQuota(found.float * blockResponseCost) diff --git a/beacon_chain/validators/validator_duties.nim b/beacon_chain/validators/validator_duties.nim index 855c3ac39..5ea63b720 100644 --- a/beacon_chain/validators/validator_duties.nim +++ b/beacon_chain/validators/validator_duties.nim @@ -367,7 +367,8 @@ proc proposeSignedBlock*(node: BeaconNode, if node.config.dumpEnabled: dump(node.config.dumpDirOutgoing, newBlock) - node.network.broadcast(node.topicBeaconBlocks, newBlock) + node.network.broadcast( + getBeaconBlocksTopic(node.dag.forkDigests.phase0), newBlock) beacon_blocks_proposed.inc() @@ -574,7 +575,8 @@ proc broadcastAggregatedAttestations( var signedAP = SignedAggregateAndProof( message: aggregateAndProof.get, signature: sig) - node.network.broadcast(node.topicAggregateAndProofs, signedAP) + node.network.broadcast( + getAggregateAndProofsTopic(node.dag.forkDigests.phase0), signedAP) notice "Aggregated attestation sent", attestation = shortLog(signedAP.message.aggregate), validator = shortLog(curr[0].v), diff --git a/tests/test_block_pool.nim b/tests/test_block_pool.nim index ba68a4408..b85d0c26d 100644 --- a/tests/test_block_pool.nim +++ b/tests/test_block_pool.nim @@ -514,7 +514,7 @@ suite "chain DAG finalization tests" & preset(): assign(tmpStateData[], dag.headState) dag.updateStateData(tmpStateData[], cur.atSlot(cur.slot), false, cache) check: - dag.get(cur).data.message.state_root == getStateRoot(tmpStateData[].data) + dag.get(cur).data.phase0Block.message.state_root == getStateRoot(tmpStateData[].data) getStateRoot(tmpStateData[].data) == hash_tree_root(tmpStateData[].data) cur = cur.parent