Add info about peer scoring to block/attestation errors (#1393)

* Add comments to `BlockError`

* Add `AttnError` comments

* Clean up

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -66,71 +66,166 @@ use types::{
 pub enum Error {
     /// The attestation is from a slot that is later than the current slot (with respect to the
     /// gossip clock disparity).
+    ///
+    /// ## Peer scoring
+    ///
+    /// Assuming the local clock is correct, the peer has sent an invalid message.
     FutureSlot {
         attestation_slot: Slot,
         latest_permissible_slot: Slot,
     },
     /// The attestation is from a slot that is prior to the earliest permissible slot (with
     /// respect to the gossip clock disparity).
+    ///
+    /// ## Peer scoring
+    ///
+    /// Assuming the local clock is correct, the peer has sent an invalid message.
     PastSlot {
         attestation_slot: Slot,
         earliest_permissible_slot: Slot,
     },
     /// The attestations aggregation bits were empty when they shouldn't be.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     EmptyAggregationBitfield,
     /// The `selection_proof` on the aggregate attestation does not elect it as an aggregator.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     InvalidSelectionProof { aggregator_index: u64 },
     /// The `selection_proof` on the aggregate attestation selects it as a validator, however the
     /// aggregator index is not in the committee for that attestation.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     AggregatorNotInCommittee { aggregator_index: u64 },
     /// The aggregator index refers to a validator index that we have not seen.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     AggregatorPubkeyUnknown(u64),
     /// The attestation has been seen before; either in a block, on the gossip network or from a
     /// local validator.
+    ///
+    /// ## Peer scoring
+    ///
+    /// It's unclear if this attestation is valid, however we have already observed it and do not
+    /// need to observe it again.
     AttestationAlreadyKnown(Hash256),
     /// There has already been an aggregation observed for this validator, we refuse to process a
     /// second.
+    ///
+    /// ## Peer scoring
+    ///
+    /// It's unclear if this attestation is valid, however we have already observed an aggregate
+    /// attestation from this validator for this epoch and should not observe another.
     AggregatorAlreadyKnown(u64),
     /// The aggregator index is higher than the maximum possible validator count.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     ValidatorIndexTooHigh(usize),
     /// The `attestation.data.beacon_block_root` block is unknown.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The attestation points to a block we have not yet imported. It's unclear if the attestation
+    /// is valid or not.
     UnknownHeadBlock { beacon_block_root: Hash256 },
-    /// The `attestation.data.slot` is not from the same epoch as `data.target.epoch` and therefore
+    /// The `attestation.data.slot` is not from the same epoch as `data.target.epoch`.
-    /// the attestation is invalid.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     BadTargetEpoch,
     /// The target root of the attestation points to a block that we have not verified.
+    ///
+    /// This is invalid behaviour whilst we first check for `UnknownHeadBlock`.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     UnknownTargetRoot(Hash256),
     /// A signature on the attestation is invalid.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     InvalidSignature,
     /// There is no committee for the slot and committee index of this attestation and the
     /// attestation should not have been produced.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     NoCommitteeForSlotAndIndex { slot: Slot, index: CommitteeIndex },
     /// The unaggregated attestation doesn't have only one aggregation bit set.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     NotExactlyOneAggregationBitSet(usize),
     /// We have already observed an attestation for the `validator_index` and refuse to process
     /// another.
+    ///
+    /// ## Peer scoring
+    ///
+    /// It's unclear if this attestation is valid, however we have already observed a
+    /// single-participant attestation from this validator for this epoch and should not observe
+    /// another.
     PriorAttestationKnown { validator_index: u64, epoch: Epoch },
     /// The attestation is for an epoch in the future (with respect to the gossip clock disparity).
+    ///
+    /// ## Peer scoring
+    ///
+    /// Assuming the local clock is correct, the peer has sent an invalid message.
     FutureEpoch {
         attestation_epoch: Epoch,
         current_epoch: Epoch,
     },
     /// The attestation is for an epoch in the past (with respect to the gossip clock disparity).
+    ///
+    /// ## Peer scoring
+    ///
+    /// Assuming the local clock is correct, the peer has sent an invalid message.
     PastEpoch {
         attestation_epoch: Epoch,
         current_epoch: Epoch,
     },
     /// The attestation is attesting to a state that is later than itself. (Viz., attesting to the
     /// future).
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     AttestsToFutureBlock { block: Slot, attestation: Slot },
     /// The attestation was received on an invalid attestation subnet.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     InvalidSubnetId {
         received: SubnetId,
         expected: SubnetId,
     },
     /// The attestation failed the `state_processing` verification stage.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
     Invalid(AttestationValidationError),
     /// There was an error whilst processing the attestation. It is not known if it is valid or invalid.
+    ///
+    /// ## Peer scoring
+    ///
+    /// We were unable to process this attestation due to an internal error. It's unclear if the
+    /// attestation is valid.
     BeaconChainError(BeaconChainError),
 }
 

beacon_node/beacon_chain/src/block_verification.rs

@@ -85,48 +85,117 @@ const WRITE_BLOCK_PROCESSING_SSZ: bool = cfg!(feature = "write_ssz_files");
 #[derive(Debug)]
 pub enum BlockError {
     /// The parent block was unknown.
+    ///
+    /// ## Peer scoring
+    ///
+    /// It's unclear if this block is valid, but it cannot be processed without already knowing
+    /// its parent.
     ParentUnknown(Hash256),
     /// The block slot is greater than the present slot.
+    ///
+    /// ## Peer scoring
+    ///
+    /// Assuming the local clock is correct, the peer has sent an invalid message.
     FutureSlot {
         present_slot: Slot,
         block_slot: Slot,
     },
     /// The block state_root does not match the generated state.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has incompatible state transition logic and is faulty.
     StateRootMismatch { block: Hash256, local: Hash256 },
     /// The block was a genesis block, these blocks cannot be re-imported.
     GenesisBlock,
     /// The slot is finalized, no need to import.
+    ///
+    /// ## Peer scoring
+    ///
+    /// It's unclear if this block is valid, but this block is for a finalized slot and is
+    /// therefore useless to us.
     WouldRevertFinalizedSlot {
         block_slot: Slot,
         finalized_slot: Slot,
     },
     /// Block is already known, no need to re-import.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The block is valid and we have already imported a block with this hash.
     BlockIsAlreadyKnown,
     /// A block for this proposer and slot has already been observed.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The `proposer` has already proposed a block at this slot. The existing block may or may not
+    /// be equal to the given block.
     RepeatProposal { proposer: u64, slot: Slot },
     /// The block slot exceeds the MAXIMUM_BLOCK_SLOT_NUMBER.
+    ///
+    /// ## Peer scoring
+    ///
+    /// We set a very, very high maximum slot number and this block exceeds it. There's no good
+    /// reason to be sending these blocks, they're from future slots.
+    ///
+    /// The block is invalid and the peer is faulty.
     BlockSlotLimitReached,
     /// The `BeaconBlock` has a `proposer_index` that does not match the index we computed locally.
     ///
-    /// The block is invalid.
+    /// ## Peer scoring
+    ///
+    /// The block is invalid and the peer is faulty.
     IncorrectBlockProposer { block: u64, local_shuffling: u64 },
     /// The proposal signature in invalid.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The block is invalid and the peer is faulty.
     ProposalSignatureInvalid,
     /// The `block.proposal_index` is not known.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The block is invalid and the peer is faulty.
     UnknownValidator(u64),
     /// A signature in the block is invalid (exactly which is unknown).
+    ///
+    /// ## Peer scoring
+    ///
+    /// The block is invalid and the peer is faulty.
     InvalidSignature,
-    /// The provided block is from an earlier slot than its parent.
+    /// The provided block is from an later slot than its parent.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The block is invalid and the peer is faulty.
     BlockIsNotLaterThanParent { block_slot: Slot, state_slot: Slot },
     /// At least one block in the chain segment did not have it's parent root set to the root of
     /// the prior block.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The chain of blocks is invalid and the peer is faulty.
     NonLinearParentRoots,
     /// The slots of the blocks in the chain segment were not strictly increasing. I.e., a child
     /// had lower slot than a parent.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The chain of blocks is invalid and the peer is faulty.
     NonLinearSlots,
     /// The block failed the specification's `per_block_processing` function, it is invalid.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The block is invalid and the peer is faulty.
     PerBlockProcessingError(BlockProcessingError),
     /// There was an error whilst processing the block. It is not necessarily invalid.
+    ///
+    /// ## Peer scoring
+    ///
+    /// We were unable to process this block due to an internal error. It's unclear if the block is
+    /// valid.
     BeaconChainError(BeaconChainError),
 }