From b263c4a098ca4a533bcd53307d61c89ce7c986cf Mon Sep 17 00:00:00 2001 From: Devashish Dixit Date: Tue, 12 Oct 2021 11:22:06 +0800 Subject: [PATCH 1/7] Problem: No way to limit total number of filters that can be created Solution: Add a config parameter to set the total number of filters that can be created --- rpc/ethereum/backend/backend.go | 5 ++++ rpc/ethereum/namespaces/eth/filters/api.go | 29 +++++++++++++++++++--- server/config/config.go | 6 +++++ server/config/toml.go | 3 +++ server/flags/flags.go | 11 ++++---- server/start.go | 1 + 6 files changed, 47 insertions(+), 8 deletions(-) diff --git a/rpc/ethereum/backend/backend.go b/rpc/ethereum/backend/backend.go index d2b9181508..58846dcf65 100644 --- a/rpc/ethereum/backend/backend.go +++ b/rpc/ethereum/backend/backend.go @@ -765,6 +765,11 @@ func (e *EVMBackend) RPCGasCap() uint64 { return e.cfg.JSONRPC.GasCap } +// RPCFilterCap is the limit for total number of filters that can be created +func (e *EVMBackend) RPCFilterCap() uint64 { + return e.cfg.JSONRPC.FilterCap +} + // RPCMinGasPrice returns the minimum gas price for a transaction obtained from // the node config. If set value is 0, it will default to 20. diff --git a/rpc/ethereum/namespaces/eth/filters/api.go b/rpc/ethereum/namespaces/eth/filters/api.go index 8e1ff2f4d5..dff6ca5e41 100644 --- a/rpc/ethereum/namespaces/eth/filters/api.go +++ b/rpc/ethereum/namespaces/eth/filters/api.go @@ -36,6 +36,8 @@ type Backend interface { BloomStatus() (uint64, uint64) GetFilteredBlocks(from int64, to int64, bloomIndexes [][]BloomIV, filterAddresses bool) ([]int64, error) + + RPCFilterCap() uint64 } // consider a filter inactive if it has not been polled for within deadline @@ -107,13 +109,20 @@ func (api *PublicFilterAPI) timeoutLoop() { // // https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_newPendingTransactionFilter func (api *PublicFilterAPI) NewPendingTransactionFilter() rpc.ID { + api.filtersMu.Lock() + + if len(api.filters) >= int(api.backend.RPCFilterCap()) { + api.filtersMu.Unlock() + return rpc.ID("error creating pending tx filter: max limit reached") + } + pendingTxSub, cancelSubs, err := api.events.SubscribePendingTxs() if err != nil { + api.filtersMu.Unlock() // wrap error on the ID return rpc.ID(fmt.Sprintf("error creating pending tx filter: %s", err.Error())) } - api.filtersMu.Lock() api.filters[pendingTxSub.ID()] = &filter{typ: filters.PendingTransactionsSubscription, deadline: time.NewTimer(deadline), hashes: make([]common.Hash, 0), s: pendingTxSub} api.filtersMu.Unlock() @@ -219,8 +228,16 @@ func (api *PublicFilterAPI) NewPendingTransactions(ctx context.Context) (*rpc.Su // // https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_newblockfilter func (api *PublicFilterAPI) NewBlockFilter() rpc.ID { + api.filtersMu.Lock() + + if len(api.filters) >= int(api.backend.RPCFilterCap()) { + api.filtersMu.Unlock() + return rpc.ID("error creating block filter: max limit reached") + } + headerSub, cancelSubs, err := api.events.SubscribeNewHeads() if err != nil { + api.filtersMu.Unlock() // wrap error on the ID return rpc.ID(fmt.Sprintf("error creating block filter: %s", err.Error())) } @@ -228,7 +245,6 @@ func (api *PublicFilterAPI) NewBlockFilter() rpc.ID { // TODO: use events to get the base fee amount baseFee := big.NewInt(params.InitialBaseFee) - api.filtersMu.Lock() api.filters[headerSub.ID()] = &filter{typ: filters.BlocksSubscription, deadline: time.NewTimer(deadline), hashes: []common.Hash{}, s: headerSub} api.filtersMu.Unlock() @@ -404,6 +420,13 @@ func (api *PublicFilterAPI) Logs(ctx context.Context, crit filters.FilterCriteri // // https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_newfilter func (api *PublicFilterAPI) NewFilter(criteria filters.FilterCriteria) (rpc.ID, error) { + api.filtersMu.Lock() + + if len(api.filters) >= int(api.backend.RPCFilterCap()) { + api.filtersMu.Unlock() + return rpc.ID(""), fmt.Errorf("error creating filter: max limit reached") + } + var ( filterID = rpc.ID("") err error @@ -411,12 +434,12 @@ func (api *PublicFilterAPI) NewFilter(criteria filters.FilterCriteria) (rpc.ID, logsSub, cancelSubs, err := api.events.SubscribeLogs(criteria) if err != nil { + api.filtersMu.Unlock() return rpc.ID(""), err } filterID = logsSub.ID() - api.filtersMu.Lock() api.filters[filterID] = &filter{typ: filters.LogsSubscription, deadline: time.NewTimer(deadline), hashes: []common.Hash{}, s: logsSub} api.filtersMu.Unlock() diff --git a/server/config/config.go b/server/config/config.go index 110b2452a7..376b08457b 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -28,6 +28,8 @@ const ( DefaultEVMTracer = "json" DefaultGasCap uint64 = 25000000 + + DefaultFilterCap uint64 = 2000000 ) var evmTracers = []string{DefaultEVMTracer, "markdown", "struct", "access_list"} @@ -61,6 +63,8 @@ type JSONRPCConfig struct { Enable bool `mapstructure:"enable"` // GasCap is the global gas cap for eth-call variants. GasCap uint64 `mapstructure:"gas-cap"` + // FilterCap is the global cap for total number of filters that can be created. + FilterCap uint64 `mapstructure:"filter-cap"` } // TLSConfig defines the certificate and matching private key for the server. @@ -145,6 +149,7 @@ func DefaultJSONRPCConfig() *JSONRPCConfig { Address: DefaultJSONRPCAddress, WsAddress: DefaultJSONRPCWsAddress, GasCap: DefaultGasCap, + FilterCap: DefaultFilterCap, } } @@ -207,6 +212,7 @@ func GetConfig(v *viper.Viper) Config { Address: v.GetString("json-rpc.address"), WsAddress: v.GetString("json-rpc.ws-address"), GasCap: v.GetUint64("json-rpc.gas-cap"), + FilterCap: v.GetUint64("json-rpc.filter-cap"), }, TLS: TLSConfig{ CertificatePath: v.GetString("tls.certificate-path"), diff --git a/server/config/toml.go b/server/config/toml.go index 6793e1da34..73bf049c25 100644 --- a/server/config/toml.go +++ b/server/config/toml.go @@ -35,6 +35,9 @@ api = "{{range $index, $elmt := .JSONRPC.API}}{{if $index}},{{$elmt}}{{else}}{{$ # GasCap sets a cap on gas that can be used in eth_call/estimateGas (0=infinite). Default: 25,000,000. gas-cap = {{ .JSONRPC.GasCap }} +# FilterCap sets the global cap for total number of filters that can be created +filter-cap = {{ .JSONRPC.FilterCap }} + ############################################################################### ### TLS Configuration ### ############################################################################### diff --git a/server/flags/flags.go b/server/flags/flags.go index fb8eaad0b1..1383e7c8f0 100644 --- a/server/flags/flags.go +++ b/server/flags/flags.go @@ -26,11 +26,12 @@ const ( // JSON-RPC flags const ( - JSONRPCEnable = "json-rpc.enable" - JSONRPCAPI = "json-rpc.api" - JSONRPCAddress = "json-rpc.address" - JSONWsAddress = "json-rpc.ws-address" - JSONRPCGasCap = "json-rpc.gas-cap" + JSONRPCEnable = "json-rpc.enable" + JSONRPCAPI = "json-rpc.api" + JSONRPCAddress = "json-rpc.address" + JSONWsAddress = "json-rpc.ws-address" + JSONRPCGasCap = "json-rpc.gas-cap" + JSONRPCFilterCap = "json-rpc.filter-cap" ) // EVM flags diff --git a/server/start.go b/server/start.go index 8b74452005..20fe1cde97 100644 --- a/server/start.go +++ b/server/start.go @@ -156,6 +156,7 @@ which accepts a path for the resulting pprof file. cmd.Flags().String(srvflags.JSONRPCAddress, config.DefaultJSONRPCAddress, "the JSON-RPC server address to listen on") cmd.Flags().String(srvflags.JSONWsAddress, config.DefaultJSONRPCWsAddress, "the JSON-RPC WS server address to listen on") cmd.Flags().Uint64(srvflags.JSONRPCGasCap, config.DefaultGasCap, "Sets a cap on gas that can be used in eth_call/estimateGas (0=infinite)") + cmd.Flags().Uint64(srvflags.JSONRPCFilterCap, config.DefaultFilterCap, "Sets the global cap for total number of filters that can be created") cmd.Flags().String(srvflags.EVMTracer, config.DefaultEVMTracer, "the EVM tracer type to collect execution traces from the EVM transaction execution (json|struct|access_list|markdown)") From f8fe748ed51f77dfd0d55f2554241bea269aa628 Mon Sep 17 00:00:00 2001 From: Devashish Dixit Date: Tue, 12 Oct 2021 13:25:01 +0800 Subject: [PATCH 2/7] Add defer statement for releasing locks --- rpc/ethereum/namespaces/eth/filters/api.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/rpc/ethereum/namespaces/eth/filters/api.go b/rpc/ethereum/namespaces/eth/filters/api.go index dff6ca5e41..52395124ab 100644 --- a/rpc/ethereum/namespaces/eth/filters/api.go +++ b/rpc/ethereum/namespaces/eth/filters/api.go @@ -110,21 +110,19 @@ func (api *PublicFilterAPI) timeoutLoop() { // https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_newPendingTransactionFilter func (api *PublicFilterAPI) NewPendingTransactionFilter() rpc.ID { api.filtersMu.Lock() + defer api.filtersMu.Unlock() if len(api.filters) >= int(api.backend.RPCFilterCap()) { - api.filtersMu.Unlock() return rpc.ID("error creating pending tx filter: max limit reached") } pendingTxSub, cancelSubs, err := api.events.SubscribePendingTxs() if err != nil { - api.filtersMu.Unlock() // wrap error on the ID return rpc.ID(fmt.Sprintf("error creating pending tx filter: %s", err.Error())) } api.filters[pendingTxSub.ID()] = &filter{typ: filters.PendingTransactionsSubscription, deadline: time.NewTimer(deadline), hashes: make([]common.Hash, 0), s: pendingTxSub} - api.filtersMu.Unlock() go func(txsCh <-chan coretypes.ResultEvent, errCh <-chan error) { defer cancelSubs() @@ -229,15 +227,14 @@ func (api *PublicFilterAPI) NewPendingTransactions(ctx context.Context) (*rpc.Su // https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_newblockfilter func (api *PublicFilterAPI) NewBlockFilter() rpc.ID { api.filtersMu.Lock() + defer api.filtersMu.Unlock() if len(api.filters) >= int(api.backend.RPCFilterCap()) { - api.filtersMu.Unlock() return rpc.ID("error creating block filter: max limit reached") } headerSub, cancelSubs, err := api.events.SubscribeNewHeads() if err != nil { - api.filtersMu.Unlock() // wrap error on the ID return rpc.ID(fmt.Sprintf("error creating block filter: %s", err.Error())) } @@ -246,7 +243,6 @@ func (api *PublicFilterAPI) NewBlockFilter() rpc.ID { baseFee := big.NewInt(params.InitialBaseFee) api.filters[headerSub.ID()] = &filter{typ: filters.BlocksSubscription, deadline: time.NewTimer(deadline), hashes: []common.Hash{}, s: headerSub} - api.filtersMu.Unlock() go func(headersCh <-chan coretypes.ResultEvent, errCh <-chan error) { defer cancelSubs() @@ -421,9 +417,9 @@ func (api *PublicFilterAPI) Logs(ctx context.Context, crit filters.FilterCriteri // https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_newfilter func (api *PublicFilterAPI) NewFilter(criteria filters.FilterCriteria) (rpc.ID, error) { api.filtersMu.Lock() + defer api.filtersMu.Unlock() if len(api.filters) >= int(api.backend.RPCFilterCap()) { - api.filtersMu.Unlock() return rpc.ID(""), fmt.Errorf("error creating filter: max limit reached") } @@ -434,14 +430,12 @@ func (api *PublicFilterAPI) NewFilter(criteria filters.FilterCriteria) (rpc.ID, logsSub, cancelSubs, err := api.events.SubscribeLogs(criteria) if err != nil { - api.filtersMu.Unlock() return rpc.ID(""), err } filterID = logsSub.ID() api.filters[filterID] = &filter{typ: filters.LogsSubscription, deadline: time.NewTimer(deadline), hashes: []common.Hash{}, s: logsSub} - api.filtersMu.Unlock() go func(eventCh <-chan coretypes.ResultEvent) { defer cancelSubs() From 1680e131555d5f28051443719f214c1efb526b84 Mon Sep 17 00:00:00 2001 From: Devashish Dixit Date: Wed, 13 Oct 2021 13:53:46 +0800 Subject: [PATCH 3/7] Change default value for filter cap to 200 --- server/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/config/config.go b/server/config/config.go index 376b08457b..88df768632 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -29,7 +29,7 @@ const ( DefaultGasCap uint64 = 25000000 - DefaultFilterCap uint64 = 2000000 + DefaultFilterCap uint64 = 200 ) var evmTracers = []string{DefaultEVMTracer, "markdown", "struct", "access_list"} From 368a3794cc0c9be56be540eeb17baebc06a827de Mon Sep 17 00:00:00 2001 From: Devashish Dixit Date: Wed, 13 Oct 2021 14:14:01 +0800 Subject: [PATCH 4/7] Changed data type of filter cap to int32 --- rpc/ethereum/backend/backend.go | 2 +- rpc/ethereum/namespaces/eth/filters/api.go | 2 +- server/config/config.go | 10 +++++++--- server/start.go | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/rpc/ethereum/backend/backend.go b/rpc/ethereum/backend/backend.go index 58846dcf65..33b72ade2e 100644 --- a/rpc/ethereum/backend/backend.go +++ b/rpc/ethereum/backend/backend.go @@ -766,7 +766,7 @@ func (e *EVMBackend) RPCGasCap() uint64 { } // RPCFilterCap is the limit for total number of filters that can be created -func (e *EVMBackend) RPCFilterCap() uint64 { +func (e *EVMBackend) RPCFilterCap() int32 { return e.cfg.JSONRPC.FilterCap } diff --git a/rpc/ethereum/namespaces/eth/filters/api.go b/rpc/ethereum/namespaces/eth/filters/api.go index 52395124ab..2922b60394 100644 --- a/rpc/ethereum/namespaces/eth/filters/api.go +++ b/rpc/ethereum/namespaces/eth/filters/api.go @@ -37,7 +37,7 @@ type Backend interface { GetFilteredBlocks(from int64, to int64, bloomIndexes [][]BloomIV, filterAddresses bool) ([]int64, error) - RPCFilterCap() uint64 + RPCFilterCap() int32 } // consider a filter inactive if it has not been polled for within deadline diff --git a/server/config/config.go b/server/config/config.go index 88df768632..9c4a8d6199 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -29,7 +29,7 @@ const ( DefaultGasCap uint64 = 25000000 - DefaultFilterCap uint64 = 200 + DefaultFilterCap int32 = 200 ) var evmTracers = []string{DefaultEVMTracer, "markdown", "struct", "access_list"} @@ -64,7 +64,7 @@ type JSONRPCConfig struct { // GasCap is the global gas cap for eth-call variants. GasCap uint64 `mapstructure:"gas-cap"` // FilterCap is the global cap for total number of filters that can be created. - FilterCap uint64 `mapstructure:"filter-cap"` + FilterCap int32 `mapstructure:"filter-cap"` } // TLSConfig defines the certificate and matching private key for the server. @@ -159,6 +159,10 @@ func (c JSONRPCConfig) Validate() error { return errors.New("cannot enable JSON-RPC without defining any API namespace") } + if c.FilterCap < 0 { + return errors.New("JSON-RPC filter-cap cannot be negative") + } + // TODO: validate APIs seenAPIs := make(map[string]bool) for _, api := range c.API { @@ -212,7 +216,7 @@ func GetConfig(v *viper.Viper) Config { Address: v.GetString("json-rpc.address"), WsAddress: v.GetString("json-rpc.ws-address"), GasCap: v.GetUint64("json-rpc.gas-cap"), - FilterCap: v.GetUint64("json-rpc.filter-cap"), + FilterCap: v.GetInt32("json-rpc.filter-cap"), }, TLS: TLSConfig{ CertificatePath: v.GetString("tls.certificate-path"), diff --git a/server/start.go b/server/start.go index 20fe1cde97..e2a387be5a 100644 --- a/server/start.go +++ b/server/start.go @@ -156,7 +156,7 @@ which accepts a path for the resulting pprof file. cmd.Flags().String(srvflags.JSONRPCAddress, config.DefaultJSONRPCAddress, "the JSON-RPC server address to listen on") cmd.Flags().String(srvflags.JSONWsAddress, config.DefaultJSONRPCWsAddress, "the JSON-RPC WS server address to listen on") cmd.Flags().Uint64(srvflags.JSONRPCGasCap, config.DefaultGasCap, "Sets a cap on gas that can be used in eth_call/estimateGas (0=infinite)") - cmd.Flags().Uint64(srvflags.JSONRPCFilterCap, config.DefaultFilterCap, "Sets the global cap for total number of filters that can be created") + cmd.Flags().Int32(srvflags.JSONRPCFilterCap, config.DefaultFilterCap, "Sets the global cap for total number of filters that can be created") cmd.Flags().String(srvflags.EVMTracer, config.DefaultEVMTracer, "the EVM tracer type to collect execution traces from the EVM transaction execution (json|struct|access_list|markdown)") From f513cf34623008e43a9937ce87530bd32c94c13b Mon Sep 17 00:00:00 2001 From: Devashish Dixit Date: Wed, 13 Oct 2021 14:17:30 +0800 Subject: [PATCH 5/7] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b06df5183..abfa49ed46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (evm) [tharsis#650](https://github.com/tharsis/ethermint/pull/650) Fix panic when flattening the cache context in case transaction is reverted. * (rpc, test) [tharsis#608](https://github.com/tharsis/ethermint/pull/608) Fix rpc test. +* (rpc) [tharsis#661](https://github.com/tharsis/ethermint/pull/661) Fix possibility of OOM error on creating too many filters using JSON-RPC. ## [v0.7.0] - 2021-10-07 From 225919d3af4d0c59937433a53075a1dcf90ce1d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Wed, 13 Oct 2021 11:00:48 +0200 Subject: [PATCH 6/7] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abfa49ed46..9f8d16f49e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (evm) [tharsis#650](https://github.com/tharsis/ethermint/pull/650) Fix panic when flattening the cache context in case transaction is reverted. * (rpc, test) [tharsis#608](https://github.com/tharsis/ethermint/pull/608) Fix rpc test. -* (rpc) [tharsis#661](https://github.com/tharsis/ethermint/pull/661) Fix possibility of OOM error on creating too many filters using JSON-RPC. +* (rpc) [tharsis#661](https://github.com/tharsis/ethermint/pull/661) Fix OOM bug when creating too many filters using JSON-RPC. ## [v0.7.0] - 2021-10-07 From 4888adee6bb0072dda12d755a076a6db164d5235 Mon Sep 17 00:00:00 2001 From: Devashish Dixit Date: Wed, 13 Oct 2021 17:54:10 +0800 Subject: [PATCH 7/7] Fix struct alignment --- server/config/config.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/config/config.go b/server/config/config.go index 9c4a8d6199..b1814b1f94 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -53,18 +53,18 @@ type EVMConfig struct { // JSONRPCConfig defines configuration for the EVM RPC server. type JSONRPCConfig struct { + // API defines a list of JSON-RPC namespaces that should be enabled + API []string `mapstructure:"api"` // Address defines the HTTP server to listen on Address string `mapstructure:"address"` // WsAddress defines the WebSocket server to listen on WsAddress string `mapstructure:"ws-address"` - // API defines a list of JSON-RPC namespaces that should be enabled - API []string `mapstructure:"api"` - // Enable defines if the EVM RPC server should be enabled. - Enable bool `mapstructure:"enable"` // GasCap is the global gas cap for eth-call variants. GasCap uint64 `mapstructure:"gas-cap"` // FilterCap is the global cap for total number of filters that can be created. FilterCap int32 `mapstructure:"filter-cap"` + // Enable defines if the EVM RPC server should be enabled. + Enable bool `mapstructure:"enable"` } // TLSConfig defines the certificate and matching private key for the server.