Add failure reason to bad block API (#1031)

* Add failure reason to bad block API * Improve bad block API result and duplicate the API under internal blockchain API * core: change return type from interface to exported BadBlockReason * Update type * remove duplicate code (#1039) * remove duplicate code * Update internal/ethapi/api.go Co-authored-by: aaronbuchwald <[email protected]> Co-authored-by: Ceyhun Onur <[email protected]>
ava-labs · Sep 19, 2022 · a65eb61 · a65eb61
1 parent b248d7d
commit a65eb61
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 63 deletions.
diff --git a/core/blockchain.go b/core/blockchain.go
@@ -1287,45 +1287,78 @@ func (bc *BlockChain) reorg(oldBlock, newBlock *types.Block) error {
 	return nil
 }
 
-// BadBlocks returns a list of the last 'bad blocks' that the client has seen on the network
-func (bc *BlockChain) BadBlocks() []*types.Block {
+type badBlock struct {
+	block  *types.Block
+	reason *BadBlockReason
+}
+
+type BadBlockReason struct {
+	ChainConfig *params.ChainConfig `json:"chainConfig"`
+	Receipts    types.Receipts      `json:"receipts"`
+	Number      uint64              `json:"number"`
+	Hash        common.Hash         `json:"hash"`
+	Error       error               `json:"error"`
+}
+
+func (b *BadBlockReason) String() string {
+	var receiptString string
+	for i, receipt := range b.Receipts {
+		receiptString += fmt.Sprintf("\t %d: cumulative: %v gas: %v contract: %v status: %v tx: %v logs: %v bloom: %x state: %x\n",
+			i, receipt.CumulativeGasUsed, receipt.GasUsed, receipt.ContractAddress.Hex(),
+			receipt.Status, receipt.TxHash.Hex(), receipt.Logs, receipt.Bloom, receipt.PostState)
+	}
+	reason := fmt.Sprintf(`
+	########## BAD BLOCK #########
+	Chain config: %v
+	
+	Number: %v
+	Hash: %#x
+	%v
+	
+	Error: %v
+	##############################
+	`, b.ChainConfig, b.Number, b.Hash, receiptString, b.Error)
+
+	return reason
+}
+
+// BadBlocks returns a list of the last 'bad blocks' that the client has seen on the network and the BadBlockReason
+// that caused each to be reported as a bad block.
+// BadBlocks ensures that the length of the blocks and the BadBlockReason slice have the same length.
+func (bc *BlockChain) BadBlocks() ([]*types.Block, []*BadBlockReason) {
 	blocks := make([]*types.Block, 0, bc.badBlocks.Len())
+	reasons := make([]*BadBlockReason, 0, bc.badBlocks.Len())
 	for _, hash := range bc.badBlocks.Keys() {
 		if blk, exist := bc.badBlocks.Peek(hash); exist {
-			block := blk.(*types.Block)
-			blocks = append(blocks, block)
+			badBlk := blk.(*badBlock)
+			blocks = append(blocks, badBlk.block)
+			reasons = append(reasons, badBlk.reason)
 		}
 	}
-	return blocks
+	return blocks, reasons
 }
 
 // addBadBlock adds a bad block to the bad-block LRU cache
-func (bc *BlockChain) addBadBlock(block *types.Block) {
-	bc.badBlocks.Add(block.Hash(), block)
+func (bc *BlockChain) addBadBlock(block *types.Block, reason *BadBlockReason) {
+	bc.badBlocks.Add(block.Hash(), &badBlock{
+		block:  block,
+		reason: reason,
+	})
 }
 
 // reportBlock logs a bad block error.
 func (bc *BlockChain) reportBlock(block *types.Block, receipts types.Receipts, err error) {
-	bc.addBadBlock(block)
-	badBlockCounter.Inc(1)
-
-	var receiptString string
-	for i, receipt := range receipts {
-		receiptString += fmt.Sprintf("\t %d: cumulative: %v gas: %v contract: %v status: %v tx: %v logs: %v bloom: %x state: %x\n",
-			i, receipt.CumulativeGasUsed, receipt.GasUsed, receipt.ContractAddress.Hex(),
-			receipt.Status, receipt.TxHash.Hex(), receipt.Logs, receipt.Bloom, receipt.PostState)
+	reason := &BadBlockReason{
+		ChainConfig: bc.chainConfig,
+		Receipts:    receipts,
+		Number:      block.NumberU64(),
+		Hash:        block.Hash(),
+		Error:       err,
 	}
-	log.Debug(fmt.Sprintf(`
-########## BAD BLOCK #########
-Chain config: %v
 
-Number: %v
-Hash: %#x
-%v
-
-Error: %v
-##############################
-`, bc.chainConfig, block.Number(), block.Hash(), receiptString, err))
+	badBlockCounter.Inc(1)
+	bc.addBadBlock(block, reason)
+	log.Debug(reason.String())
 }
 
 func (bc *BlockChain) RemoveRejectedBlocks(start, end uint64) error {

diff --git a/eth/api.go b/eth/api.go
@@ -219,41 +219,11 @@ func (api *DebugAPI) Preimage(ctx context.Context, hash common.Hash) (hexutil.By
 	return nil, errors.New("unknown preimage")
 }
 
-// BadBlockArgs represents the entries in the list returned when bad blocks are queried.
-type BadBlockArgs struct {
-	Hash  common.Hash            `json:"hash"`
-	Block map[string]interface{} `json:"block"`
-	RLP   string                 `json:"rlp"`
-}
-
 // GetBadBlocks returns a list of the last 'bad blocks' that the client has seen on the network
 // and returns them as a JSON list of block hashes.
-func (api *DebugAPI) GetBadBlocks(ctx context.Context) ([]*BadBlockArgs, error) {
-	var (
-		err     error
-		blocks  = api.eth.BlockChain().BadBlocks()
-		results = make([]*BadBlockArgs, 0, len(blocks))
-	)
-	for _, block := range blocks {
-		var (
-			blockRlp  string
-			blockJSON map[string]interface{}
-		)
-		if rlpBytes, err := rlp.EncodeToBytes(block); err != nil {
-			blockRlp = err.Error() // Hacky, but hey, it works
-		} else {
-			blockRlp = fmt.Sprintf("%#x", rlpBytes)
-		}
-		if blockJSON, err = ethapi.RPCMarshalBlock(block, true, true, api.eth.APIBackend.ChainConfig()); err != nil {
-			blockJSON = map[string]interface{}{"error": err.Error()}
-		}
-		results = append(results, &BadBlockArgs{
-			Hash:  block.Hash(),
-			RLP:   blockRlp,
-			Block: blockJSON,
-		})
-	}
-	return results, nil
+func (api *DebugAPI) GetBadBlocks(ctx context.Context) ([]*ethapi.BadBlockArgs, error) {
+	internalAPI := ethapi.NewBlockChainAPI(api.eth.APIBackend)
+	return internalAPI.GetBadBlocks(ctx)
 }
 
 // AccountRangeMaxResults is the maximum number of results to be returned per call

diff --git a/eth/api_backend.go b/eth/api_backend.go
@@ -200,7 +200,7 @@ func (b *EthAPIBackend) BlockByNumberOrHash(ctx context.Context, blockNrOrHash r
 	return nil, errors.New("invalid arguments; neither block nor hash specified")
 }
 
-func (b *EthAPIBackend) BadBlocks() []*types.Block {
+func (b *EthAPIBackend) BadBlocks() ([]*types.Block, []*core.BadBlockReason) {
 	return b.eth.blockchain.BadBlocks()
 }
 

diff --git a/eth/tracers/api.go b/eth/tracers/api.go
@@ -78,7 +78,7 @@ type Backend interface {
 	HeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Header, error)
 	BlockByHash(ctx context.Context, hash common.Hash) (*types.Block, error)
 	BlockByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Block, error)
-	BadBlocks() []*types.Block
+	BadBlocks() ([]*types.Block, []*core.BadBlockReason)
 	GetTransaction(ctx context.Context, txHash common.Hash) (*types.Transaction, common.Hash, uint64, uint64, error)
 	RPCGasCap() uint64
 	ChainConfig() *params.ChainConfig
@@ -477,8 +477,8 @@ func (api *API) TraceBlock(ctx context.Context, blob hexutil.Bytes, config *Trac
 func (api *API) TraceBadBlock(ctx context.Context, hash common.Hash, config *TraceConfig) ([]*txTraceResult, error) {
 	// Search for the bad block corresponding to [hash].
 	var (
-		badBlocks = api.backend.BadBlocks()
-		block     *types.Block
+		badBlocks, _ = api.backend.BadBlocks()
+		block        *types.Block
 	)
 	for _, badBlock := range badBlocks {
 		if hash == block.Hash() {

diff --git a/eth/tracers/api_test.go b/eth/tracers/api_test.go
@@ -132,7 +132,7 @@ func (b *testBackend) BlockByNumber(ctx context.Context, number rpc.BlockNumber)
 	return b.chain.GetBlockByNumber(uint64(number)), nil
 }
 
-func (b *testBackend) BadBlocks() []*types.Block { return nil }
+func (b *testBackend) BadBlocks() ([]*types.Block, []*core.BadBlockReason) { return nil, nil }
 
 func (b *testBackend) GetTransaction(ctx context.Context, txHash common.Hash) (*types.Transaction, common.Hash, uint64, uint64, error) {
 	tx, hash, blockNumber, index := rawdb.ReadTransaction(b.chaindb, txHash)

diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go
@@ -1521,6 +1521,49 @@ func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrH
 	}
 }
 
+// Note: this API is moved directly from ./eth/api.go to ensure that it is available under an API that is enabled by
+// default without duplicating the code and serving the same API in the original location as well without creating a
+// cyclic import.
+//
+// BadBlockArgs represents the entries in the list returned when bad blocks are queried.
+type BadBlockArgs struct {
+	Hash   common.Hash            `json:"hash"`
+	Block  map[string]interface{} `json:"block"`
+	RLP    string                 `json:"rlp"`
+	Reason *core.BadBlockReason   `json:"reason"`
+}
+
+// GetBadBlocks returns a list of the last 'bad blocks' that the client has seen on the network
+// and returns them as a JSON list of block hashes.
+func (s *BlockChainAPI) GetBadBlocks(ctx context.Context) ([]*BadBlockArgs, error) {
+	var (
+		err                error
+		badBlocks, reasons = s.b.BadBlocks()
+		results            = make([]*BadBlockArgs, 0, len(badBlocks))
+	)
+	for i, block := range badBlocks {
+		var (
+			blockRlp  string
+			blockJSON map[string]interface{}
+		)
+		if rlpBytes, err := rlp.EncodeToBytes(block); err != nil {
+			blockRlp = err.Error() // Hacky, but hey, it works
+		} else {
+			blockRlp = fmt.Sprintf("%#x", rlpBytes)
+		}
+		if blockJSON, err = RPCMarshalBlock(block, true, true, s.b.ChainConfig()); err != nil {
+			blockJSON = map[string]interface{}{"error": err.Error()}
+		}
+		results = append(results, &BadBlockArgs{
+			Hash:   block.Hash(),
+			RLP:    blockRlp,
+			Block:  blockJSON,
+			Reason: reasons[i],
+		})
+	}
+	return results, nil
+}
+
 // TransactionAPI exposes methods for reading and creating transaction data.
 type TransactionAPI struct {
 	b         Backend

diff --git a/internal/ethapi/backend.go b/internal/ethapi/backend.go
@@ -78,6 +78,7 @@ type Backend interface {
 	SubscribeChainEvent(ch chan<- core.ChainEvent) event.Subscription
 	SubscribeChainHeadEvent(ch chan<- core.ChainHeadEvent) event.Subscription
 	SubscribeChainSideEvent(ch chan<- core.ChainSideEvent) event.Subscription
+	BadBlocks() ([]*types.Block, []*core.BadBlockReason)
 
 	// Transaction pool API
 	SendTx(ctx context.Context, signedTx *types.Transaction) error