feat(eth): return consistent error for null rounds from RPC methods (#12655)

Author: virajbhartiya

CHANGELOG.md

@@ -3,6 +3,7 @@
 # UNRELEASED
 
 ## New features
+- Return a consistent error when encountering null rounds in ETH RPC method calls. ([filecoin-project/lotus#12655](https://github.com/filecoin-project/lotus/pull/12655))
 - Reduce size of embedded genesis CAR files by removing WASM actor blocks and compressing with zstd. This reduces the `lotus` binary size by approximately 10 MiB. ([filecoin-project/lotus#12439](https://github.com/filecoin-project/lotus/pull/12439))
 - Add ChainSafe operated Calibration archival node to the bootstrap list ([filecoin-project/lotus#12517](https://github.com/filecoin-project/lotus/pull/12517))
 - `lotus chain head` now supports a `--height` flag to print just the epoch number of the current chain head ([filecoin-project/lotus#12609](https://github.com/filecoin-project/lotus/pull/12609))

api/api_errors.go

@@ -2,11 +2,13 @@ package api
 
 import (
 	"errors"
+	"fmt"
 	"reflect"
 
 	"golang.org/x/xerrors"
 
 	"github.com/filecoin-project/go-jsonrpc"
+	"github.com/filecoin-project/go-state-types/abi"
 )
 
 var invalidExecutionRevertedMsg = xerrors.New("invalid execution reverted error")
@@ -22,6 +24,7 @@ const (
 	EF3ParticipationTicketStartBeforeExisting
 	EF3NotReady
 	EExecutionReverted
+	ENullRound
 )
 
 var (
@@ -54,6 +57,8 @@ var (
 	_ error                 = (*errF3NotReady)(nil)
 	_ error                 = (*ErrExecutionReverted)(nil)
 	_ jsonrpc.RPCErrorCodec = (*ErrExecutionReverted)(nil)
+	_ error                 = (*ErrNullRound)(nil)
+	_ jsonrpc.RPCErrorCodec = (*ErrNullRound)(nil)
 )
 
 func init() {
@@ -67,6 +72,7 @@ func init() {
 	RPCErrors.Register(EF3ParticipationTicketStartBeforeExisting, new(*errF3ParticipationTicketStartBeforeExisting))
 	RPCErrors.Register(EF3NotReady, new(*errF3NotReady))
 	RPCErrors.Register(EExecutionReverted, new(*ErrExecutionReverted))
+	RPCErrors.Register(ENullRound, new(*ErrNullRound))
 }
 
 func ErrorIsIn(err error, errorTypes []error) bool {
@@ -160,3 +166,49 @@ func NewErrExecutionReverted(reason string) *ErrExecutionReverted {
 		Data:    reason,
 	}
 }
+
+type ErrNullRound struct {
+	Epoch   abi.ChainEpoch
+	Message string
+}
+
+func NewErrNullRound(epoch abi.ChainEpoch) *ErrNullRound {
+	return &ErrNullRound{
+		Epoch:   epoch,
+		Message: fmt.Sprintf("requested epoch was a null round (%d)", epoch),
+	}
+}
+
+func (e *ErrNullRound) Error() string {
+	return e.Message
+}
+
+func (e *ErrNullRound) FromJSONRPCError(jerr jsonrpc.JSONRPCError) error {
+	if jerr.Code != ENullRound {
+		return fmt.Errorf("unexpected error code: %d", jerr.Code)
+	}
+
+	epoch, ok := jerr.Data.(float64)
+	if !ok {
+		return fmt.Errorf("expected number data in null round error, got %T", jerr.Data)
+	}
+
+	e.Epoch = abi.ChainEpoch(epoch)
+	e.Message = jerr.Message
+	return nil
+}
+
+func (e *ErrNullRound) ToJSONRPCError() (jsonrpc.JSONRPCError, error) {
+	return jsonrpc.JSONRPCError{
+		Code:    ENullRound,
+		Message: e.Message,
+		Data:    e.Epoch,
+	}, nil
+}
+
+// Is performs a non-strict type check, we only care if the target is an ErrNullRound
+// and will ignore the contents (specifically there is no matching on Epoch).
+func (e *ErrNullRound) Is(target error) bool {
+	_, ok := target.(*ErrNullRound)
+	return ok
+}

itests/fevm_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/filecoin-project/go-address"
+	"github.com/filecoin-project/go-jsonrpc"
 	"github.com/filecoin-project/go-state-types/abi"
 	"github.com/filecoin-project/go-state-types/big"
 	builtintypes "github.com/filecoin-project/go-state-types/builtin"
@@ -1546,7 +1547,7 @@ func TestEthGetTransactionByBlockHashAndIndexAndNumber(t *testing.T) {
 		// 2. Invalid block number
 		_, err = client.EthGetTransactionByBlockNumberAndIndex(ctx, (blockNumber + 1000).Hex(), ethtypes.EthUint64(0))
 		require.Error(t, err)
-		require.ErrorContains(t, err, "failed to get tipset")
+		require.ErrorContains(t, err, "requested a future epoch")
 
 		// 3. Index out of range
 		_, err = client.EthGetTransactionByBlockHashAndIndex(ctx, blockHash, ethtypes.EthUint64(100))
@@ -1659,3 +1660,95 @@ func TestEthEstimateGas(t *testing.T) {
 		})
 	}
 }
+
+func TestEthNullRoundHandling(t *testing.T) {
+	blockTime := 100 * time.Millisecond
+	client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.ThroughRPC())
+
+	bms := ens.InterconnectAll().BeginMining(blockTime)
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+	defer cancel()
+
+	client.WaitTillChain(ctx, kit.HeightAtLeast(10))
+
+	bms[0].InjectNulls(10)
+
+	tctx, cancel := context.WithTimeout(ctx, 30*time.Second)
+	defer cancel()
+	ch, err := client.ChainNotify(tctx)
+	require.NoError(t, err)
+	<-ch
+	hc := <-ch
+	require.Equal(t, store.HCApply, hc[0].Type)
+
+	afterNullHeight := hc[0].Val.Height()
+
+	nullHeight := afterNullHeight - 1
+	for nullHeight > 0 {
+		ts, err := client.ChainGetTipSetByHeight(ctx, nullHeight, types.EmptyTSK)
+		require.NoError(t, err)
+		if ts.Height() == nullHeight {
+			nullHeight--
+		} else {
+			break
+		}
+	}
+
+	nullBlockHex := fmt.Sprintf("0x%x", int(nullHeight))
+	client.WaitTillChain(ctx, kit.HeightAtLeast(nullHeight+2))
+	testCases := []struct {
+		name     string
+		testFunc func() error
+	}{
+		{
+			name: "EthGetBlockByNumber",
+			testFunc: func() error {
+				_, err := client.EthGetBlockByNumber(ctx, nullBlockHex, true)
+				return err
+			},
+		},
+		{
+			name: "EthFeeHistory",
+			testFunc: func() error {
+				_, err := client.EthFeeHistory(ctx, jsonrpc.RawParams([]byte(`[1,"`+nullBlockHex+`",[]]`)))
+				return err
+			},
+		},
+		{
+			name: "EthTraceBlock",
+			testFunc: func() error {
+				_, err := client.EthTraceBlock(ctx, nullBlockHex)
+				return err
+			},
+		},
+		{
+			name: "EthTraceReplayBlockTransactions",
+			testFunc: func() error {
+				_, err := client.EthTraceReplayBlockTransactions(ctx, nullBlockHex, []string{"trace"})
+				return err
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.testFunc()
+			if err == nil {
+				return
+			}
+			require.Error(t, err)
+
+			// Test errors.Is
+			require.ErrorIs(t, err, new(api.ErrNullRound), "error should be or wrap ErrNullRound")
+
+			// Test errors.As and verify message
+			var nullRoundErr *api.ErrNullRound
+			require.ErrorAs(t, err, &nullRoundErr, "error should be convertible to ErrNullRound")
+
+			expectedMsg := fmt.Sprintf("requested epoch was a null round (%d)", nullHeight)
+			require.Equal(t, expectedMsg, nullRoundErr.Error())
+			require.Equal(t, nullHeight, nullRoundErr.Epoch)
+		})
+	}
+}

node/impl/full/eth.go

@@ -177,8 +177,6 @@ type EthAPI struct {
 	EthEventAPI
 }
 
-var ErrNullRound = errors.New("requested epoch was a null round")
-
 func (a *EthModule) StateNetworkName(ctx context.Context) (dtypes.NetworkName, error) {
 	return stmgr.GetNetworkName(ctx, a.StateManager, a.Chain.GetHeaviestTipSet().ParentState())
 }
@@ -243,10 +241,9 @@ func (a *EthAPI) FilecoinAddressToEthAddress(ctx context.Context, p jsonrpc.RawP
 		blkParam = *params.BlkParam
 	}
 
-	// Get the tipset for the specified block
-	ts, err := getTipsetByBlockNumber(ctx, a.Chain, blkParam, true)
+	ts, err := getTipsetByBlockNumber(ctx, a.Chain, blkParam, false)
 	if err != nil {
-		return ethtypes.EthAddress{}, xerrors.Errorf("failed to get tipset for block %s: %w", blkParam, err)
+		return ethtypes.EthAddress{}, err
 	}
 
 	// Lookup the ID address
@@ -571,7 +568,7 @@ func (a *EthAPI) EthGetTransactionByBlockHashAndIndex(ctx context.Context, blkHa
 func (a *EthAPI) EthGetTransactionByBlockNumberAndIndex(ctx context.Context, blkParam string, index ethtypes.EthUint64) (*ethtypes.EthTx, error) {
 	ts, err := getTipsetByBlockNumber(ctx, a.Chain, blkParam, true)
 	if err != nil {
-		return nil, xerrors.Errorf("failed to get tipset for block %s: %w", blkParam, err)
+		return nil, err
 	}
 
 	if ts == nil {
@@ -942,7 +939,7 @@ func (a *EthModule) EthFeeHistory(ctx context.Context, p jsonrpc.RawParams) (eth
 
 	ts, err := getTipsetByBlockNumber(ctx, a.Chain, params.NewestBlkNum, false)
 	if err != nil {
-		return ethtypes.EthFeeHistory{}, fmt.Errorf("bad block parameter %s: %s", params.NewestBlkNum, err)
+		return ethtypes.EthFeeHistory{}, err
 	}
 
 	var (
@@ -1099,9 +1096,9 @@ func (a *EthModule) Web3ClientVersion(ctx context.Context) (string, error) {
 }
 
 func (a *EthModule) EthTraceBlock(ctx context.Context, blkNum string) ([]*ethtypes.EthTraceBlock, error) {
-	ts, err := getTipsetByBlockNumber(ctx, a.Chain, blkNum, false)
+	ts, err := getTipsetByBlockNumber(ctx, a.Chain, blkNum, true)
 	if err != nil {
-		return nil, xerrors.Errorf("failed to get tipset: %w", err)
+		return nil, err
 	}
 
 	stRoot, trace, err := a.StateManager.ExecutionTrace(ctx, ts)
@@ -1170,10 +1167,9 @@ func (a *EthModule) EthTraceReplayBlockTransactions(ctx context.Context, blkNum
 	if len(traceTypes) != 1 || traceTypes[0] != "trace" {
 		return nil, fmt.Errorf("only 'trace' is supported")
 	}
-
-	ts, err := getTipsetByBlockNumber(ctx, a.Chain, blkNum, false)
+	ts, err := getTipsetByBlockNumber(ctx, a.Chain, blkNum, true)
 	if err != nil {
-		return nil, xerrors.Errorf("failed to get tipset: %w", err)
+		return nil, err
 	}
 
 	stRoot, trace, err := a.StateManager.ExecutionTrace(ctx, ts)

node/impl/full/eth_utils.go

@@ -88,7 +88,7 @@ func getTipsetByBlockNumber(ctx context.Context, chain *store.ChainStore, blkPar
 			return nil, fmt.Errorf("cannot get tipset at height: %v", num)
 		}
 		if strict && ts.Height() != abi.ChainEpoch(num) {
-			return nil, ErrNullRound
+			return nil, api.NewErrNullRound(abi.ChainEpoch(num))
 		}
 		return ts, nil
 	}