Skip to content

Commit

Permalink
validation: add package encapsulating validation error categories
Browse files Browse the repository at this point in the history
  • Loading branch information
schomatis committed Apr 13, 2020
1 parent 9eecd9d commit 931a384
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 19 deletions.
52 changes: 33 additions & 19 deletions chain/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package chain
import (
"bytes"
"context"
"errors"
"fmt"
"strings"
"sync"
Expand Down Expand Up @@ -43,6 +42,7 @@ import (
"github.com/filecoin-project/lotus/chain/types"
"github.com/filecoin-project/lotus/lib/sigs"
"github.com/filecoin-project/lotus/metrics"
"github.com/filecoin-project/lotus/validation"
)

var log = logging.Logger("chain")
Expand Down Expand Up @@ -442,8 +442,15 @@ func (syncer *Syncer) Sync(ctx context.Context, maybeHead *types.TipSet) error {
return nil
}

// FIXME: Document.
func isPermanent(err error) bool {
return !errors.Is(err, ErrTemporal)
if errorWithCategory, ok := err.(validation.ErrorWithCategory); ok {
return errorWithCategory.Category() != validation.BlockFutureTimestamp
}
// FIXME: Include this in `ErrorWithCategory` if it makes sense, it could allow
// us to extend the logic and traverse the hierarchy.

return false
}

func (syncer *Syncer) ValidateTipSet(ctx context.Context, fts *store.FullTipSet) error {
Expand Down Expand Up @@ -491,8 +498,6 @@ func (syncer *Syncer) minerIsValid(ctx context.Context, maddr address.Address, b
return nil
}

var ErrTemporal = errors.New("temporal error")

// Should match up with 'Semantical Validation' in validation.md in the spec
func (syncer *Syncer) ValidateBlock(ctx context.Context, b *types.FullBlock) error {
ctx, span := trace.StartSpan(ctx, "validateBlock")
Expand All @@ -506,6 +511,8 @@ func (syncer *Syncer) ValidateBlock(ctx context.Context, b *types.FullBlock) err
baseTs, err := syncer.store.LoadTipSet(types.NewTipSetKey(h.Parents...))
if err != nil {
return xerrors.Errorf("load parent tipset failed (%s): %w", h.Parents, err)
// FIXME: If the block is referencing unknown parents we may want to add this to the
// validation package, but probably there's an early point where that is discovered.
}

prevBeacon, err := syncer.store.GetLatestBeaconEntry(baseTs)
Expand All @@ -517,34 +524,36 @@ func (syncer *Syncer) ValidateBlock(ctx context.Context, b *types.FullBlock) err

// fast checks first
if h.BlockSig == nil {
return xerrors.Errorf("block had nil signature")
return validation.NilSignature.Error()
}

now := uint64(time.Now().Unix())
if h.Timestamp > now+build.AllowableClockDrift {
return xerrors.Errorf("block was from the future (now=%d, blk=%d): %w", now, h.Timestamp, ErrTemporal)
maxTimeDrift := time.Now().Add(build.AllowableClockDrift)
if h.Timestamp > uint64(maxTimeDrift.Unix()) {
return validation.BlockFutureTimestamp.FromString(fmt.Sprintf("%s > %s", time.Unix(int64(h.Timestamp), 0).Format("%FT%T"), maxTimeDrift.Format("%FT%T")))
// FIXME: There surely is a simpler way to format these times.
}
if h.Timestamp > now {
if h.Timestamp > uint64(time.Now().Unix()) {
log.Warn("Got block from the future, but within threshold", h.Timestamp, time.Now().Unix())
}

if h.Timestamp < baseTs.MinTimestamp()+(build.BlockDelay*uint64(h.Height-baseTs.Height())) {
log.Warn("timestamp funtimes: ", h.Timestamp, baseTs.MinTimestamp(), h.Height, baseTs.Height())
diff := (baseTs.MinTimestamp() + (build.BlockDelay * uint64(h.Height-baseTs.Height()))) - h.Timestamp

return xerrors.Errorf("block was generated too soon (h.ts:%d < base.mints:%d + BLOCK_DELAY:%d * deltaH:%d; diff %d)", h.Timestamp, baseTs.MinTimestamp(), build.BlockDelay, h.Height-baseTs.Height(), diff)
return validation.Timestamp.FromString(fmt.Sprintf("h.ts:%d < base.mints:%d + BLOCK_DELAY:%d * deltaH:%d; diff %d",
h.Timestamp, baseTs.MinTimestamp(), build.BlockDelay, h.Height-baseTs.Height(), diff))
}

msgsCheck := async.Err(func() error {
if err := syncer.checkBlockMessages(ctx, b, baseTs); err != nil {
return xerrors.Errorf("block had invalid messages: %w", err)
return err
}
return nil
})

minerCheck := async.Err(func() error {
if err := syncer.minerIsValid(ctx, h.Miner, baseTs); err != nil {
return xerrors.Errorf("minerIsValid failed: %w", err)
return validation.Miner.WrapError(err)
}
return nil
})
Expand Down Expand Up @@ -804,6 +813,7 @@ func (syncer *Syncer) VerifyElectionPoStProof(ctx context.Context, h *types.Bloc
}
*/

// Returns validation errors from the `InvalidMessage` category.
func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock, baseTs *types.TipSet) error {
{
var sigCids []cid.Cid // this is what we get for people not wanting the marshalcbor method on the cid type
Expand All @@ -815,13 +825,14 @@ func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock
pubk, err := syncer.sm.GetBlsPublicKey(ctx, m.From, baseTs)
if err != nil {
return xerrors.Errorf("failed to load bls public to validate block: %w", err)
// FIXME: Is there a validation error here?
}

pubks = append(pubks, pubk)
}

if err := syncer.verifyBlsAggregate(ctx, b.Header.BLSAggregate, sigCids, pubks); err != nil {
return xerrors.Errorf("bls aggregate signature was invalid: %w", err)
return validation.InvalidMessage.WrapError(err)
}
}

Expand All @@ -840,20 +851,22 @@ func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock

checkMsg := func(m *types.Message) error {
if m.To == address.Undef {
return xerrors.New("'To' address cannot be empty")
return validation.EmptyRecipient.Error()
}

if _, ok := nonces[m.From]; !ok {
// `GetActor` does not validate that this is an account actor.
act, err := st.GetActor(m.From)
if err != nil {
return xerrors.Errorf("failed to get actor: %w", err)
// FIXME: Some of `GetActor` errors should be in the validation package (it
// does many checks besides retrieving information).
}
nonces[m.From] = act.Nonce
}

if nonces[m.From] != m.Nonce {
return xerrors.Errorf("wrong nonce (exp: %d, got: %d)", nonces[m.From], m.Nonce)
return validation.WrongNonce.FromString(fmt.Sprintf("exp: %d, got: %d", nonces[m.From], m.Nonce))
}
nonces[m.From]++

Expand All @@ -864,7 +877,7 @@ func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock

for i, m := range b.BlsMessages {
if err := checkMsg(m); err != nil {
return xerrors.Errorf("block had invalid bls message at index %d: %w", i, err)
return validation.InvalidMessage.FromString(fmt.Sprintf("invalid bls message at index %d: %s", i, err))
}

c := cbg.CborCid(m.Cid())
Expand All @@ -874,7 +887,7 @@ func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock
var secpkCids []cbg.CBORMarshaler
for i, m := range b.SecpkMessages {
if err := checkMsg(&m.Message); err != nil {
return xerrors.Errorf("block had invalid secpk message at index %d: %w", i, err)
return validation.InvalidMessage.FromString(fmt.Sprintf("invalid secpk message at index %d: %s", i, err))
}

// `From` being an account actor is only validated inside the `vm.ResolveToKeyAddr` call
Expand All @@ -885,7 +898,7 @@ func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock
}

if err := sigs.Verify(&m.Signature, kaddr, m.Message.Cid().Bytes()); err != nil {
return xerrors.Errorf("secpk message %s has invalid signature: %w", m.Cid(), err)
return validation.InvalidSecpkSignature.FromString(fmt.Sprintf("%s: %s", err.Error(), m.Cid()))
}

c := cbg.CborCid(m.Cid())
Expand All @@ -911,7 +924,8 @@ func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock
}

if b.Header.Messages != mrcid {
return fmt.Errorf("messages didnt match message root in header")
return validation.InvalidMessagesCID.Error()
// FIXME: This is more of a block check than a message check.
}

return nil
Expand Down
39 changes: 39 additions & 0 deletions validation/category.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package validation

// Errors extracted from `ValidateBlock` and `checkBlockMessages` and converted
// into more generic "categories" that are related with each other in a hierarchical
// fashion and allow to spawn errors from them.
// Explicitly split into separate `var` declarations instead of having a single
// block (to avoid `go fmt` potentially changing alignment of all errors and
// producing harder-to-read diffs).
// FIXME: How to better express this error hierarchy *without* reflection? (nested packages?)

var InvalidBlock = NewHierarchicalErrorClass("invalid block")

var NilSignature = InvalidBlock.Child("nil signature")

var Timestamp = InvalidBlock.Child("invalid timestamp")
var BlockFutureTimestamp = Timestamp.Child("ahead of current time")

var BlockMinedEarly = InvalidBlock.Child("block was generated too soon")

var Winner = InvalidBlock.Child("not a winner")
var SlashedMiner = Winner.Child("slashed or invalid miner")
var NoCandidates = Winner.Child("no candidates")
var DuplicateCandidates = Winner.Child("duplicate epost candidates")

// FIXME: Might want to include these in some EPost category.

var Miner = InvalidBlock.Child("invalid miner")

var InvalidMessagesCID = InvalidBlock.Child("messages CID didn't match message root in header")

var InvalidMessage = InvalidBlock.Child("invalid message")
var InvalidBlsSignature = InvalidMessage.Child("invalid bls aggregate signature")
var InvalidSecpkSignature = InvalidMessage.Child("invalid secpk signature")
var EmptyRecipient = InvalidMessage.Child("empty 'To' address")
var WrongNonce = InvalidMessage.Child("wrong nonce")

// FIXME: Errors from `checkBlockMessages` are too generic and should probably be extracted, is there
// another place where we validate them (like the message pool)? In that case we need a different
// root category like `InvalidMessage`.
95 changes: 95 additions & 0 deletions validation/error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package validation

import (
"fmt"

"golang.org/x/xerrors"
)

// Hierarchy of error categories to programmatically preserve in the language
// information linking errors from different contexts. Used inside
// `ErrorWithCategory`, it complements the standard `xerrors.Errorf` wrapping
// mechanism that preserves call stack information. This is not an error itself,
// it explicitly does not implement the interface forcing explicit calls to
// convert a category into a specific error instance used in validation functions.
// The hierarchy is implemented through a pointer to a (unique) `parent` category
// (if any). The `description` string identifies the category and will be used
// as the error message if one is not explicitly provided (like in `FromString()`).
// FIXME: This is general enough to be outside of this package.
type ErrorCategory struct {
parent *ErrorCategory
description string
}

func NewHierarchicalErrorClass(description string) *ErrorCategory {
return &ErrorCategory{description: description}
}

// Creates a new (child) category included in this one.
func (c *ErrorCategory) Child(description string) *ErrorCategory {
return &ErrorCategory{parent: c, description: description}
}

// Returns a new error with the category description as its message.
func (c *ErrorCategory) Error() error {
return &ErrorWithCategory{category: c, msg: c.description, frame: getCallerFrame()}
}

// Returns a new error from this category with the specified message.
func (c *ErrorCategory) FromString(msg string) error {
return &ErrorWithCategory{category: c, msg: msg, frame: getCallerFrame()}
}

// Similar to `Error`, returns an error err.
func (c *ErrorCategory) WrapError(wrappedError error) error {
err := c.Error().(ErrorWithCategory)
err.wrappedError = wrappedError
return err
}

// Error implementing the `error` interface providing category information
// and wrapped error semantics similar to `xerrors.Errorf`, including frame
// information (based on `xerrors.wrapError`).
type ErrorWithCategory struct {
category *ErrorCategory

// Copied `xerrors.wrapError` attributes (since its a private structure)
wrappedError error
msg string
frame xerrors.Frame
}

func (e *ErrorWithCategory) Category() *ErrorCategory {
return e.category
}

// FIXME: Why can't it have a pointer receiver to satisfy the `error` interface?
// (`xerrors.wrapError` uses pointer)
func (e ErrorWithCategory) Error() string {
return fmt.Sprint(e)
}

func (e *ErrorWithCategory) Format(s fmt.State, v rune) { xerrors.FormatError(e, s, v) }

func (e *ErrorWithCategory) FormatError(p xerrors.Printer) (next error) {
p.Print(e.msg)
// FIXME: If we're using an external string as the error message we should
// probably want to include the category description here.
e.frame.Format(p)
return e.wrappedError
}

func (e *ErrorWithCategory) Unwrap() error {
return e.wrappedError
}

func getCallerFrame() xerrors.Frame {
frame := xerrors.Frame{}
// if internal.EnableTrace {
frame = xerrors.Caller(1)
// }
// FIXME: Can't reference internal package, if the flag is necessary we can extract
// it from an oracle of `xerrors` behavior (hacky).

return frame
}

0 comments on commit 931a384

Please sign in to comment.