Relax PartialEq constraint on error enums (#1179)

This commit is contained in:
Adam Szkoda 2020-05-21 02:21:44 +02:00 committed by GitHub
parent 681e013d31
commit d79e07902e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 251 additions and 187 deletions

View File

@ -62,7 +62,7 @@ use types::{
/// other than `BeaconChainError`).
/// - The application encountered an internal error whilst attempting to determine validity
/// (the `BeaconChainError` variant)
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub enum Error {
/// The attestation is from a slot that is later than the current slot (with respect to the
/// gossip clock disparity).

View File

@ -86,7 +86,7 @@ const WRITE_BLOCK_PROCESSING_SSZ: bool = cfg!(feature = "write_ssz_files");
///
/// - The block is malformed/invalid (indicated by all results other than `BeaconChainError`.
/// - We encountered an error whilst trying to verify the block (a `BeaconChainError`).
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub enum BlockError {
/// The parent block was unknown.
ParentUnknown(Hash256),

View File

@ -26,7 +26,7 @@ macro_rules! easy_from_to {
};
}
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub enum BeaconChainError {
InsufficientValidators,
UnableToReadSlot,
@ -87,7 +87,7 @@ easy_from_to!(ObservedBlockProducersError, BeaconChainError);
easy_from_to!(BlockSignatureVerifierError, BeaconChainError);
easy_from_to!(ArithError, BeaconChainError);
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub enum BlockProductionError {
UnableToGetBlockRootFromState,
UnableToReadSlot,

View File

@ -19,7 +19,7 @@ use types::{
type BlockNumber = u64;
type Eth1DataVoteCount = HashMap<(Eth1Data, BlockNumber), u64>;
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub enum Error {
/// Unable to return an Eth1Data for the given epoch.
EpochUnavailable,

View File

@ -13,7 +13,7 @@ use types::{BeaconBlock, BeaconState, BeaconStateError, Epoch, Hash256, IndexedA
type Result<T> = std::result::Result<T, Error>;
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub enum Error {
MissingBlock(Hash256),
MissingState(Hash256),

View File

@ -199,8 +199,9 @@ fn aggregated_gossip_verification() {
get_valid_aggregated_attestation(&harness.chain, valid_attestation);
macro_rules! assert_invalid {
($desc: tt, $attn_getter: expr, $error: expr) => {
assert_eq!(
($desc: tt, $attn_getter: expr, $($error: pat) |+ $( if $guard: expr )?) => {
assert!(
matches!(
harness
.chain
.verify_aggregated_attestation_for_gossip($attn_getter)
@ -209,7 +210,8 @@ fn aggregated_gossip_verification() {
"{} should error during verify_aggregated_attestation_for_gossip",
$desc
)),
$error,
$( $error ) |+ $( if $guard )?
),
"case: {}",
$desc,
);
@ -235,10 +237,8 @@ fn aggregated_gossip_verification() {
a.message.aggregate.data.slot = future_slot;
a
},
AttnError::FutureSlot {
attestation_slot: future_slot,
latest_permissible_slot: current_slot,
}
AttnError::FutureSlot { attestation_slot, latest_permissible_slot }
if attestation_slot == future_slot && latest_permissible_slot == current_slot
);
let early_slot = current_slot
@ -254,11 +254,12 @@ fn aggregated_gossip_verification() {
a
},
AttnError::PastSlot {
attestation_slot: early_slot,
attestation_slot,
// Subtract an additional slot since the harness will be exactly on the start of the
// slot and the propagation tolerance will allow an extra slot.
earliest_permissible_slot: current_slot - E::slots_per_epoch() - 1,
earliest_permissible_slot
}
if attestation_slot == early_slot && earliest_permissible_slot == current_slot - E::slots_per_epoch() - 1
);
/*
@ -278,8 +279,9 @@ fn aggregated_gossip_verification() {
a
},
AttnError::UnknownHeadBlock {
beacon_block_root: unknown_root
beacon_block_root
}
if beacon_block_root == unknown_root
);
/*
@ -379,7 +381,8 @@ fn aggregated_gossip_verification() {
a.message.aggregator_index = too_high_index;
a
},
AttnError::ValidatorIndexTooHigh(too_high_index as usize)
AttnError::ValidatorIndexTooHigh(index)
if index == too_high_index as usize
);
/*
@ -406,8 +409,9 @@ fn aggregated_gossip_verification() {
//
// However the following error is triggered first:
AttnError::AggregatorNotInCommittee {
aggregator_index: unknown_validator
aggregator_index
}
if aggregator_index == unknown_validator
);
/*
@ -436,8 +440,9 @@ fn aggregated_gossip_verification() {
)
},
AttnError::InvalidSelectionProof {
aggregator_index: non_aggregator_index as u64
aggregator_index: index
}
if index == non_aggregator_index as u64
);
assert!(
@ -464,7 +469,8 @@ fn aggregated_gossip_verification() {
assert_invalid!(
"aggregate with that has already been seen",
valid_aggregate.clone(),
AttnError::AttestationAlreadyKnown(valid_aggregate.message.aggregate.tree_hash_root())
AttnError::AttestationAlreadyKnown(hash)
if hash == valid_aggregate.message.aggregate.tree_hash_root()
);
/*
@ -483,7 +489,8 @@ fn aggregated_gossip_verification() {
a.message.aggregate.data.beacon_block_root = Hash256::from_low_u64_le(42);
a
},
AttnError::AggregatorAlreadyKnown(aggregator_index as u64)
AttnError::AggregatorAlreadyKnown(index)
if index == aggregator_index as u64
);
}
@ -512,12 +519,13 @@ fn unaggregated_gossip_verification() {
"the test requires a new epoch to avoid already-seen errors"
);
let (valid_attestation, validator_index, validator_committee_index, validator_sk) =
let (valid_attestation, expected_validator_index, validator_committee_index, validator_sk) =
get_valid_unaggregated_attestation(&harness.chain);
macro_rules! assert_invalid {
($desc: tt, $attn_getter: expr, $error: expr) => {
assert_eq!(
($desc: tt, $attn_getter: expr, $($error: pat) |+ $( if $guard: expr )?) => {
assert!(
matches!(
harness
.chain
.verify_unaggregated_attestation_for_gossip($attn_getter)
@ -526,7 +534,8 @@ fn unaggregated_gossip_verification() {
"{} should error during verify_unaggregated_attestation_for_gossip",
$desc
)),
$error,
$( $error ) |+ $( if $guard )?
),
"case: {}",
$desc,
);
@ -553,9 +562,10 @@ fn unaggregated_gossip_verification() {
a
},
AttnError::FutureSlot {
attestation_slot: future_slot,
latest_permissible_slot: current_slot,
attestation_slot,
latest_permissible_slot,
}
if attestation_slot == future_slot && latest_permissible_slot == current_slot
);
let early_slot = current_slot
@ -571,11 +581,12 @@ fn unaggregated_gossip_verification() {
a
},
AttnError::PastSlot {
attestation_slot: early_slot,
attestation_slot,
// Subtract an additional slot since the harness will be exactly on the start of the
// slot and the propagation tolerance will allow an extra slot.
earliest_permissible_slot: current_slot - E::slots_per_epoch() - 1,
earliest_permissible_slot,
}
if attestation_slot == early_slot && earliest_permissible_slot == current_slot - E::slots_per_epoch() - 1
);
/*
@ -633,8 +644,9 @@ fn unaggregated_gossip_verification() {
a
},
AttnError::UnknownHeadBlock {
beacon_block_root: unknown_root
beacon_block_root,
}
if beacon_block_root == unknown_root
);
/*
@ -681,9 +693,10 @@ fn unaggregated_gossip_verification() {
"attestation that has already been seen",
valid_attestation.clone(),
AttnError::PriorAttestationKnown {
validator_index: validator_index as u64,
epoch: current_epoch
validator_index,
epoch,
}
if validator_index == expected_validator_index as u64 && epoch == current_epoch
);
}
@ -719,7 +732,7 @@ fn fork_choice_verification() {
);
let current_slot = chain.slot().expect("should get slot");
let current_epoch = chain.epoch().expect("should get epoch");
let expected_current_epoch = chain.epoch().expect("should get epoch");
let attestation = harness
.chain
@ -727,8 +740,9 @@ fn fork_choice_verification() {
.expect("precondition: should gossip verify attestation");
macro_rules! assert_invalid {
($desc: tt, $attn_getter: expr, $error: expr) => {
assert_eq!(
($desc: tt, $attn_getter: expr, $($error: pat) |+ $( if $guard: expr )?) => {
assert!(
matches!(
harness
.chain
.apply_attestation_to_fork_choice(&$attn_getter)
@ -737,7 +751,8 @@ fn fork_choice_verification() {
"{} should error during apply_attestation_to_fork_choice",
$desc
)),
$error,
$( $error ) |+ $( if $guard )?
),
"case: {}",
$desc,
);
@ -759,10 +774,10 @@ fn fork_choice_verification() {
*
* Spec v0.11.2
*
* assert target.epoch in [current_epoch, previous_epoch]
* assert target.epoch in [expected_current_epoch, previous_epoch]
*/
let future_epoch = current_epoch + 1;
let future_epoch = expected_current_epoch + 1;
assert_invalid!(
"attestation from future epoch",
{
@ -771,17 +786,18 @@ fn fork_choice_verification() {
a
},
AttnError::FutureEpoch {
attestation_epoch: future_epoch,
current_epoch
attestation_epoch,
current_epoch,
}
if attestation_epoch == future_epoch && current_epoch == expected_current_epoch
);
assert!(
current_epoch > 1,
expected_current_epoch > 1,
"precondition: must be able to have a past epoch"
);
let past_epoch = current_epoch - 2;
let past_epoch = expected_current_epoch - 2;
assert_invalid!(
"attestation from past epoch",
{
@ -790,9 +806,10 @@ fn fork_choice_verification() {
a
},
AttnError::PastEpoch {
attestation_epoch: past_epoch,
current_epoch
attestation_epoch,
current_epoch,
}
if attestation_epoch == past_epoch && current_epoch == expected_current_epoch
);
/*
@ -836,7 +853,7 @@ fn fork_choice_verification() {
indexed.data.target.root = unknown_root;
a
},
AttnError::UnknownTargetRoot(unknown_root)
AttnError::UnknownTargetRoot(hash) if hash == unknown_root
);
// NOTE: we're not testing an assert from the spec:
@ -868,8 +885,9 @@ fn fork_choice_verification() {
a
},
AttnError::UnknownHeadBlock {
beacon_block_root: unknown_root
beacon_block_root
}
if beacon_block_root == unknown_root
);
let future_block = harness
@ -894,8 +912,9 @@ fn fork_choice_verification() {
},
AttnError::AttestsToFutureBlock {
block: current_slot,
attestation: current_slot - 1
attestation: slot,
}
if slot == current_slot - 1
);
// Note: we're not checking the "attestations can only affect the fork choice of subsequent

View File

@ -193,12 +193,14 @@ fn chain_segment_non_linear_parent_roots() {
let mut blocks = chain_segment_blocks();
blocks.remove(2);
assert_eq!(
assert!(
matches!(
harness
.chain
.process_chain_segment(blocks.clone())
.to_block_error(),
Err(BlockError::NonLinearParentRoots),
Err(BlockError::NonLinearParentRoots)
),
"should not import chain with missing parent"
);
@ -208,12 +210,14 @@ fn chain_segment_non_linear_parent_roots() {
let mut blocks = chain_segment_blocks();
blocks[3].message.parent_root = Hash256::zero();
assert_eq!(
assert!(
matches!(
harness
.chain
.process_chain_segment(blocks.clone())
.to_block_error(),
Err(BlockError::NonLinearParentRoots),
Err(BlockError::NonLinearParentRoots)
),
"should not import chain with a broken parent root link"
);
}
@ -233,12 +237,14 @@ fn chain_segment_non_linear_slots() {
let mut blocks = chain_segment_blocks();
blocks[3].message.slot = Slot::new(0);
assert_eq!(
assert!(
matches!(
harness
.chain
.process_chain_segment(blocks.clone())
.to_block_error(),
Err(BlockError::NonLinearSlots),
Err(BlockError::NonLinearSlots)
),
"should not import chain with a parent that has a lower slot than its child"
);
@ -249,12 +255,14 @@ fn chain_segment_non_linear_slots() {
let mut blocks = chain_segment_blocks();
blocks[3].message.slot = blocks[2].message.slot;
assert_eq!(
assert!(
matches!(
harness
.chain
.process_chain_segment(blocks.clone())
.to_block_error(),
Err(BlockError::NonLinearSlots),
Err(BlockError::NonLinearSlots)
),
"should not import chain with a parent that has an equal slot to its child"
);
}
@ -297,19 +305,23 @@ fn invalid_signatures() {
.collect();
// Ensure the block will be rejected if imported in a chain segment.
assert_eq!(
assert!(
matches!(
harness.chain.process_chain_segment(blocks).to_block_error(),
Err(BlockError::InvalidSignature),
Err(BlockError::InvalidSignature)
),
"should not import chain segment with an invalid {} signature",
item
);
// Ensure the block will be rejected if imported on its own (without gossip checking).
assert_eq!(
assert!(
matches!(
harness
.chain
.process_block(snapshots[block_index].beacon_block.clone()),
Err(BlockError::InvalidSignature),
Err(BlockError::InvalidSignature)
),
"should not import individual block with an invalid {} signature",
item
);
@ -332,17 +344,21 @@ fn invalid_signatures() {
.map(|snapshot| snapshot.beacon_block.clone())
.collect();
// Ensure the block will be rejected if imported in a chain segment.
assert_eq!(
assert!(
matches!(
harness.chain.process_chain_segment(blocks).to_block_error(),
Err(BlockError::InvalidSignature),
Err(BlockError::InvalidSignature)
),
"should not import chain segment with an invalid gossip signature",
);
// Ensure the block will be rejected if imported on its own (without gossip checking).
assert_eq!(
assert!(
matches!(
harness
.chain
.process_block(snapshots[block_index].beacon_block.clone()),
Err(BlockError::InvalidSignature),
Err(BlockError::InvalidSignature)
),
"should not import individual block with an invalid gossip signature",
);
@ -467,8 +483,10 @@ fn invalid_signatures() {
.map(|snapshot| snapshot.beacon_block.clone())
.collect();
assert!(
harness.chain.process_chain_segment(blocks).to_block_error()
!= Err(BlockError::InvalidSignature),
!matches!(
harness.chain.process_chain_segment(blocks).to_block_error(),
Err(BlockError::InvalidSignature)
),
"should not throw an invalid signature error for a bad deposit signature"
);
@ -543,14 +561,17 @@ fn block_gossip_verification() {
*/
let mut block = CHAIN_SEGMENT[block_index].beacon_block.clone();
let block_slot = block.message.slot + 1;
block.message.slot = block_slot;
assert_eq!(
let expected_block_slot = block.message.slot + 1;
block.message.slot = expected_block_slot;
assert!(
matches!(
unwrap_err(harness.chain.verify_block_for_gossip(block)),
BlockError::FutureSlot {
present_slot: block_slot - 1,
block_slot
},
present_slot,
block_slot,
}
if present_slot == expected_block_slot - 1 && block_slot == expected_block_slot
),
"should not import a block with a future slot"
);
@ -567,20 +588,23 @@ fn block_gossip_verification() {
*/
let mut block = CHAIN_SEGMENT[block_index].beacon_block.clone();
let finalized_slot = harness
let expected_finalized_slot = harness
.chain
.head_info()
.expect("should get head info")
.finalized_checkpoint
.epoch
.start_slot(E::slots_per_epoch());
block.message.slot = finalized_slot;
assert_eq!(
block.message.slot = expected_finalized_slot;
assert!(
matches!(
unwrap_err(harness.chain.verify_block_for_gossip(block)),
BlockError::WouldRevertFinalizedSlot {
block_slot: finalized_slot,
finalized_slot
},
block_slot,
finalized_slot,
}
if block_slot == expected_finalized_slot && finalized_slot == expected_finalized_slot
),
"should not import a block with a finalized slot"
);
@ -595,9 +619,11 @@ fn block_gossip_verification() {
let mut block = CHAIN_SEGMENT[block_index].beacon_block.clone();
block.signature = junk_signature();
assert_eq!(
assert!(
matches!(
unwrap_err(harness.chain.verify_block_for_gossip(block)),
BlockError::ProposalSignatureInvalid,
BlockError::ProposalSignatureInvalid
),
"should not import a block with an invalid proposal signature"
);
@ -625,21 +651,27 @@ fn block_gossip_verification() {
harness.chain.genesis_validators_root,
&harness.chain.spec,
);
assert_eq!(
assert!(
matches!(
unwrap_err(harness.chain.verify_block_for_gossip(block.clone())),
BlockError::IncorrectBlockProposer {
block: other_proposer,
local_shuffling: expected_proposer
},
block,
local_shuffling,
}
if block == other_proposer && local_shuffling == expected_proposer
),
"should not import a block with the wrong proposer index"
);
// Check to ensure that we registered this is a valid block from this proposer.
assert_eq!(
assert!(
matches!(
unwrap_err(harness.chain.verify_block_for_gossip(block.clone())),
BlockError::RepeatProposal {
proposer: other_proposer,
slot: block.message.slot
},
proposer,
slot,
}
if proposer == other_proposer && slot == block.message.slot
),
"should register any valid signature against the proposer, even if the block failed later verification"
);
@ -659,16 +691,19 @@ fn block_gossip_verification() {
*/
let block = CHAIN_SEGMENT[block_index].beacon_block.clone();
assert_eq!(
assert!(
matches!(
harness
.chain
.verify_block_for_gossip(block.clone())
.err()
.expect("should error when processing known block"),
BlockError::RepeatProposal {
proposer: block.message.proposer_index,
slot: block.message.slot,
},
proposer,
slot,
}
if proposer == block.message.proposer_index && slot == block.message.slot
),
"the second proposal by this validator should be rejected"
);
}

View File

@ -143,7 +143,11 @@ fn finalizes_after_resuming_from_db() {
fn assert_chains_pretty_much_the_same<T: BeaconChainTypes>(a: &BeaconChain<T>, b: &BeaconChain<T>) {
assert_eq!(a.spec, b.spec, "spec should be equal");
assert_eq!(a.op_pool, b.op_pool, "op_pool should be equal");
assert_eq!(a.head(), b.head(), "head() should be equal");
assert_eq!(
a.head().unwrap(),
b.head().unwrap(),
"head() should be equal"
);
assert_eq!(a.heads(), b.heads(), "heads() should be equal");
assert_eq!(
a.genesis_block_root, b.genesis_block_root,

View File

@ -317,21 +317,22 @@ fn epoch_boundary_state_attestation_processing() {
.verify_unaggregated_attestation_for_gossip(attestation.clone());
let current_slot = harness.chain.slot().expect("should get slot");
let attestation_slot = attestation.data.slot;
let expected_attestation_slot = attestation.data.slot;
// Extra -1 to handle gossip clock disparity.
let earliest_permissible_slot = current_slot - E::slots_per_epoch() - 1;
let expected_earliest_permissible_slot = current_slot - E::slots_per_epoch() - 1;
if attestation_slot <= finalized_epoch.start_slot(E::slots_per_epoch())
|| attestation_slot < earliest_permissible_slot
if expected_attestation_slot <= finalized_epoch.start_slot(E::slots_per_epoch())
|| expected_attestation_slot < expected_earliest_permissible_slot
{
checked_pre_fin = true;
assert_eq!(
assert!(matches!(
res.err().unwrap(),
AttnError::PastSlot {
attestation_slot,
earliest_permissible_slot,
}
);
if attestation_slot == expected_attestation_slot && earliest_permissible_slot == expected_earliest_permissible_slot
));
} else {
res.expect("should have verified attetation");
}
@ -397,13 +398,13 @@ fn delete_blocks_and_states() {
// Delete faulty fork
// Attempting to load those states should find them unavailable
for (state_root, slot) in &states_to_delete {
assert_eq!(store.delete_state(state_root, *slot), Ok(()));
assert_eq!(store.get_state(state_root, Some(*slot)), Ok(None));
store.delete_state(state_root, *slot).unwrap();
assert_eq!(store.get_state(state_root, Some(*slot)).unwrap(), None);
}
// Double-deleting should also be OK (deleting non-existent things is fine)
for (state_root, slot) in &states_to_delete {
assert_eq!(store.delete_state(state_root, *slot), Ok(()));
store.delete_state(state_root, *slot).unwrap();
}
// Deleting the blocks from the fork should remove them completely
@ -413,8 +414,8 @@ fn delete_blocks_and_states() {
.collect::<Vec<_>>();
for (block_root, _) in blocks_to_delete {
assert_eq!(store.delete_block(&block_root), Ok(()));
assert_eq!(store.get_block(&block_root), Ok(None));
store.delete_block(&block_root).unwrap();
assert_eq!(store.get_block(&block_root).unwrap(), None);
}
// Deleting frozen states should do nothing
@ -426,7 +427,7 @@ fn delete_blocks_and_states() {
.filter(|(_, slot)| *slot < split_slot);
for (state_root, slot) in finalized_states {
assert_eq!(store.delete_state(&state_root, slot), Ok(()));
store.delete_state(&state_root, slot).unwrap();
}
// After all that, the chain dump should still be OK

View File

@ -453,17 +453,19 @@ fn attestations_with_increasing_slots() {
.verify_unaggregated_attestation_for_gossip(attestation.clone());
let current_slot = harness.chain.slot().expect("should get slot");
let attestation_slot = attestation.data.slot;
let earliest_permissible_slot = current_slot - MinimalEthSpec::slots_per_epoch() - 1;
let expected_attestation_slot = attestation.data.slot;
let expected_earliest_permissible_slot =
current_slot - MinimalEthSpec::slots_per_epoch() - 1;
if attestation_slot < earliest_permissible_slot {
assert_eq!(
if expected_attestation_slot < expected_earliest_permissible_slot {
assert!(matches!(
res.err().unwrap(),
AttnError::PastSlot {
attestation_slot,
earliest_permissible_slot,
}
)
if attestation_slot == expected_attestation_slot && earliest_permissible_slot == expected_earliest_permissible_slot
))
} else {
res.expect("should process attestation");
}
@ -558,19 +560,22 @@ fn run_skip_slot_test(skip_slots: u64) {
);
assert_eq!(
harness_b.chain.process_block(
harness_b
.chain
.process_block(
harness_a
.chain
.head()
.expect("should get head")
.beacon_block
.clone(),
),
Ok(harness_a
)
.unwrap(),
harness_a
.chain
.head()
.expect("should get head")
.beacon_block_root)
.beacon_block_root
);
harness_b

View File

@ -3,7 +3,7 @@ use crate::hot_cold_store::HotColdDBError;
use ssz::DecodeError;
use types::{BeaconStateError, Hash256};
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub enum Error {
SszDecodeError(DecodeError),
VectorChunkError(ChunkError),

View File

@ -337,20 +337,20 @@ mod tests {
let key = Hash256::random();
let item = StorableThing { a: 1, b: 42 };
assert_eq!(store.exists::<StorableThing>(&key), Ok(false));
assert_eq!(store.exists::<StorableThing>(&key).unwrap(), false);
store.put(&key, &item).unwrap();
assert_eq!(store.exists::<StorableThing>(&key), Ok(true));
assert_eq!(store.exists::<StorableThing>(&key).unwrap(), true);
let retrieved = store.get(&key).unwrap().unwrap();
assert_eq!(item, retrieved);
store.delete::<StorableThing>(&key).unwrap();
assert_eq!(store.exists::<StorableThing>(&key), Ok(false));
assert_eq!(store.exists::<StorableThing>(&key).unwrap(), false);
assert_eq!(store.get::<StorableThing>(&key), Ok(None));
assert_eq!(store.get::<StorableThing>(&key).unwrap(), None);
}
#[test]