Skip to content

Commit

Permalink
Merge #4350
Browse files Browse the repository at this point in the history
4350: [BFT] Reporting compliance engine protocol violations  r=durkmurder a=durkmurder

### Context

This is a follow-up PR which addresses outstanding comments in [previous](#4174) PR.
In compliance engine I have removed usage of `engine.InvalidInputError` since I wanted to consolidate slashing evidence reporting in one place(which works with `model.InvalidProposalError`) and `engine.InvalidInputError` was just adding another level of wrapping without real benefit. I've decided to replace it with `model.InvalidProposalError` which simplified error handling. 

Reference comments: [1](#4174 (comment)), [2](#4174 (comment)), [3](#4174 (comment))

Co-authored-by: Yurii Oleksyshyn <yuraolex@gmail.com>
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
  • Loading branch information
3 people authored Jun 19, 2023
2 parents b7e0736 + 0622054 commit c94a5a4
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 6 deletions.
3 changes: 2 additions & 1 deletion engine/collection/compliance/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,8 @@ func checkForAndLogOutdatedInputError(err error, log zerolog.Logger) bool {
func checkForAndLogUnverifiableInputError(err error, log zerolog.Logger) bool {
if engine.IsUnverifiableInputError(err) {
// the block cannot be validated
log.Err(err).Msg("received unverifiable block proposal; this is an indicator of a proposal that cannot be verified under current state")
log.Warn().Err(err).Msg("received collection proposal with unknown reference block; " +
"this might be an indicator that the node is slightly behind or the proposer published an invalid collection")
return true
}
return false
Expand Down
2 changes: 1 addition & 1 deletion engine/common/follower/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type Cache struct {
byView map[uint64]BlocksByID // lookup of blocks by their respective view; used to detect equivocation
byParent map[flow.Identifier]BlocksByID // lookup of blocks by their parentID, for finding a block's known children

notifier hotstuff.ProposalViolationConsumer // equivocation will be reported using this notifier
notifier hotstuff.ProposalViolationConsumer // equivocations will be reported using this notifier
lowestView counters.StrictMonotonousCounter // lowest view that the cache accepts blocks for
}

Expand Down
7 changes: 4 additions & 3 deletions engine/consensus/compliance/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,9 @@ func (c *Core) processBlockProposal(proposal *flow.Block) error {
// We know:
// - the parent of this block is valid and was appended to the state (ie. we knew the epoch for it)
// - if we then see this for the child, one of two things must have happened:
// 1. the proposer malicious created the block for a view very far in the future (it's invalid)
// 1. the proposer maliciously created the block for a view very far in the future (it's invalid)
// -> in this case we can disregard the block
// 2. no blocks have been finalized within the epoch commitment deadline, and the epoch ended
// 2. no blocks have been finalized within the epoch commitment deadline, and the epoch ended
// (breaking a critical assumption - see EpochCommitSafetyThreshold in protocol.Params for details)
// -> in this case, the network has encountered a critical failure
// - we assume in general that Case 2 will not happen, therefore this must be Case 1 - an invalid block
Expand Down Expand Up @@ -428,7 +428,8 @@ func checkForAndLogOutdatedInputError(err error, log zerolog.Logger) bool {
func checkForAndLogUnverifiableInputError(err error, log zerolog.Logger) bool {
if engine.IsUnverifiableInputError(err) {
// the block cannot be validated
log.Err(err).Msg("received unverifiable block proposal; this is an indicator of a proposal that cannot be verified under current state")
log.Warn().Err(err).Msg("received unverifiable block proposal; " +
"this might be an indicator that a malicious proposer is generating detached blocks very far ahead")
return true
}
return false
Expand Down
2 changes: 1 addition & 1 deletion state/cluster/badger/mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func (m *MutableState) checkHeaderValidity(candidate *cluster.Block) error {
// checkConnectsToFinalizedState validates that the candidate block connects to
// the latest finalized state (ie. is not extending an orphaned fork).
// Expected error returns:
// - state.UnverifiableExtensionError if the candidate extends an orphaned fork
// - state.OutdatedExtensionError if the candidate extends an orphaned fork
func (m *MutableState) checkConnectsToFinalizedState(ctx extendContext) error {
header := ctx.candidate.Header
finalizedID := ctx.finalizedClusterBlock.ID()
Expand Down

0 comments on commit c94a5a4

Please sign in to comment.