Merge branch 'develop' into bump_deps

CHANGELOG.md

@@ -72,6 +72,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (rpc) [#534](https://github.com/crypto-org-chain/ethermint/pull/534), [#540](https://github.com/crypto-org-chain/ethermint/pull/540) Fix opBlockhash when no block header in abci request.
 * (rpc) [#536](https://github.com/crypto-org-chain/ethermint/pull/536) Fix validate basic after transaction conversion with raw field.
 * (cli) [#537](https://github.com/crypto-org-chain/ethermint/pull/537) Fix unsuppored sign mode SIGN_MODE_TEXTUAL for bank transfer.
+* (cli) [#543](https://github.com/crypto-org-chain/ethermint/pull/543) Fix graceful shutdown.
+* (rpc) [#545](https://github.com/crypto-org-chain/ethermint/pull/545) Fix state overwrite in debug trace APIs.
 
 ### Improvements
 

server/json_rpc.go

@@ -16,6 +16,7 @@
 package server
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"time"
@@ -47,18 +48,20 @@ type AppWithPendingTxStream interface {
 }
 
 // StartJSONRPC starts the JSON-RPC server
-func StartJSONRPC(srvCtx *server.Context,
+func StartJSONRPC(
+	ctx context.Context,
+	srvCtx *server.Context,
 	clientCtx client.Context,
 	g *errgroup.Group,
 	config *config.Config,
 	indexer ethermint.EVMTxIndexer,
 	app AppWithPendingTxStream,
-) (*http.Server, chan struct{}, error) {
+) (*http.Server, error) {
 	logger := srvCtx.Logger.With("module", "geth")
 
 	evtClient, ok := clientCtx.Client.(rpcclient.EventsClient)
 	if !ok {
-		return nil, nil, fmt.Errorf("client %T does not implement EventsClient", clientCtx.Client)
+		return nil, fmt.Errorf("client %T does not implement EventsClient", clientCtx.Client)
 	}
 
 	var rpcStream *stream.RPCStream
@@ -73,7 +76,7 @@ func StartJSONRPC(srvCtx *server.Context,
 	}
 
 	if err != nil {
-		return nil, nil, fmt.Errorf("failed to create rpc streams after %d attempts: %w", MaxRetry, err)
+		return nil, fmt.Errorf("failed to create rpc streams after %d attempts: %w", MaxRetry, err)
 	}
 
 	app.RegisterPendingTxListener(rpcStream.ListenPendingTx)
@@ -104,7 +107,7 @@ func StartJSONRPC(srvCtx *server.Context,
 				"namespace", api.Namespace,
 				"service", api.Service,
 			)
-			return nil, nil, err
+			return nil, err
 		}
 	}
 
@@ -128,25 +131,41 @@ func StartJSONRPC(srvCtx *server.Context,
 
 	ln, err := Listen(httpSrv.Addr, config)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	g.Go(func() error {
 		srvCtx.Logger.Info("Starting JSON-RPC server", "address", config.JSONRPC.Address)
-		if err := httpSrv.Serve(ln); err != nil {
+		errCh := make(chan error)
+		go func() {
+			errCh <- httpSrv.Serve(ln)
+		}()
+
+		// Start a blocking select to wait for an indication to stop the server or that
+		// the server failed to start properly.
+		select {
+		case <-ctx.Done():
+			// The calling process canceled or closed the provided context, so we must
+			// gracefully stop the JSON-RPC server.
+			logger.Info("stopping JSON-RPC server...", "address", config.JSONRPC.Address)
+			if err := httpSrv.Shutdown(context.Background()); err != nil {
+				logger.Error("failed to shutdown JSON-RPC server", "error", err.Error())
+			}
+			return ln.Close()
+
+		case err := <-errCh:
 			if err == http.ErrServerClosed {
 				close(httpSrvDone)
 			}
 
 			srvCtx.Logger.Error("failed to start JSON-RPC server", "error", err.Error())
 			return err
 		}
-		return nil
 	})
 
 	srvCtx.Logger.Info("Starting JSON WebSocket server", "address", config.JSONRPC.WsAddress)
 
 	wsSrv := rpc.NewWebsocketsServer(clientCtx, srvCtx.Logger, rpcStream, config)
 	wsSrv.Start()
-	return httpSrv, httpSrvDone, nil
+	return httpSrv, nil
 }

server/start.go

@@ -20,11 +20,9 @@ import (
 	"fmt"
 	"io"
 	"net"
-	"net/http"
 	"os"
 	"path/filepath"
 	"runtime/pprof"
-	"time"
 
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
@@ -419,10 +417,11 @@ func startInProcess(svrCtx *server.Context, clientCtx client.Context, opts Start
 		idxer = indexer.NewKVIndexer(idxDB, idxLogger, clientCtx)
 		indexerService := NewEVMIndexerService(idxer, clientCtx.Client.(rpcclient.Client), config.JSONRPC.AllowIndexerGap)
 		indexerService.SetLogger(servercmtlog.CometLoggerWrapper{Logger: idxLogger})
-
-		g.Go(func() error {
-			return indexerService.Start()
-		})
+		go func() {
+			if err := indexerService.Start(); err != nil {
+				logger.Error("failed to start evm indexer service", "error", err.Error())
+			}
+		}()
 	}
 
 	if config.API.Enable || config.JSONRPC.Enable {
@@ -443,30 +442,12 @@ func startInProcess(svrCtx *server.Context, clientCtx client.Context, opts Start
 	if err != nil {
 		return err
 	}
-	if grpcSrv != nil {
-		defer grpcSrv.GracefulStop()
-	}
 
-	apiSrv := startAPIServer(ctx, svrCtx, clientCtx, g, config.Config, app, grpcSrv, metrics)
-	if apiSrv != nil {
-		defer apiSrv.Close()
-	}
+	startAPIServer(ctx, svrCtx, clientCtx, g, config.Config, app, grpcSrv, metrics)
 
-	clientCtx, httpSrv, httpSrvDone, err := startJSONRPCServer(svrCtx, clientCtx, g, config, genDocProvider, idxer, app)
-	if httpSrv != nil {
-		defer func() {
-			shutdownCtx, cancelFn := context.WithTimeout(context.Background(), 10*time.Second)
-			defer cancelFn()
-			if err := httpSrv.Shutdown(shutdownCtx); err != nil {
-				logger.Error("HTTP server shutdown produced a warning", "error", err.Error())
-			} else {
-				logger.Info("HTTP server shut down, waiting 5 sec")
-				select {
-				case <-time.Tick(5 * time.Second):
-				case <-httpSrvDone:
-				}
-			}
-		}()
+	clientCtx, err = startJSONRPCServer(ctx, svrCtx, clientCtx, g, config, genDocProvider, idxer, app)
+	if err != nil {
+		return err
 	}
 
 	// At this point it is safe to block the process if we're in query only mode as
@@ -619,9 +600,9 @@ func startAPIServer(
 	app types.Application,
 	grpcSrv *grpc.Server,
 	metrics *telemetry.Metrics,
-) *api.Server {
+) {
 	if !svrCfg.API.Enable {
-		return nil
+		return
 	}
 
 	apiSrv := api.New(clientCtx, svrCtx.Logger.With("server", "api"), grpcSrv)
@@ -634,38 +615,35 @@ func startAPIServer(
 	g.Go(func() error {
 		return apiSrv.Start(ctx, svrCfg)
 	})
-	return apiSrv
 }
 
 func startJSONRPCServer(
+	stdCtx context.Context,
 	svrCtx *server.Context,
 	clientCtx client.Context,
 	g *errgroup.Group,
 	config config.Config,
 	genDocProvider node.GenesisDocProvider,
 	idxer ethermint.EVMTxIndexer,
 	app types.Application,
-) (ctx client.Context, httpSrv *http.Server, httpSrvDone chan struct{}, err error) {
+) (ctx client.Context, err error) {
 	ctx = clientCtx
 	if !config.JSONRPC.Enable {
 		return
 	}
 
 	txApp, ok := app.(AppWithPendingTxStream)
 	if !ok {
-		return ctx, httpSrv, httpSrvDone, fmt.Errorf("json-rpc server requires AppWithPendingTxStream")
+		return ctx, fmt.Errorf("json-rpc server requires AppWithPendingTxStream")
 	}
 
 	genDoc, err := genDocProvider()
 	if err != nil {
-		return ctx, httpSrv, httpSrvDone, err
+		return ctx, err
 	}
 
 	ctx = clientCtx.WithChainID(genDoc.ChainID)
-	g.Go(func() error {
-		httpSrv, httpSrvDone, err = StartJSONRPC(svrCtx, clientCtx, g, &config, idxer, txApp)
-		return err
-	})
+	_, err = StartJSONRPC(stdCtx, svrCtx, clientCtx, g, &config, idxer, txApp)
 	return
 }
 

tests/integration_tests/configs/default.jsonnet

@@ -88,6 +88,7 @@
           params: {
             no_base_fee: false,
             base_fee: '100000000000',
+            min_gas_multiplier: '0.5',
           },
         },
       },

tests/integration_tests/test_tracers.py

@@ -113,8 +113,7 @@ def process(w3):
         assert res[0] == res[-1] == EXPECTED_CONTRACT_CREATE_TRACER, res
 
 
-def fund_acc(w3, acc):
-    fund = 3000000000000000000
+def fund_acc(w3, acc, fund=3000000000000000000):
     addr = acc.address
     if w3.eth.get_balance(addr, "latest") == 0:
         tx = {"to": addr, "value": fund, "gasPrice": w3.eth.gas_price}
@@ -459,6 +458,100 @@ def process(w3):
         assert res[0] == res[-1] == balance, res
 
 
+def test_refund_unused_gas_when_contract_tx_reverted(ethermint):
+    w3 = ethermint.w3
+    test_revert, _ = deploy_contract(w3, CONTRACTS["TestRevert"])
+    gas = 1000000
+    gas_price = 6060000000000
+    acc = derive_new_account(10)
+    fund_acc(w3, acc, fund=10000000000000000000)
+    p = ethermint.cosmos_cli().get_params("feemarket")["params"]
+    min_gas_multiplier = float(p["min_gas_multiplier"])
+    sender = acc.address.lower()
+    tx_res = w3.provider.make_request(
+        "debug_traceCall",
+        [
+            {
+                "value": "0x0",
+                "to": test_revert.address,
+                "from": sender,
+                "data": "0x9ffb86a5",
+                "gas": hex(gas),
+                "gasPrice": hex(gas_price),
+            },
+            "latest",
+            {
+                "tracer": "prestateTracer",
+                "tracerConfig": {
+                    "diffMode": True,
+                },
+            },
+        ],
+    )
+    assert "result" in tx_res
+    tx_res = tx_res["result"]
+    pre = int(tx_res["pre"][sender]["balance"], 16)
+    post = int(tx_res["post"][sender]["balance"], 16)
+    diff = pre - gas * gas_price * min_gas_multiplier - post
+    assert diff == 0, diff
+
+    pre = w3.eth.get_balance(acc.address)
+    receipt = send_transaction(
+        w3,
+        test_revert.functions.revertWithMsg().build_transaction(
+            {
+                "gas": gas,
+                "gasPrice": gas_price,
+            }
+        ),
+        key=acc.key,
+    )
+    assert receipt["status"] == 0, receipt["status"]
+    post = w3.eth.get_balance(acc.address)
+    diff = pre - gas * gas_price * min_gas_multiplier - post
+    assert diff == 0, diff
+
+
+def test_refund_unused_gas_when_contract_tx_reverted_state_overrides(ethermint):
+    w3 = ethermint.w3
+    test_revert, _ = deploy_contract(w3, CONTRACTS["TestRevert"])
+    gas = 21000
+    gas_price = 6060000000000
+    acc = derive_new_account(10)
+    fund_acc(w3, acc, fund=10000000000000000000)
+    sender = acc.address.lower()
+    balance = 10000000000000000000000
+    nonce = 1000
+    tx_res = w3.provider.make_request(
+        "debug_traceCall",
+        [
+            {
+                "value": "0x1",
+                "to": test_revert.address,
+                "from": sender,
+                "gas": hex(gas),
+                "gasPrice": hex(gas_price),
+            },
+            "latest",
+            {
+                "tracer": "prestateTracer",
+                "stateOverrides": {
+                    sender: {
+                        "balance": hex(balance),
+                        "nonce": hex(nonce),
+                    }
+                }
+            },
+        ],
+    )
+    assert "result" in tx_res
+    tx_res = tx_res["result"]
+    balance_af = int(tx_res[sender]["balance"], 16)
+    nonce_af = tx_res[sender]["nonce"]
+    assert balance_af == balance, balance_af
+    assert nonce_af == nonce, nonce_af
+
+
 def test_debug_tracecall_return_revert_data_when_call_failed(ethermint, geth):
     expected = "08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001a46756e6374696f6e20686173206265656e207265766572746564000000000000"  # noqa: E501
 

testutil/network/network.go

@@ -196,14 +196,13 @@ type (
 		RPCClient     tmrpcclient.Client
 		JSONRPCClient *ethclient.Client
 
-		tmNode      *node.Node
-		api         *api.Server
-		grpc        *grpc.Server
-		grpcWeb     *http.Server
-		jsonrpc     *http.Server
-		jsonrpcDone chan struct{}
-		errGroup    *errgroup.Group
-		cancelFn    context.CancelFunc
+		tmNode   *node.Node
+		api      *api.Server
+		grpc     *grpc.Server
+		grpcWeb  *http.Server
+		jsonrpc  *http.Server
+		errGroup *errgroup.Group
+		cancelFn context.CancelFunc
 	}
 )
 
@@ -648,18 +647,7 @@ func (n *Network) Cleanup() {
 		}
 
 		if v.jsonrpc != nil {
-			shutdownCtx, cancelFn := context.WithTimeout(context.Background(), 10*time.Second)
-			defer cancelFn()
-
-			if err := v.jsonrpc.Shutdown(shutdownCtx); err != nil {
-				v.tmNode.Logger.Error("HTTP server shutdown produced a warning", "error", err.Error())
-			} else {
-				v.tmNode.Logger.Info("HTTP server shut down, waiting 5 sec")
-				select {
-				case <-time.Tick(5 * time.Second):
-				case <-v.jsonrpcDone:
-				}
-			}
+			_ = v.jsonrpc.Close()
 		}
 	}
 

testutil/network/util.go

@@ -143,8 +143,8 @@ func startInProcess(cfg Config, val *Validator) error {
 			return fmt.Errorf("validator %s context is nil", val.Moniker)
 		}
 
-		val.jsonrpc, val.jsonrpcDone, err = server.StartJSONRPC(
-			val.Ctx, val.ClientCtx, val.errGroup, val.AppConfig,
+		val.jsonrpc, err = server.StartJSONRPC(
+			ctx, val.Ctx, val.ClientCtx, val.errGroup, val.AppConfig,
 			nil, app.(server.AppWithPendingTxStream),
 		)
 		if err != nil {