From 57519bebacc7776e18f81c653af6eccafc9e1a39 Mon Sep 17 00:00:00 2001 From: Dustin Brody Date: Thu, 14 May 2020 13:19:10 +0200 Subject: [PATCH] remove some unused imports, add tests for pubsub topics, and subscribe to interop attestations --- AllTests-mainnet.md | 7 +-- AllTests-minimal.md | 7 +-- beacon_chain/beacon_node.nim | 64 +++++++++++++++++++--------- beacon_chain/inspector.nim | 8 ++-- beacon_chain/spec/network.nim | 21 +++++++-- beacon_chain/validator_duties.nim | 10 ++--- ncli/ncli_hash_tree_root.nim | 2 +- tests/test_honest_validator.nim | 71 ++++++++++++++++++++++--------- 8 files changed, 128 insertions(+), 62 deletions(-) diff --git a/AllTests-mainnet.md b/AllTests-mainnet.md index 7f7607247..6d89a556a 100644 --- a/AllTests-mainnet.md +++ b/AllTests-mainnet.md @@ -67,9 +67,10 @@ OK: 2/2 Fail: 0/2 Skip: 0/2 OK: 4/4 Fail: 0/4 Skip: 0/4 ## Honest validator ```diff -+ Attestation topics OK ++ General pubsub topics: OK ++ Mainnet attestation topics OK ``` -OK: 1/1 Fail: 0/1 Skip: 0/1 +OK: 2/2 Fail: 0/2 Skip: 0/2 ## Interop ```diff + Interop genesis OK @@ -241,4 +242,4 @@ OK: 4/4 Fail: 0/4 Skip: 0/4 OK: 8/8 Fail: 0/8 Skip: 0/8 ---TOTAL--- -OK: 148/151 Fail: 3/151 Skip: 0/151 +OK: 149/152 Fail: 3/152 Skip: 0/152 diff --git a/AllTests-minimal.md b/AllTests-minimal.md index 800b8b62b..9b14af405 100644 --- a/AllTests-minimal.md +++ b/AllTests-minimal.md @@ -84,9 +84,10 @@ OK: 2/2 Fail: 0/2 Skip: 0/2 OK: 4/4 Fail: 0/4 Skip: 0/4 ## Honest validator ```diff -+ Attestation topics OK ++ General pubsub topics: OK ++ Mainnet attestation topics OK ``` -OK: 1/1 Fail: 0/1 Skip: 0/1 +OK: 2/2 Fail: 0/2 Skip: 0/2 ## Interop ```diff + Interop genesis OK @@ -258,4 +259,4 @@ OK: 4/4 Fail: 0/4 Skip: 0/4 OK: 8/8 Fail: 0/8 Skip: 0/8 ---TOTAL--- -OK: 157/160 Fail: 3/160 Skip: 0/160 +OK: 158/161 Fail: 3/161 Skip: 0/161 diff --git a/beacon_chain/beacon_node.nim b/beacon_chain/beacon_node.nim index 666531041..3c5d64940 100644 --- a/beacon_chain/beacon_node.nim +++ b/beacon_chain/beacon_node.nim @@ -666,6 +666,48 @@ proc installRpcHandlers(rpcServer: RpcServer, node: BeaconNode) = rpcServer.installBeaconApiHandlers(node) rpcServer.installDebugApiHandlers(node) +proc installAttestationHandlers(node: BeaconNode) = + proc attestationHandler(attestation: Attestation) = + # Avoid double-counting attestation-topic attestations on shared codepath + # when they're reflected through beacon blocks + beacon_attestations_received.inc() + beacon_attestation_received_seconds_from_slot_start.observe( + node.beaconClock.now.int64 - + (attestation.data.slot.int64 * SECONDS_PER_SLOT.int64)) + + node.onAttestation(attestation) + + var attestationSubscriptions: seq[Future[void]] = @[] + + # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#mainnet-3 + for it in 0'u64 ..< ATTESTATION_SUBNET_COUNT.uint64: + closureScope: + let ci = it + attestationSubscriptions.add(node.network.subscribe( + getMainnetAttestationTopic(node.forkDigest, ci), attestationHandler, + # This proc needs to be within closureScope; don't lift out of loop. + proc(attestation: Attestation): bool = + # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#attestation-subnets + let (afterGenesis, slot) = node.beaconClock.now().toSlot() + if not afterGenesis: + return false + node.attestationPool.isValidAttestation(attestation, slot, ci, {}))) + + # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#interop-3 + attestationSubscriptions.add(node.network.subscribe( + getInteropAttestationTopic(node.forkDigest), attestationHandler, + proc(attestation: Attestation): bool = + # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#attestation-subnets + let (afterGenesis, slot) = node.beaconClock.now().toSlot() + if not afterGenesis: + return false + # isValidAttestation checks attestation.data.index == topicCommitteeIndex + # which doesn't make sense here, so rig that check to vacuously pass. + node.attestationPool.isValidAttestation( + attestation, slot, attestation.data.index, {}))) + + waitFor allFutures(attestationSubscriptions) + proc run*(node: BeaconNode) = if node.rpcServer != nil: node.rpcServer.installRpcHandlers(node) @@ -679,27 +721,7 @@ proc run*(node: BeaconNode) = return false node.blockPool.isValidBeaconBlock(signedBlock, slot, {}) - proc attestationHandler(attestation: Attestation) = - # Avoid double-counting attestation-topic attestations on shared codepath - # when they're reflected through beacon blocks - beacon_attestations_received.inc() - beacon_attestation_received_seconds_from_slot_start.observe(node.beaconClock.now.int64 - (attestation.data.slot.int64 * SECONDS_PER_SLOT.int64)) - - node.onAttestation(attestation) - - var attestationSubscriptions: seq[Future[void]] = @[] - for it in 0'u64 ..< ATTESTATION_SUBNET_COUNT.uint64: - closureScope: - let ci = it - attestationSubscriptions.add(node.network.subscribe( - getAttestationTopic(node.forkDigest, ci), attestationHandler, - proc(attestation: Attestation): bool = - # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#attestation-subnets - let (afterGenesis, slot) = node.beaconClock.now().toSlot() - if not afterGenesis: - return false - node.attestationPool.isValidAttestation(attestation, slot, ci, {}))) - waitFor allFutures(attestationSubscriptions) + installAttestationHandlers(node) let t = node.beaconClock.now().toSlot() diff --git a/beacon_chain/inspector.nim b/beacon_chain/inspector.nim index 79b7a96ba..19dec979b 100644 --- a/beacon_chain/inspector.nim +++ b/beacon_chain/inspector.nim @@ -204,8 +204,8 @@ func getTopics(forkDigest: ForkDigest, var topics = newSeq[string](ATTESTATION_SUBNET_COUNT * 2) var offset = 0 for i in 0'u64 ..< ATTESTATION_SUBNET_COUNT.uint64: - topics[offset] = getAttestationTopic(forkDigest, i) - topics[offset + 1] = getAttestationTopic(forkDigest, i) & "_snappy" + topics[offset] = getMainnetAttestationTopic(forkDigest, i) + topics[offset + 1] = getMainnetAttestationTopic(forkDigest, i) & "_snappy" offset += 2 topics @@ -514,8 +514,8 @@ proc pubsubLogger(conf: InspectorConf, switch: Switch, if topic.endsWith(topicBeaconBlocksSuffix) or topic.endsWith(topicBeaconBlocksSuffix & "_snappy"): info "SignedBeaconBlock", msg = SSZ.decode(buffer, SignedBeaconBlock) - elif topic.endsWith(topicAttestationsSuffix) or - topic.endsWith(topicAttestationsSuffix & "_snappy"): + elif topic.endsWith(topicMainnetAttestationsSuffix) or + topic.endsWith(topicMainnetAttestationsSuffix & "_snappy"): info "Attestation", msg = SSZ.decode(buffer, Attestation) elif topic.endsWith(topicVoluntaryExitsSuffix) or topic.endsWith(topicVoluntaryExitsSuffix & "_snappy"): diff --git a/beacon_chain/spec/network.nim b/beacon_chain/spec/network.nim index 54cecb990..57d25bc51 100644 --- a/beacon_chain/spec/network.nim +++ b/beacon_chain/spec/network.nim @@ -13,10 +13,11 @@ import const topicBeaconBlocksSuffix* = "beacon_block/ssz" - topicAttestationsSuffix* = "_beacon_attestation/ssz" + topicMainnetAttestationsSuffix* = "_beacon_attestation/ssz" topicVoluntaryExitsSuffix* = "voluntary_exit/ssz" topicProposerSlashingsSuffix* = "proposer_slashing/ssz" topicAttesterSlashingsSuffix* = "attester_slashing/ssz" + topicInteropAttestationSuffix* = "beacon_attestation/ssz" topicAggregateAndProofsSuffix* = "beacon_aggregate_and_proof/ssz" # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#configuration @@ -27,40 +28,52 @@ const # This is not part of the spec yet! defaultEth2RpcPort* = 9090 +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#topics-and-messages func getBeaconBlocksTopic*(forkDigest: ForkDigest): string = try: &"/eth2/{$forkDigest}/{topicBeaconBlocksSuffix}" except ValueError as e: raiseAssert e.msg +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#topics-and-messages func getVoluntaryExitsTopic*(forkDigest: ForkDigest): string = try: &"/eth2/{$forkDigest}/{topicVoluntaryExitsSuffix}" except ValueError as e: raiseAssert e.msg +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#topics-and-messages func getProposerSlashingsTopic*(forkDigest: ForkDigest): string = try: &"/eth2/{$forkDigest}/{topicProposerSlashingsSuffix}" except ValueError as e: raiseAssert e.msg +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#topics-and-messages func getAttesterSlashingsTopic*(forkDigest: ForkDigest): string = try: &"/eth2/{$forkDigest}/{topicAttesterSlashingsSuffix}" except ValueError as e: raiseAssert e.msg +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#interop-3 +func getInteropAttestationTopic*(forkDigest: ForkDigest): string = + try: + &"/eth2/{$forkDigest}/{topicInteropAttestationSuffix}" + except ValueError as e: + raiseAssert e.msg + +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#topics-and-messages func getAggregateAndProofsTopic*(forkDigest: ForkDigest): string = try: &"/eth2/{$forkDigest}/{topicAggregateAndProofsSuffix}" except ValueError as e: raiseAssert e.msg -func getAttestationTopic*(forkDigest: ForkDigest, committeeIndex: uint64): string = - # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/validator.md#broadcast-attestation +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#mainnet-3 +func getMainnetAttestationTopic*(forkDigest: ForkDigest, committeeIndex: uint64): string = try: let topicIndex = committeeIndex mod ATTESTATION_SUBNET_COUNT - &"/eth2/{$forkDigest}/committee_index{topicIndex}{topicAttestationsSuffix}" + &"/eth2/{$forkDigest}/committee_index{topicIndex}{topicMainnetAttestationsSuffix}" except ValueError as e: raiseAssert e.msg diff --git a/beacon_chain/validator_duties.nim b/beacon_chain/validator_duties.nim index 79210760a..f806c075a 100644 --- a/beacon_chain/validator_duties.nim +++ b/beacon_chain/validator_duties.nim @@ -14,17 +14,16 @@ import chronos, metrics, json_rpc/[rpcserver, jsonmarshal], chronicles, json_serialization/std/[options, sets, net], serialization/errors, - eth/db/kvstore, eth/db/kvstore_sqlite3, + eth/db/kvstore, eth/[keys, async_utils], eth/p2p/discoveryv5/[protocol, enr], # Local modules spec/[datatypes, digest, crypto, beaconstate, helpers, validator, network, state_transition_block], - conf, time, beacon_chain_db, validator_pool, + conf, time, validator_pool, attestation_pool, block_pool, eth2_network, beacon_node_common, beacon_node_types, - mainchain_monitor, version, ssz, ssz/dynamic_navigator, - request_manager, interop, statusbar, + mainchain_monitor, version, ssz, interop, attestation_aggregation, sync_manager, sszdump # Metrics for tracking attestation and beacon block loss @@ -124,7 +123,8 @@ proc sendAttestation(node: BeaconNode, # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/validator.md#broadcast-attestation node.network.broadcast( - getAttestationTopic(node.forkDigest, attestationData.index), attestation) + getMainnetAttestationTopic(node.forkDigest, attestationData.index), + attestation) if node.config.dumpEnabled: dump(node.config.dumpDir, attestationData, validator.pubKey) diff --git a/ncli/ncli_hash_tree_root.nim b/ncli/ncli_hash_tree_root.nim index 55516de78..e8dd85c4f 100644 --- a/ncli/ncli_hash_tree_root.nim +++ b/ncli/ncli_hash_tree_root.nim @@ -1,5 +1,5 @@ import - confutils, os, strutils, chronicles, json_serialization, + confutils, os, strutils, json_serialization, stew/byteutils, ../beacon_chain/spec/[crypto, datatypes, digest], ../beacon_chain/ssz diff --git a/tests/test_honest_validator.nim b/tests/test_honest_validator.nim index ba9f4dbe8..a020a4657 100644 --- a/tests/test_honest_validator.nim +++ b/tests/test_honest_validator.nim @@ -7,25 +7,54 @@ import suiteReport "Honest validator": var forkDigest: ForkDigest - timedTest "Attestation topics": + timedTest "General pubsub topics:": check: - getAttestationTopic(forkDigest, 0) == "/eth2/00000000/committee_index0_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 9) == "/eth2/00000000/committee_index9_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 10) == "/eth2/00000000/committee_index10_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 11) == "/eth2/00000000/committee_index11_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 14) == "/eth2/00000000/committee_index14_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 22) == "/eth2/00000000/committee_index22_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 34) == "/eth2/00000000/committee_index34_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 46) == "/eth2/00000000/committee_index46_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 60) == "/eth2/00000000/committee_index60_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 63) == "/eth2/00000000/committee_index63_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 200) == "/eth2/00000000/committee_index8_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 400) == "/eth2/00000000/committee_index16_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 469) == "/eth2/00000000/committee_index21_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 550) == "/eth2/00000000/committee_index38_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 600) == "/eth2/00000000/committee_index24_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 613) == "/eth2/00000000/committee_index37_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 733) == "/eth2/00000000/committee_index29_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 775) == "/eth2/00000000/committee_index7_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 888) == "/eth2/00000000/committee_index56_beacon_attestation/ssz" - getAttestationTopic(forkDigest, 995) == "/eth2/00000000/committee_index35_beacon_attestation/ssz" + getBeaconBlocksTopic(forkDigest) == "/eth2/00000000/beacon_block/ssz" + getVoluntaryExitsTopic(forkDigest) == "/eth2/00000000/voluntary_exit/ssz" + getProposerSlashingsTopic(forkDigest) == "/eth2/00000000/proposer_slashing/ssz" + getAttesterSlashingsTopic(forkDigest) == "/eth2/00000000/attester_slashing/ssz" + getInteropAttestationTopic(forkDigest) == "/eth2/00000000/beacon_attestation/ssz" + getAggregateAndProofsTopic(forkDigest) == "/eth2/00000000/beacon_aggregate_and_proof/ssz" + + timedTest "Mainnet attestation topics": + check: + getMainnetAttestationTopic(forkDigest, 0) == + "/eth2/00000000/committee_index0_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 9) == + "/eth2/00000000/committee_index9_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 10) == + "/eth2/00000000/committee_index10_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 11) == + "/eth2/00000000/committee_index11_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 14) == + "/eth2/00000000/committee_index14_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 22) == + "/eth2/00000000/committee_index22_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 34) == + "/eth2/00000000/committee_index34_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 46) == + "/eth2/00000000/committee_index46_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 60) == + "/eth2/00000000/committee_index60_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 63) == + "/eth2/00000000/committee_index63_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 200) == + "/eth2/00000000/committee_index8_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 400) == + "/eth2/00000000/committee_index16_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 469) == + "/eth2/00000000/committee_index21_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 550) == + "/eth2/00000000/committee_index38_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 600) == + "/eth2/00000000/committee_index24_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 613) == + "/eth2/00000000/committee_index37_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 733) == + "/eth2/00000000/committee_index29_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 775) == + "/eth2/00000000/committee_index7_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 888) == + "/eth2/00000000/committee_index56_beacon_attestation/ssz" + getMainnetAttestationTopic(forkDigest, 995) == + "/eth2/00000000/committee_index35_beacon_attestation/ssz"