fix suggestions

Author: jewei1997

evmrpc/config.go

@@ -75,9 +75,6 @@ type Config struct {
 	// controls whether to synchronously flush receipts before block finalze or not
 	FlushReceiptSync bool `mapstructure:"flush_receipt_sync"`
 
-	// DisableWatermark disables cross-store watermark gating (intended for tests)
-	DisableWatermark bool `mapstructure:"disable_watermark"`
-
 	// Deny list defines list of methods that EVM RPC should fail fast
 	DenyList []string `mapstructure:"deny_list"`
 
@@ -129,7 +126,6 @@ var DefaultConfig = Config{
 	MaxTxPoolTxs:                 1000,
 	Slow:                         false,
 	FlushReceiptSync:             false,
-	DisableWatermark:             false,
 	DenyList:                     make([]string, 0),
 	MaxLogNoBlock:                10000,
 	MaxBlocksForLog:              2000,
@@ -160,7 +156,6 @@ const (
 	flagCheckTxTimeout               = "evm.checktx_timeout"
 	flagSlow                         = "evm.slow"
 	FlagFlushReceiptSync             = "evm.flush_receipt_sync"
-	flagDisableWatermark             = "evm.disable_watermark"
 	flagDenyList                     = "evm.deny_list"
 	flagMaxLogNoBlock                = "evm.max_log_no_block"
 	flagMaxBlocksForLog              = "evm.max_blocks_for_log"
@@ -261,11 +256,6 @@ func ReadConfig(opts servertypes.AppOptions) (Config, error) {
 			return cfg, err
 		}
 	}
-	if v := opts.Get(flagDisableWatermark); v != nil {
-		if cfg.DisableWatermark, err = cast.ToBoolE(v); err != nil {
-			return cfg, err
-		}
-	}
 	if v := opts.Get(flagDenyList); v != nil {
 		if cfg.DenyList, err = cast.ToStringSliceE(v); err != nil {
 			return cfg, err

evmrpc/info.go

@@ -394,19 +394,45 @@ func (i *InfoAPI) CalculateGasUsedRatio(ctx context.Context, blockHeight int64)
 }
 
 func (i *InfoAPI) latestHeight(ctx context.Context) (int64, error) {
-	if i.watermarks != nil {
-		return i.watermarks.LatestHeight(ctx)
+	return i.heightWithWatermarks(ctx, func(c context.Context) (int64, error) {
+		return i.watermarks.LatestHeight(c)
+	}, i.latestHeightFromContext)
+}
+
+func (i *InfoAPI) earliestHeight(ctx context.Context) (int64, error) {
+	return i.heightWithWatermarks(ctx, func(c context.Context) (int64, error) {
+		return i.watermarks.EarliestHeight(c)
+	}, i.earliestHeightFromContext)
+}
+
+func (i *InfoAPI) heightWithWatermarks(
+	ctx context.Context,
+	wmFunc func(context.Context) (int64, error),
+	fallback func() (int64, error),
+) (int64, error) {
+	if i.watermarks != nil && wmFunc != nil {
+		height, err := wmFunc(ctx)
+		if err == nil {
+			return height, nil
+		}
+		if !errors.Is(err, errNoHeightSource) {
+			return 0, err
+		}
+	}
+	if fallback == nil {
+		return 0, fmt.Errorf("ctx provider not configured")
 	}
+	return fallback()
+}
+
+func (i *InfoAPI) latestHeightFromContext() (int64, error) {
 	if i.ctxProvider == nil {
 		return 0, fmt.Errorf("ctx provider not configured")
 	}
 	return i.ctxProvider(LatestCtxHeight).BlockHeight(), nil
 }
 
-func (i *InfoAPI) earliestHeight(ctx context.Context) (int64, error) {
-	if i.watermarks != nil {
-		return i.watermarks.EarliestHeight(ctx)
-	}
+func (i *InfoAPI) earliestHeightFromContext() (int64, error) {
 	if i.ctxProvider == nil {
 		return 0, fmt.Errorf("ctx provider not configured")
 	}

evmrpc/server.go

@@ -64,10 +64,7 @@ func NewEVMHTTPServer(
 
 	ctx := ctxProvider(LatestCtxHeight)
 
-	var watermarks *WatermarkManager
-	if !config.DisableWatermark {
-		watermarks = NewWatermarkManager(tmClient, ctxProvider, nil, k.ReceiptStore())
-	}
+	watermarks := NewWatermarkManager(tmClient, ctxProvider, nil, k.ReceiptStore())
 	txAPI := NewTransactionAPI(tmClient, k, ctxProvider, txConfigProvider, homeDir, ConnectionTypeHTTP, watermarks)
 	debugAPI := NewDebugAPI(tmClient, k, ctxProvider, txConfigProvider, simulateConfig, app, antehandler, ConnectionTypeHTTP, config)
 	if isPanicOrSyntheticTxFunc == nil {
@@ -231,10 +228,7 @@ func NewEVMWebSocketServer(
 		EVMTimeout:                   config.SimulationEVMTimeout,
 		MaxConcurrentSimulationCalls: config.MaxConcurrentSimulationCalls,
 	}
-	var watermarks *WatermarkManager
-	if !config.DisableWatermark {
-		watermarks = NewWatermarkManager(tmClient, ctxProvider, nil, k.ReceiptStore())
-	}
+	watermarks := NewWatermarkManager(tmClient, ctxProvider, nil, k.ReceiptStore())
 	dbReadSemaphore := make(chan struct{}, MaxDBReadConcurrency)
 	globalBlockCache := NewBlockCache(3000)
 	globalLogSlicePool := NewLogSlicePool()

evmrpc/setup_test.go

@@ -573,7 +573,6 @@ func init() {
 	goodConfig.HTTPPort = TestPort
 	goodConfig.WSPort = TestWSPort
 	goodConfig.FilterTimeout = 500 * time.Millisecond
-	goodConfig.DisableWatermark = true
 	goodConfig.MaxLogNoBlock = 10
 	infoLog, err := log.NewDefaultLogger("text", "info")
 	if err != nil {
@@ -591,7 +590,6 @@ func init() {
 	// Start bad http server
 	badConfig := evmrpc.DefaultConfig
 	badConfig.HTTPPort = TestBadPort
-	badConfig.DisableWatermark = true
 	badConfig.FilterTimeout = 500 * time.Millisecond
 	badHTTPServer, err := evmrpc.NewEVMHTTPServer(infoLog, badConfig, &MockBadClient{}, EVMKeeper, testApp.BaseApp, testApp.TracerAnteHandler, ctxProvider, txConfigProvider, "", nil)
 	if err != nil {
@@ -1074,6 +1072,13 @@ func setupLogs() {
 	EVMKeeper.SetBlockBloom(Ctx, []ethtypes.Bloom{bloomSynth, bloom4, bloomTx1})
 	EVMKeeper.SetEvmOnlyBlockBloom(Ctx, []ethtypes.Bloom{bloom4, bloomTx1})
 
+	if store := EVMKeeper.ReceiptStore(); store != nil {
+		if err := store.SetLatestVersion(MockHeight103); err != nil {
+			panic(err)
+		}
+		_ = store.SetEarliestVersion(1, true)
+	}
+
 }
 
 //nolint:deadcode

evmrpc/tests/utils.go

@@ -137,7 +137,6 @@ func setupTestServer(
 	cfg := evmrpc.DefaultConfig
 	cfg.HTTPEnabled = true
 	cfg.HTTPPort = port
-	cfg.DisableWatermark = true
 	s, err := evmrpc.NewEVMHTTPServer(
 		log.NewNopLogger(),
 		cfg,

evmrpc/tx_test.go

@@ -81,7 +81,7 @@ func TestGetTransactionCount(t *testing.T) {
 
 func TestGetTransactionError(t *testing.T) {
 	h := common.HexToHash("0x1111111111111111111111111111111111111111111111111111111111111111")
-	EVMKeeper.MockReceipt(Ctx, h, &types.Receipt{VmError: "test error"})
+	EVMKeeper.MockReceipt(Ctx, h, &types.Receipt{VmError: "test error", BlockNumber: 1})
 	resObj := sendRequestGood(t, "getTransactionErrorByHash", "0x1111111111111111111111111111111111111111111111111111111111111111")
 	require.Equal(t, "test error", resObj["result"])
 

evmrpc/watermark_manager.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 
-	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/ethereum/go-ethereum/rpc"
 	sstypes "github.com/sei-protocol/sei-db/ss/types"
@@ -48,44 +47,64 @@ func (m *WatermarkManager) Watermarks(ctx context.Context) (int64, int64, error)
 	}
 
 	var (
-		earliestCandidates []int64
-		latestCandidates   []int64
+		latest      int64
+		latestSet   bool
+		earliest    int64
+		earliestSet bool
 	)
 
+	setLatest := func(candidate int64) {
+		if candidate < 0 {
+			return
+		}
+		if !latestSet || candidate < latest {
+			latest = candidate
+			latestSet = true
+		}
+	}
+
+	setEarliest := func(candidate int64) {
+		if candidate < 0 {
+			return
+		}
+		if !earliestSet || candidate > earliest {
+			earliest = candidate
+			earliestSet = true
+		}
+	}
+
 	// Tendermint heights
-	if latest, earliest, err := m.fetchTendermintWatermarks(ctx); err == nil {
-		latestCandidates = append(latestCandidates, latest)
-		earliestCandidates = append(earliestCandidates, earliest)
+	if tmLatest, tmEarliest, err := m.fetchTendermintWatermarks(ctx); err == nil {
+		setLatest(tmLatest)
+		setEarliest(tmEarliest)
 	} else if !errors.Is(err, errNoHeightSource) {
 		return 0, 0, err
 	}
 
+	if m.ctxProvider != nil {
+		if ctxHeight := m.ctxProvider(LatestCtxHeight).BlockHeight(); ctxHeight > 0 {
+			setLatest(ctxHeight)
+		}
+	}
+
 	// State store heights (historical state DB)
 	if ssLatest, ssEarliest, ok := m.fetchStateStoreWatermarks(); ok {
-		latestCandidates = append(latestCandidates, ssLatest)
-		earliestCandidates = append(earliestCandidates, ssEarliest)
-	} else if msLatest, msEarliest, ok := m.fetchMultiStoreWatermarks(); ok {
-		// Fall back to application multistore heights if state store unavailable
-		latestCandidates = append(latestCandidates, msLatest)
-		earliestCandidates = append(earliestCandidates, msEarliest)
+		setLatest(ssLatest)
+		setEarliest(ssEarliest)
 	}
 
 	// Receipt store height participates only in the latest watermark, since
 	// pruning guarantees the earliest watermark is bounded by TM+SS.
 	if m.receiptStore != nil {
-		if latest := m.receiptStore.GetLatestVersion(); latest > 0 {
-			latestCandidates = append(latestCandidates, latest)
-		}
+		setLatest(m.receiptStore.GetLatestVersion())
 	}
 
-	if len(latestCandidates) == 0 {
+	if !latestSet {
 		return 0, 0, errNoHeightSource
 	}
 
-	latest := minInt64(latestCandidates)
-	earliest := int64(0)
-	if len(earliestCandidates) > 0 {
-		earliest = maxInt64(earliestCandidates)
+	if !earliestSet {
+		earliest = 0
 	}
 
 	if latest < earliest {
@@ -224,50 +243,3 @@ func (m *WatermarkManager) fetchStateStoreWatermarks() (int64, int64, bool) {
 	}
 	return m.stateStore.GetLatestVersion(), m.stateStore.GetEarliestVersion(), true
 }
-
-func (m *WatermarkManager) fetchMultiStoreWatermarks() (latest int64, earliest int64, ok bool) {
-	if m.ctxProvider == nil {
-		return 0, 0, false
-	}
-	ctx := m.ctxProvider(LatestCtxHeight)
-	ms := ctx.MultiStore()
-	if ms == nil {
-		return 0, 0, false
-	}
-	defer func() {
-		if r := recover(); r != nil {
-			latest, earliest, ok = 0, 0, false
-		}
-	}()
-	earliest = ms.GetEarliestVersion()
-	if earliest == 0 {
-		earliest = ctx.BlockHeight()
-	}
-	if commitStore, implements := ms.(interface{ LastCommitID() storetypes.CommitID }); implements {
-		latest = commitStore.LastCommitID().Version
-	}
-	if latest == 0 {
-		latest = ctx.BlockHeight()
-	}
-	return latest, earliest, true
-}
-
-func minInt64(values []int64) int64 {
-	min := values[0]
-	for _, v := range values[1:] {
-		if v < min {
-			min = v
-		}
-	}
-	return min
-}
-
-func maxInt64(values []int64) int64 {
-	max := values[0]
-	for _, v := range values[1:] {
-		if v > max {
-			max = v
-		}
-	}
-	return max
-}

evmrpc/watermark_manager_test.go

@@ -37,7 +37,7 @@ func TestWatermarksAggregatesSources(t *testing.T) {
 	require.Equal(t, int64(8), latest)
 }
 
-func TestWatermarksFallbackToMultiStore(t *testing.T) {
+func TestWatermarksIncludesCtxProviderHeight(t *testing.T) {
 	multi := &fakeMultiStore{earliest: 0, latest: 0}
 	ctx := sdk.Context{}.
 		WithBlockHeight(12).
@@ -49,7 +49,7 @@ func TestWatermarksFallbackToMultiStore(t *testing.T) {
 
 	earliest, latest, err := wm.Watermarks(context.Background())
 	require.NoError(t, err)
-	require.Equal(t, int64(12), earliest)
+	require.Equal(t, int64(5), earliest)
 	require.Equal(t, int64(12), latest)
 }