Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eth,rpc: allow for flag configured timeouts for eth_call #23645

Merged
merged 3 commits into from Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/geth/main.go
Expand Up @@ -173,6 +173,7 @@ var (
utils.IPCPathFlag,
utils.InsecureUnlockAllowedFlag,
utils.RPCGlobalGasCapFlag,
utils.RPCGlobalCallTimeoutFlag,
utils.RPCGlobalTxFeeCapFlag,
utils.AllowUnprotectedTxs,
}
Expand Down
1 change: 1 addition & 0 deletions cmd/geth/usage.go
Expand Up @@ -150,6 +150,7 @@ var AppHelpFlagGroups = []flags.FlagGroup{
utils.GraphQLCORSDomainFlag,
utils.GraphQLVirtualHostsFlag,
utils.RPCGlobalGasCapFlag,
utils.RPCGlobalCallTimeoutFlag,
utils.RPCGlobalTxFeeCapFlag,
utils.AllowUnprotectedTxs,
utils.JSpathFlag,
Expand Down
8 changes: 8 additions & 0 deletions cmd/utils/flags.go
Expand Up @@ -493,6 +493,11 @@ var (
Usage: "Sets a cap on gas that can be used in eth_call/estimateGas (0=infinite)",
Value: ethconfig.Defaults.RPCGasCap,
}
RPCGlobalCallTimeoutFlag = cli.DurationFlag{
Name: "rpc.calltimeout",
noam-alchemy marked this conversation as resolved.
Show resolved Hide resolved
noam-alchemy marked this conversation as resolved.
Show resolved Hide resolved
Usage: "Sets a timeout used for eth_call (0=infinite)",
Value: ethconfig.Defaults.RPCCallTimeout,
}
RPCGlobalTxFeeCapFlag = cli.Float64Flag{
Name: "rpc.txfeecap",
Usage: "Sets a cap on transaction fee (in ether) that can be sent via the RPC APIs (0 = no cap)",
Expand Down Expand Up @@ -1563,6 +1568,9 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
} else {
log.Info("Global gas cap disabled")
}
if ctx.GlobalIsSet(RPCGlobalCallTimeoutFlag.Name) {
cfg.RPCCallTimeout = ctx.GlobalDuration(RPCGlobalCallTimeoutFlag.Name)
}
if ctx.GlobalIsSet(RPCGlobalTxFeeCapFlag.Name) {
cfg.RPCTxFeeCap = ctx.GlobalFloat64(RPCGlobalTxFeeCapFlag.Name)
}
Expand Down
5 changes: 5 additions & 0 deletions eth/api_backend.go
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"math/big"
"time"

"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/accounts"
Expand Down Expand Up @@ -315,6 +316,10 @@ func (b *EthAPIBackend) RPCGasCap() uint64 {
return b.eth.config.RPCGasCap
}

func (b *EthAPIBackend) RPCCallTimeout() time.Duration {
return b.eth.config.RPCCallTimeout
}

func (b *EthAPIBackend) RPCTxFeeCap() float64 {
return b.eth.config.RPCTxFeeCap
}
Expand Down
12 changes: 8 additions & 4 deletions eth/ethconfig/config.go
Expand Up @@ -87,10 +87,11 @@ var Defaults = Config{
GasPrice: big.NewInt(params.GWei),
Recommit: 3 * time.Second,
},
TxPool: core.DefaultTxPoolConfig,
RPCGasCap: 50000000,
GPO: FullNodeGPO,
RPCTxFeeCap: 1, // 1 ether
TxPool: core.DefaultTxPoolConfig,
RPCGasCap: 50000000,
RPCCallTimeout: 5 * time.Second,
GPO: FullNodeGPO,
RPCTxFeeCap: 1, // 1 ether
}

func init() {
Expand Down Expand Up @@ -188,6 +189,9 @@ type Config struct {
// RPCGasCap is the global gas cap for eth-call variants.
RPCGasCap uint64

// RPCCallTimeout is the global timeout for eth-call.
RPCCallTimeout time.Duration

// RPCTxFeeCap is the global transaction fee(price * gaslimit) cap for
// send-transction variants. The unit is ether.
RPCTxFeeCap float64
Expand Down
6 changes: 6 additions & 0 deletions eth/ethconfig/gen_config.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions graphql/graphql.go
Expand Up @@ -23,7 +23,6 @@ import (
"fmt"
"math/big"
"strconv"
"time"

"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -954,7 +953,7 @@ func (b *Block) Call(ctx context.Context, args struct {
return nil, err
}
}
result, err := ethapi.DoCall(ctx, b.backend, args.Data, *b.numberOrHash, nil, 5*time.Second, b.backend.RPCGasCap())
result, err := ethapi.DoCall(ctx, b.backend, args.Data, *b.numberOrHash, nil, b.backend.RPCCallTimeout(), b.backend.RPCGasCap())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1024,7 +1023,7 @@ func (p *Pending) Call(ctx context.Context, args struct {
Data ethapi.TransactionArgs
}) (*CallResult, error) {
pendingBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.PendingBlockNumber)
result, err := ethapi.DoCall(ctx, p.backend, args.Data, pendingBlockNr, nil, 5*time.Second, p.backend.RPCGasCap())
result, err := ethapi.DoCall(ctx, p.backend, args.Data, pendingBlockNr, nil, p.backend.RPCCallTimeout(), p.backend.RPCGasCap())
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/ethapi/api.go
Expand Up @@ -972,7 +972,7 @@ func (e *revertError) ErrorData() interface{} {
// Note, this function doesn't make and changes in the state/blockchain and is
// useful to execute and retrieve values.
func (s *PublicBlockChainAPI) Call(ctx context.Context, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride) (hexutil.Bytes, error) {
result, err := DoCall(ctx, s.b, args, blockNrOrHash, overrides, 5*time.Second, s.b.RPCGasCap())
result, err := DoCall(ctx, s.b, args, blockNrOrHash, overrides, s.b.RPCCallTimeout(), s.b.RPCGasCap())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also put this timeout in DoEstimateCall

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming you mean DoEstimateGas? Wouldn't that be a regression for anyone using eth_estimateGas before sending XL transactions? Currently estimateGas would not time out while running the EVM call. Adding this change here would enforce a new default limit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github's making me resolve this conversation because of the requested changes, but the open question still stands.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes right. The original timeout for Estimate is 0(no timeout), lgtm.

if err != nil {
return nil, err
}
Expand Down
8 changes: 5 additions & 3 deletions internal/ethapi/backend.go
Expand Up @@ -20,6 +20,7 @@ package ethapi
import (
"context"
"math/big"
"time"

"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/accounts"
Expand Down Expand Up @@ -47,9 +48,10 @@ type Backend interface {
ChainDb() ethdb.Database
AccountManager() *accounts.Manager
ExtRPCEnabled() bool
RPCGasCap() uint64 // global gas cap for eth_call over rpc: DoS protection
RPCTxFeeCap() float64 // global tx fee cap for all transaction related APIs
UnprotectedAllowed() bool // allows only for EIP155 transactions.
RPCGasCap() uint64 // global gas cap for eth_call over rpc: DoS protection
RPCCallTimeout() time.Duration // global timeout for eth_call over rpc: DoS protection
RPCTxFeeCap() float64 // global tx fee cap for all transaction related APIs
UnprotectedAllowed() bool // allows only for EIP155 transactions.

// Blockchain API
SetHead(number uint64)
Expand Down
5 changes: 5 additions & 0 deletions les/api_backend.go
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"math/big"
"time"

"github.com/ethereum/go-ethereum"
"github.com/ethereum/go-ethereum/accounts"
Expand Down Expand Up @@ -293,6 +294,10 @@ func (b *LesApiBackend) RPCGasCap() uint64 {
return b.eth.config.RPCGasCap
}

func (b *LesApiBackend) RPCCallTimeout() time.Duration {
return b.eth.config.RPCCallTimeout
}

func (b *LesApiBackend) RPCTxFeeCap() float64 {
return b.eth.config.RPCTxFeeCap
}
Expand Down