Merge pull request #1253 from filecoin-project/feat/bad-ts-cache-reasons

mark down reason for block inclusion in bad tipset cache

api/api_full.go

@@ -46,6 +46,7 @@ type FullNode interface {
 	SyncSubmitBlock(ctx context.Context, blk *types.BlockMsg) error
 	SyncIncomingBlocks(ctx context.Context) (<-chan *types.BlockHeader, error)
 	SyncMarkBad(ctx context.Context, bcid cid.Cid) error
+	SyncCheckBad(ctx context.Context, bcid cid.Cid) (string, error)
 
 	// messages
 	MpoolPending(context.Context, *types.TipSet) ([]*types.SignedMessage, error)

api/apistruct/struct.go

@@ -66,6 +66,7 @@ type FullNodeStruct struct {
 		SyncSubmitBlock    func(ctx context.Context, blk *types.BlockMsg) error         `perm:"write"`
 		SyncIncomingBlocks func(ctx context.Context) (<-chan *types.BlockHeader, error) `perm:"read"`
 		SyncMarkBad        func(ctx context.Context, bcid cid.Cid) error                `perm:"admin"`
+		SyncCheckBad       func(ctx context.Context, bcid cid.Cid) (string, error)      `perm:"read"`
 
 		MpoolPending     func(context.Context, *types.TipSet) ([]*types.SignedMessage, error) `perm:"read"`
 		MpoolPush        func(context.Context, *types.SignedMessage) (cid.Cid, error)         `perm:"write"`
@@ -401,6 +402,10 @@ func (c *FullNodeStruct) SyncMarkBad(ctx context.Context, bcid cid.Cid) error {
 	return c.Internal.SyncMarkBad(ctx, bcid)
 }
 
+func (c *FullNodeStruct) SyncCheckBad(ctx context.Context, bcid cid.Cid) (string, error) {
+	return c.Internal.SyncCheckBad(ctx, bcid)
+}
+
 func (c *FullNodeStruct) StateMinerSectors(ctx context.Context, addr address.Address, ts *types.TipSet) ([]*api.ChainSectorInfo, error) {
 	return c.Internal.StateMinerSectors(ctx, addr, ts)
 }

chain/badtscache.go

@@ -21,10 +21,15 @@ func NewBadBlockCache() *BadBlockCache {
 	}
 }
 
-func (bts *BadBlockCache) Add(c cid.Cid) {
+func (bts *BadBlockCache) Add(c cid.Cid, reason string) {
-	bts.badBlocks.Add(c, nil)
+	bts.badBlocks.Add(c, reason)
 }
 
-func (bts *BadBlockCache) Has(c cid.Cid) bool {
+func (bts *BadBlockCache) Has(c cid.Cid) (string, bool) {
-	return bts.badBlocks.Contains(c)
+	rval, ok := bts.badBlocks.Get(c)
+	if !ok {
+		return "", false
+	}
+
+	return rval.(string), true
 }

chain/sync.go

@@ -118,8 +118,8 @@ func (syncer *Syncer) InformNewHead(from peer.ID, fts *store.FullTipSet) bool {
 	}
 
 	for _, b := range fts.Blocks {
-		if syncer.bad.Has(b.Cid()) {
+		if reason, ok := syncer.bad.Has(b.Cid()); ok {
-			log.Warnf("InformNewHead called on block marked as bad: %s", b.Cid())
+			log.Warnf("InformNewHead called on block marked as bad: %s (reason: %s)", b.Cid(), reason)
 			return false
 		}
 		if err := syncer.ValidateMsgMeta(b); err != nil {
@@ -450,7 +450,7 @@ func (syncer *Syncer) ValidateTipSet(ctx context.Context, fts *store.FullTipSet)
 	for _, b := range fts.Blocks {
 		if err := syncer.ValidateBlock(ctx, b); err != nil {
 			if isPermanent(err) {
-				syncer.bad.Add(b.Cid())
+				syncer.bad.Add(b.Cid(), err.Error())
 			}
 			return xerrors.Errorf("validating block %s: %w", b.Cid(), err)
 		}
@@ -876,11 +876,11 @@ func (syncer *Syncer) collectHeaders(ctx context.Context, from *types.TipSet, to
 	)
 
 	for _, pcid := range from.Parents().Cids() {
-		if syncer.bad.Has(pcid) {
+		if reason, ok := syncer.bad.Has(pcid); ok {
 			for _, b := range from.Cids() {
-				syncer.bad.Add(b)
+				syncer.bad.Add(b, fmt.Sprintf("linked to %s", pcid))
 			}
-			return nil, xerrors.Errorf("chain linked to block marked previously as bad (%s, %s)", from.Cids(), pcid)
+			return nil, xerrors.Errorf("chain linked to block marked previously as bad (%s, %s) (reason: %s)", from.Cids(), pcid, reason)
 		}
 	}
 
@@ -898,12 +898,12 @@ func (syncer *Syncer) collectHeaders(ctx context.Context, from *types.TipSet, to
 loop:
 	for blockSet[len(blockSet)-1].Height() > untilHeight {
 		for _, bc := range at.Cids() {
-			if syncer.bad.Has(bc) {
+			if reason, ok := syncer.bad.Has(bc); ok {
 				for _, b := range acceptedBlocks {
-					syncer.bad.Add(b)
+					syncer.bad.Add(b, fmt.Sprintf("chain contained %s", bc))
 				}
 
-				return nil, xerrors.Errorf("chain contained block marked previously as bad (%s, %s)", from.Cids(), bc)
+				return nil, xerrors.Errorf("chain contained block marked previously as bad (%s, %s) (reason: %s)", from.Cids(), bc, reason)
 			}
 		}
 
@@ -946,12 +946,12 @@ loop:
 				break loop
 			}
 			for _, bc := range b.Cids() {
-				if syncer.bad.Has(bc) {
+				if reason, ok := syncer.bad.Has(bc); ok {
 					for _, b := range acceptedBlocks {
-						syncer.bad.Add(b)
+						syncer.bad.Add(b, fmt.Sprintf("chain contained %s", bc))
 					}
 
-					return nil, xerrors.Errorf("chain contained block marked previously as bad (%s, %s)", from.Cids(), bc)
+					return nil, xerrors.Errorf("chain contained block marked previously as bad (%s, %s) (reason: %s)", from.Cids(), bc, reason)
 				}
 			}
 			blockSet = append(blockSet, b)
@@ -978,7 +978,7 @@ loop:
 				// TODO: we're marking this block bad in the same way that we mark invalid blocks bad. Maybe distinguish?
 				log.Warn("adding forked chain to our bad tipset cache")
 				for _, b := range from.Blocks() {
-					syncer.bad.Add(b.Cid())
+					syncer.bad.Add(b.Cid(), "fork past finality")
 				}
 			}
 			return nil, xerrors.Errorf("failed to sync fork: %w", err)
@@ -1195,5 +1195,9 @@ func (syncer *Syncer) State() []SyncerState {
 }
 
 func (syncer *Syncer) MarkBad(blk cid.Cid) {
-	syncer.bad.Add(blk)
+	syncer.bad.Add(blk, "manually marked bad")
+}
+
+func (syncer *Syncer) CheckBadBlockCache(blk cid.Cid) (string, bool) {
+	return syncer.bad.Has(blk)
 }

cli/sync.go

@@ -20,6 +20,7 @@ var syncCmd = &cli.Command{
 		syncStatusCmd,
 		syncWaitCmd,
 		syncMarkBadCmd,
+		syncCheckBadCmd,
 	},
 }
 
@@ -115,6 +116,41 @@ var syncMarkBadCmd = &cli.Command{
 	},
 }
 
+var syncCheckBadCmd = &cli.Command{
+	Name:  "check-bad",
+	Usage: "check if the given block was marked bad, and for what reason",
+	Action: func(cctx *cli.Context) error {
+		napi, closer, err := GetFullNodeAPI(cctx)
+		if err != nil {
+			return err
+		}
+		defer closer()
+		ctx := ReqContext(cctx)
+
+		if !cctx.Args().Present() {
+			return fmt.Errorf("must specify block cid to check")
+		}
+
+		bcid, err := cid.Decode(cctx.Args().First())
+		if err != nil {
+			return fmt.Errorf("failed to decode input as a cid: %s", err)
+		}
+
+		reason, err := napi.SyncCheckBad(ctx, bcid)
+		if err != nil {
+			return err
+		}
+
+		if reason == "" {
+			fmt.Println("block was not marked as bad")
+			return nil
+		}
+
+		fmt.Println(reason)
+		return nil
+	},
+}
+
 func SyncWait(ctx context.Context, napi api.FullNode) error {
 	for {
 		state, err := napi.SyncState(ctx)

node/impl/full/sync.go

@@ -88,3 +88,12 @@ func (a *SyncAPI) SyncMarkBad(ctx context.Context, bcid cid.Cid) error {
 	a.Syncer.MarkBad(bcid)
 	return nil
 }
+
+func (a *SyncAPI) SyncCheckBad(ctx context.Context, bcid cid.Cid) (string, error) {
+	reason, ok := a.Syncer.CheckBadBlockCache(bcid)
+	if !ok {
+		return "", nil
+	}
+
+	return reason, nil
+}