flow-go
flow-go copied to clipboard
receipt validator error handling
This PR updates and refines ReceiptValidator
's internal error handling.
Due to our consistent work on error handling, some of the lower-level components that ReceiptValidator
calls into document their errors better now compared to the time the ReceiptValidator
was initially implemented 2 years ago. With a few minor documentation improvements (part of this PR) of some storage components, it is now better possible to comprehensively specify the error handling and error propagation within the ReceiptValidator
, which is the main focus of this PR. Details:
- The central algorithmic improvement this PR brings is that the receipt validator now clearly identifies via dedicated sentinel errors (
UnknownResultError
andUnknownBlockError
) which data is missing to verify receipts. Previously this was all lumped together intoUnverifiableInputError
. But depending on the caller, a missing parent block might indicate state corruption or just be expected. Similarly, a missing parent result could be fatal or expected. Though,UnverifiableInputError
is too coarse, for the calling code to easily differentiate expected from unexpected error scenarios. - we had technical debt where code calling into the receipt validator was checking for errors that were not documented in the API: https://github.com/onflow/flow-go/blob/645923211a85641ade7d335c773d627390de4901/state/protocol/badger/mutator.go#L444-L446 However, the error propagation in the receipt validator internally was only sparsely documented so it was very hard to see where the
storage.ErrNotFound
could be possibly originating from and whether this check was still useful. This PR consistently documents error propagation within theReceiptValidator
and cleans up this tech debt in theParticipantState
- We had the following
TODO
in theParticipantState
, where our understanding has evolved since: https://github.com/onflow/flow-go/blob/645923211a85641ade7d335c773d627390de4901/state/protocol/badger/mutator.go#L443 It turned out that this error actually indicates fatal state corruption, but not just some missing data that could be requested. This PR updates and documents this detail. - While the error documentation of
Snapshot
was lacking: https://github.com/onflow/flow-go/blob/f3bb713dcad80d86ea686bc2aa6ba34513756fdf/state/protocol/snapshot.go#L36-L37 we were checking for specific error types already in theReceiptValidator
. This PR extends the error documentation ofSnapshot.Head()
(I reviewed its implementation to make sure thatErrUnknownSnapshotReference
is really the only possible benign error type). - I think the
FollowerState
had an edge case, where it would erroneously interpret a missing parent block as the candidate block being invalid. This could lead to a wrong slashing challenge! This PR fixes this problem. - updated and extended test
This PR should be fully downwards compatible, i.e. it can be merged into master
and rolled out to nodes at arbitrary times.