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.

Sign up for GitHub

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

Jump to bottom

internal/ethapi: disable sending of non eip155 replay protected tx #22339

Merged
merged 9 commits into from
Feb 23, 2021
Merged

internal/ethapi: disable sending of non eip155 replay protected tx #22339

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1d71eb7
internal/ethapi: disable sending of non eip155 replay protected trans…
MariusVanDerWijden Feb 17, 2021
b1f1634
internal/ethapi: improved error message
MariusVanDerWijden Feb 17, 2021
56ff034
internal/ethapi: improved error message
MariusVanDerWijden Feb 17, 2021
ede6175
node: add EIP155Required option
MariusVanDerWijden Feb 17, 2021
90f7ab2
node: rename EIP155Required to UnprotectedAllowed
MariusVanDerWijden Feb 17, 2021
671e748
cmd: add flag
MariusVanDerWijden Feb 18, 2021
82208ad
fixed read from config, renamed
MariusVanDerWijden Feb 18, 2021
d73a0ea
typo
MariusVanDerWijden Feb 18, 2021
80b4c42
cmd: wrong logic
MariusVanDerWijden Feb 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions cmd/utils/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,10 @@ var (
Name: "preload",
Usage: "Comma separated list of JavaScript files to preload into the console",
}
AllowUnprotectedTxs = cli.BoolFlag{
Name: "rpc.allow-unprotected-txs",
Usage: "Allow for unprotected (non EIP155 signed transactions to be submitted via RPC",
}

// Network Settings
MaxPeersFlag = cli.IntFlag{
Expand Down Expand Up @@ -966,6 +970,10 @@ func setHTTP(ctx *cli.Context, cfg *node.Config) {
if ctx.GlobalIsSet(HTTPPathPrefixFlag.Name) {
cfg.HTTPPathPrefix = ctx.GlobalString(HTTPPathPrefixFlag.Name)
}

if ctx.GlobalIsSet(AllowUnprotectedTxs.Name) {
Copy link
Member

Choose a reason for hiding this comment

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

There is no reason to check if it's set or not. That's only needed if the default has some special meaning. We only care about the actual value of the flag, so pls just use cfg.UnprotectedAllowed = ctx.GlobalBool(AllowUnprotectedTxs.Name)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes there is, when we simply override it here, then we will always overwrite the value we read out of the config.
See martins comments below

cfg.UnprotectedAllowed = ctx.GlobalBool(AllowUnprotectedTxs.Name)
}
}

// setGraphQL creates the GraphQL listener interface string from the set
Expand Down
11 changes: 8 additions & 3 deletions eth/api_backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ import (

// EthAPIBackend implements ethapi.Backend for full nodes
type EthAPIBackend struct {
extRPCEnabled bool
eth *Ethereum
gpo *gasprice.Oracle
extRPCEnabled bool
unprotectedAllowed bool
eth *Ethereum
gpo *gasprice.Oracle
}

// ChainConfig returns the active chain configuration.
Expand Down Expand Up @@ -292,6 +293,10 @@ func (b *EthAPIBackend) ExtRPCEnabled() bool {
return b.extRPCEnabled
}

func (b *EthAPIBackend) UnprotectedAllowed() bool {
return b.unprotectedAllowed
}

func (b *EthAPIBackend) RPCGasCap() uint64 {
return b.eth.config.RPCGasCap
}
Expand Down
2 changes: 1 addition & 1 deletion eth/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) {
eth.miner = miner.New(eth, &config.Miner, chainConfig, eth.EventMux(), eth.engine, eth.isLocalBlock)
eth.miner.SetExtra(makeExtraData(config.Miner.ExtraData))

eth.APIBackend = &EthAPIBackend{stack.Config().ExtRPCEnabled(), eth, nil}
eth.APIBackend = &EthAPIBackend{stack.Config().ExtRPCEnabled(), stack.Config().UnprotectedAllowed, eth, nil}
gpoParams := config.GPO
if gpoParams.Default == nil {
gpoParams.Default = config.Miner.GasPrice
Expand Down
4 changes: 4 additions & 0 deletions internal/ethapi/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1555,6 +1555,10 @@ func SubmitTransaction(ctx context.Context, b Backend, tx *types.Transaction) (c
if err := checkTxFee(tx.GasPrice(), tx.Gas(), b.RPCTxFeeCap()); err != nil {
return common.Hash{}, err
}
if !b.UnprotectedAllowed() && !tx.Protected() {
// Ensure only eip155 signed transactions are submitted if EIP155Required is set.
return common.Hash{}, errors.New("only replay-protected (EIP-155) transactions allowed over RPC")
}
if err := b.SendTx(ctx, tx); err != nil {
return common.Hash{}, err
}
Expand Down
5 changes: 3 additions & 2 deletions internal/ethapi/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ 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
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.

// Blockchain API
SetHead(number uint64)
Expand Down
11 changes: 8 additions & 3 deletions les/api_backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ import (
)

type LesApiBackend struct {
extRPCEnabled bool
eth *LightEthereum
gpo *gasprice.Oracle
extRPCEnabled bool
unprotectedAllowed bool
eth *LightEthereum
gpo *gasprice.Oracle
}

func (b *LesApiBackend) ChainConfig() *params.ChainConfig {
Expand Down Expand Up @@ -263,6 +264,10 @@ func (b *LesApiBackend) ExtRPCEnabled() bool {
return b.extRPCEnabled
}

func (b *LesApiBackend) UnprotectedAllowed() bool {
return b.unprotectedAllowed
}

func (b *LesApiBackend) RPCGasCap() uint64 {
return b.eth.config.RPCGasCap
}
Expand Down
2 changes: 1 addition & 1 deletion les/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func New(stack *node.Node, config *ethconfig.Config) (*LightEthereum, error) {
rawdb.WriteChainConfig(chainDb, genesisHash, chainConfig)
}

leth.ApiBackend = &LesApiBackend{stack.Config().ExtRPCEnabled(), leth, nil}
leth.ApiBackend = &LesApiBackend{stack.Config().ExtRPCEnabled(), stack.Config().UnprotectedAllowed, leth, nil}
gpoParams := config.GPO
if gpoParams.Default == nil {
gpoParams.Default = config.Miner.GasPrice
Expand Down
3 changes: 3 additions & 0 deletions node/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ type Config struct {
staticNodesWarning bool
trustedNodesWarning bool
oldGethResourceWarning bool

// Require all transactions to be protected with EIP-155 to prevent replaying on other chains.
MariusVanDerWijden marked this conversation as resolved.
Show resolved Hide resolved
UnprotectedAllowed bool `toml:",omitempty"`
}

// IPCEndpoint resolves an IPC endpoint based on a configured value, taking into
Expand Down