From 6cd2989c582ef2377d77089e544efb2a3fe7b37c Mon Sep 17 00:00:00 2001 From: SatpalSandhu61 Date: Wed, 7 Aug 2019 15:35:27 +0100 Subject: [PATCH 1/3] Add allowedFutureBlockTime flag for Istanbul. --- cmd/geth/main.go | 1 + cmd/geth/usage.go | 4 +++- cmd/utils/flags.go | 10 +++++++++- consensus/istanbul/backend/engine.go | 6 +++--- consensus/istanbul/backend/engine_test.go | 12 ++++++++++++ consensus/istanbul/config.go | 10 ++++++---- 6 files changed, 34 insertions(+), 9 deletions(-) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 701d608bba..847e7d14c9 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -141,6 +141,7 @@ var ( utils.EmitCheckpointsFlag, utils.IstanbulRequestTimeoutFlag, utils.IstanbulBlockPeriodFlag, + utils.IstanbulAllowedFutureBlockTimeFlag, } rpcFlags = []cli.Flag{ diff --git a/cmd/geth/usage.go b/cmd/geth/usage.go index 40fe83acb2..77be16cb97 100644 --- a/cmd/geth/usage.go +++ b/cmd/geth/usage.go @@ -273,11 +273,13 @@ var AppHelpFlagGroups = []flagGroup{ }, { Name: "MISC", - },{ + }, + { Name: "ISTANBUL", Flags: []cli.Flag{ utils.IstanbulRequestTimeoutFlag, utils.IstanbulBlockPeriodFlag, + utils.IstanbulAllowedFutureBlockTimeFlag, }, }, } diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index b0a190235d..9b62c71136 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -26,6 +26,7 @@ import ( "path/filepath" "strconv" "strings" + "time" "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/accounts/keystore" @@ -57,7 +58,6 @@ import ( "github.com/ethereum/go-ethereum/params" whisper "github.com/ethereum/go-ethereum/whisper/whisperv6" "gopkg.in/urfave/cli.v1" - "time" ) var ( @@ -618,6 +618,11 @@ var ( Usage: "Default minimum difference between two consecutive block's timestamps in seconds", Value: eth.DefaultConfig.Istanbul.BlockPeriod, } + IstanbulAllowedFutureBlockTimeFlag = cli.Uint64Flag{ + Name: "istanbul.allowedfutureblocktime", + Usage: "Time threshold allowed when detecting future blocks in seconds", + Value: eth.DefaultConfig.Istanbul.AllowedFutureBlockTime, + } // Metrics flags MetricsEnabledFlag = cli.BoolFlag{ @@ -1126,6 +1131,9 @@ func setIstanbul(ctx *cli.Context, cfg *eth.Config) { if ctx.GlobalIsSet(IstanbulBlockPeriodFlag.Name) { cfg.Istanbul.BlockPeriod = ctx.GlobalUint64(IstanbulBlockPeriodFlag.Name) } + if ctx.GlobalIsSet(IstanbulAllowedFutureBlockTimeFlag.Name) { + cfg.Istanbul.AllowedFutureBlockTime = ctx.GlobalUint64(IstanbulAllowedFutureBlockTimeFlag.Name) + } } // checkExclusive verifies that only a single instance of the provided flags was diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go index 3fd681bd2c..8e4cd02185 100644 --- a/consensus/istanbul/backend/engine.go +++ b/consensus/istanbul/backend/engine.go @@ -119,8 +119,9 @@ func (sb *backend) verifyHeader(chain consensus.ChainReader, header *types.Heade return errUnknownBlock } - // Don't waste time checking blocks from the future - if header.Time.Cmp(big.NewInt(now().Unix())) > 0 { + // Don't waste time checking blocks from the future (adjusting for allowed threshold) + adjustedTimeNow := now().Add(time.Duration(sb.config.AllowedFutureBlockTime) * time.Second).Unix() + if header.Time.Cmp(big.NewInt(adjustedTimeNow)) > 0 { return consensus.ErrFutureBlock } @@ -616,7 +617,6 @@ func sigHash(header *types.Header) (hash common.Hash) { return hash } - // SealHash returns the hash of a block prior to it being sealed. func (sb *backend) SealHash(header *types.Header) common.Hash { return sigHash(header) diff --git a/consensus/istanbul/backend/engine_test.go b/consensus/istanbul/backend/engine_test.go index d14032afca..6b93e3edde 100644 --- a/consensus/istanbul/backend/engine_test.go +++ b/consensus/istanbul/backend/engine_test.go @@ -311,6 +311,18 @@ func TestVerifyHeader(t *testing.T) { t.Errorf("error mismatch: have %v, want %v", err, consensus.ErrFutureBlock) } + // future block which is within AllowedFutureBlockTime + block = makeBlockWithoutSeal(chain, engine, chain.Genesis()) + header = block.Header() + header.Time = new(big.Int).Add(big.NewInt(now().Unix()), new(big.Int).SetUint64(10)) + priorValue := engine.config.AllowedFutureBlockTime + engine.config.AllowedFutureBlockTime = 10 + err = engine.VerifyHeader(chain, header, false) + engine.config.AllowedFutureBlockTime = priorValue //restore changed value + if err == consensus.ErrFutureBlock { + t.Errorf("error mismatch: have %v, want nil", err) + } + // invalid nonce block = makeBlockWithoutSeal(chain, engine, chain.Genesis()) header = block.Header() diff --git a/consensus/istanbul/config.go b/consensus/istanbul/config.go index d2d44bf2e8..9eebd9379c 100644 --- a/consensus/istanbul/config.go +++ b/consensus/istanbul/config.go @@ -24,10 +24,11 @@ const ( ) type Config struct { - RequestTimeout uint64 `toml:",omitempty"` // The timeout for each Istanbul round in milliseconds. - BlockPeriod uint64 `toml:",omitempty"` // Default minimum difference between two consecutive block's timestamps in second - ProposerPolicy ProposerPolicy `toml:",omitempty"` // The policy for proposer selection - Epoch uint64 `toml:",omitempty"` // The number of blocks after which to checkpoint and reset the pending votes + RequestTimeout uint64 `toml:",omitempty"` // The timeout for each Istanbul round in milliseconds. + BlockPeriod uint64 `toml:",omitempty"` // Default minimum difference between two consecutive block's timestamps in second + ProposerPolicy ProposerPolicy `toml:",omitempty"` // The policy for proposer selection + Epoch uint64 `toml:",omitempty"` // The number of blocks after which to checkpoint and reset the pending votes + AllowedFutureBlockTime uint64 `toml:",omitempty"` // Time threshold allowed when detecting future blocks, in seconds } var DefaultConfig = &Config{ @@ -35,4 +36,5 @@ var DefaultConfig = &Config{ BlockPeriod: 1, ProposerPolicy: RoundRobin, Epoch: 30000, + AllowedFutureBlockTime: 1, } From 74b7c959670fd3f3a30c660aa258f7af0fd3e2fa Mon Sep 17 00:00:00 2001 From: SatpalSandhu61 Date: Tue, 17 Mar 2020 11:44:31 +0000 Subject: [PATCH 2/3] Change flag description to match comment in upstream --- cmd/utils/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index fe9e6c227c..d5b8adb9a1 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -649,7 +649,7 @@ var ( } IstanbulAllowedFutureBlockTimeFlag = cli.Uint64Flag{ Name: "istanbul.allowedfutureblocktime", - Usage: "Time threshold allowed when detecting future blocks in seconds", + Usage: "Max time (in seconds) from current time allowed for blocks, before they're considered future blocks", Value: eth.DefaultConfig.Istanbul.AllowedFutureBlockTime, } From 32cdcae1c464683789eb86250d7a01276d4ed22f Mon Sep 17 00:00:00 2001 From: SatpalSandhu61 Date: Fri, 20 Mar 2020 14:17:34 +0000 Subject: [PATCH 3/3] Make flag generic, so it also applies to Clique. --- cmd/geth/main.go | 2 +- cmd/geth/usage.go | 2 +- cmd/utils/flags.go | 15 +++++++-------- consensus/clique/clique.go | 5 +++-- consensus/istanbul/backend/engine.go | 4 ++-- consensus/istanbul/config.go | 14 +++++++------- eth/backend.go | 2 ++ eth/config.go | 3 ++- eth/handler_test.go | 12 ++++++------ params/config.go | 15 +++++++++------ 10 files changed, 40 insertions(+), 34 deletions(-) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 4e6aba5fa6..5e606251b4 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -145,11 +145,11 @@ var ( utils.EmitCheckpointsFlag, utils.IstanbulRequestTimeoutFlag, utils.IstanbulBlockPeriodFlag, - utils.IstanbulAllowedFutureBlockTimeFlag, utils.PluginSettingsFlag, utils.PluginSkipVerifyFlag, utils.PluginLocalVerifyFlag, utils.PluginPublicKeyFlag, + utils.AllowedFutureBlockTimeFlag, // End-Quorum } diff --git a/cmd/geth/usage.go b/cmd/geth/usage.go index 8071eebde1..024adc919b 100644 --- a/cmd/geth/usage.go +++ b/cmd/geth/usage.go @@ -145,6 +145,7 @@ var AppHelpFlagGroups = []flagGroup{ utils.PluginSkipVerifyFlag, utils.PluginLocalVerifyFlag, utils.PluginPublicKeyFlag, + utils.AllowedFutureBlockTimeFlag, }, }, { @@ -269,7 +270,6 @@ var AppHelpFlagGroups = []flagGroup{ Flags: []cli.Flag{ utils.IstanbulRequestTimeoutFlag, utils.IstanbulBlockPeriodFlag, - utils.IstanbulAllowedFutureBlockTimeFlag, }, }, { diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index d5b8adb9a1..f9c9c0601d 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -619,6 +619,11 @@ var ( Name: "permissioned", Usage: "If enabled, the node will allow only a defined list of nodes to connect", } + AllowedFutureBlockTimeFlag = cli.Uint64Flag{ + Name: "allowedfutureblocktime", + Usage: "Max time (in seconds) from current time allowed for blocks, before they're considered future blocks", + Value: 0, + } // Plugins settings PluginSettingsFlag = cli.StringFlag{ Name: "plugins", @@ -647,11 +652,6 @@ var ( Usage: "Default minimum difference between two consecutive block's timestamps in seconds", Value: eth.DefaultConfig.Istanbul.BlockPeriod, } - IstanbulAllowedFutureBlockTimeFlag = cli.Uint64Flag{ - Name: "istanbul.allowedfutureblocktime", - Usage: "Max time (in seconds) from current time allowed for blocks, before they're considered future blocks", - Value: eth.DefaultConfig.Istanbul.AllowedFutureBlockTime, - } // Metrics flags MetricsEnabledFlag = cli.BoolFlag{ @@ -1206,9 +1206,6 @@ func setIstanbul(ctx *cli.Context, cfg *eth.Config) { if ctx.GlobalIsSet(IstanbulBlockPeriodFlag.Name) { cfg.Istanbul.BlockPeriod = ctx.GlobalUint64(IstanbulBlockPeriodFlag.Name) } - if ctx.GlobalIsSet(IstanbulAllowedFutureBlockTimeFlag.Name) { - cfg.Istanbul.AllowedFutureBlockTime = ctx.GlobalUint64(IstanbulAllowedFutureBlockTimeFlag.Name) - } } // checkExclusive verifies that only a single instance of the provided flags was @@ -1278,6 +1275,8 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *eth.Config) { setEthash(ctx, cfg) setIstanbul(ctx, cfg) + cfg.AllowedFutureBlockTime = ctx.GlobalUint64(AllowedFutureBlockTimeFlag.Name) //Quorum + if ctx.GlobalIsSet(SyncModeFlag.Name) { cfg.SyncMode = *GlobalTextMarshaler(ctx, SyncModeFlag.Name).(*downloader.SyncMode) } diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index dc2614dfa3..41ef68b659 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -278,8 +278,9 @@ func (c *Clique) verifyHeader(chain consensus.ChainReader, header *types.Header, } number := header.Number.Uint64() - // Don't waste time checking blocks from the future - if header.Time.Cmp(big.NewInt(time.Now().Unix())) > 0 { + // Don't waste time checking blocks from the future (adjusting for allowed threshold) + adjustedTimeNow := time.Now().Add(time.Duration(c.config.AllowedFutureBlockTime) * time.Second).Unix() + if header.Time.Cmp(big.NewInt(adjustedTimeNow)) > 0 { return consensus.ErrFutureBlock } // Checkpoint blocks need to enforce zero beneficiary diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go index ae8a8fd687..264e384445 100644 --- a/consensus/istanbul/backend/engine.go +++ b/consensus/istanbul/backend/engine.go @@ -27,8 +27,8 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/consensus" "github.com/ethereum/go-ethereum/consensus/istanbul" - "github.com/ethereum/go-ethereum/consensus/istanbul/validator" istanbulCore "github.com/ethereum/go-ethereum/consensus/istanbul/core" + "github.com/ethereum/go-ethereum/consensus/istanbul/validator" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto/sha3" @@ -309,7 +309,7 @@ func (sb *backend) verifyCommittedSeals(chain consensus.ChainReader, header *typ return errInvalidCommittedSeals } - // The length of validSeal should be larger than number of faulty node + 1 + // The length of validSeal should be larger than number of faulty node + 1 if validSeal <= snap.ValSet.F() { return errInvalidCommittedSeals } diff --git a/consensus/istanbul/config.go b/consensus/istanbul/config.go index 9531458772..dfe6c087d0 100644 --- a/consensus/istanbul/config.go +++ b/consensus/istanbul/config.go @@ -31,14 +31,14 @@ type Config struct { ProposerPolicy ProposerPolicy `toml:",omitempty"` // The policy for proposer selection Epoch uint64 `toml:",omitempty"` // The number of blocks after which to checkpoint and reset the pending votes Ceil2Nby3Block *big.Int `toml:",omitempty"` // Number of confirmations required to move from one state to next [2F + 1 to Ceil(2N/3)] - AllowedFutureBlockTime uint64 `toml:",omitempty"` // Time threshold allowed when detecting future blocks, in seconds + AllowedFutureBlockTime uint64 `toml:",omitempty"` // Max time (in seconds) from current time allowed for blocks, before they're considered future blocks } var DefaultConfig = &Config{ - RequestTimeout: 10000, - BlockPeriod: 1, - ProposerPolicy: RoundRobin, - Epoch: 30000, - Ceil2Nby3Block: big.NewInt(0), - AllowedFutureBlockTime: 1, + RequestTimeout: 10000, + BlockPeriod: 1, + ProposerPolicy: RoundRobin, + Epoch: 30000, + Ceil2Nby3Block: big.NewInt(0), + AllowedFutureBlockTime: 0, } diff --git a/eth/backend.go b/eth/backend.go index d7af9b20fd..9906671e6b 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -250,6 +250,7 @@ func CreateDB(ctx *node.ServiceContext, config *Config, name string) (ethdb.Data func CreateConsensusEngine(ctx *node.ServiceContext, chainConfig *params.ChainConfig, config *Config, notify []string, noverify bool, db ethdb.Database) consensus.Engine { // If proof-of-authority is requested, set it up if chainConfig.Clique != nil { + chainConfig.Clique.AllowedFutureBlockTime = config.AllowedFutureBlockTime //Quorum return clique.New(chainConfig.Clique, db) } // If Istanbul is requested, set it up @@ -259,6 +260,7 @@ func CreateConsensusEngine(ctx *node.ServiceContext, chainConfig *params.ChainCo } config.Istanbul.ProposerPolicy = istanbul.ProposerPolicy(chainConfig.Istanbul.ProposerPolicy) config.Istanbul.Ceil2Nby3Block = chainConfig.Istanbul.Ceil2Nby3Block + config.Istanbul.AllowedFutureBlockTime = config.AllowedFutureBlockTime return istanbulBackend.New(&config.Istanbul, ctx.NodeKey(), db) } diff --git a/eth/config.go b/eth/config.go index 751a659d5d..f104cdc95f 100644 --- a/eth/config.go +++ b/eth/config.go @@ -128,7 +128,8 @@ type Config struct { Istanbul istanbul.Config // Miscellaneous options - DocRoot string `toml:"-"` + DocRoot string `toml:"-"` + AllowedFutureBlockTime uint64 //Quorum // Type of the EWASM interpreter ("" for default) EWASMInterpreter string diff --git a/eth/handler_test.go b/eth/handler_test.go index da3a779ef3..7f0ebbff49 100644 --- a/eth/handler_test.go +++ b/eth/handler_test.go @@ -82,8 +82,8 @@ func TestNodeInfo(t *testing.T) { }{ {"ethash", nil, nil, false}, {"raft", nil, nil, true}, - {"istanbul", nil, ¶ms.IstanbulConfig{1, 1, big.NewInt(0)}, false}, - {"clique", ¶ms.CliqueConfig{1, 1}, nil, false}, + {"istanbul", nil, ¶ms.IstanbulConfig{1, 1, big.NewInt(0), 0}, false}, + {"clique", ¶ms.CliqueConfig{1, 1, 0}, nil, false}, } // Make sure anything we screw up is restored @@ -285,10 +285,10 @@ func testGetBlockBodies(t *testing.T, protocol int) { available []bool // Availability of explicitly requested blocks expected int // Total number of existing blocks to expect }{ - {1, nil, nil, 1}, // A single random block should be retrievable - {10, nil, nil, 10}, // Multiple random blocks should be retrievable - {limit, nil, nil, limit}, // The maximum possible blocks should be retrievable - {limit + 1, nil, nil, limit}, // No more than the possible block count should be returned + {1, nil, nil, 1}, // A single random block should be retrievable + {10, nil, nil, 10}, // Multiple random blocks should be retrievable + {limit, nil, nil, limit}, // The maximum possible blocks should be retrievable + {limit + 1, nil, nil, limit}, // No more than the possible block count should be returned {0, []common.Hash{pm.blockchain.Genesis().Hash()}, []bool{true}, 1}, // The genesis block should be retrievable {0, []common.Hash{pm.blockchain.CurrentBlock().Hash()}, []bool{true}, 1}, // The chains head block should be retrievable {0, []common.Hash{{}}, []bool{false}, 0}, // A non existent block should not be returned diff --git a/params/config.go b/params/config.go index d16afa6a55..d0e56e649a 100644 --- a/params/config.go +++ b/params/config.go @@ -195,7 +195,7 @@ type ChainConfig struct { // Quorum // // QIP714Block implements the permissions related changes - QIP714Block *big.Int `json:"qip714Block,omitempty"` + QIP714Block *big.Int `json:"qip714Block,omitempty"` MaxCodeSizeChangeBlock *big.Int `json:"maxCodeSizeChangeBlock,omitempty"` } @@ -209,8 +209,9 @@ func (c *EthashConfig) String() string { // CliqueConfig is the consensus engine configs for proof-of-authority based sealing. type CliqueConfig struct { - Period uint64 `json:"period"` // Number of seconds between blocks to enforce - Epoch uint64 `json:"epoch"` // Epoch length to reset votes and checkpoint + Period uint64 `json:"period"` // Number of seconds between blocks to enforce + Epoch uint64 `json:"epoch"` // Epoch length to reset votes and checkpoint + AllowedFutureBlockTime uint64 `json:"allowedFutureBlockTime"` // Max time (in seconds) from current time allowed for blocks, before they're considered future blocks } // String implements the stringer interface, returning the consensus engine details. @@ -220,9 +221,10 @@ func (c *CliqueConfig) String() string { // IstanbulConfig is the consensus engine configs for Istanbul based sealing. type IstanbulConfig struct { - Epoch uint64 `json:"epoch"` // Epoch length to reset votes and checkpoint - ProposerPolicy uint64 `json:"policy"` // The policy for proposer selection - Ceil2Nby3Block *big.Int `json:"ceil2Nby3Block,omitempty"` // Number of confirmations required to move from one state to next [2F + 1 to Ceil(2N/3)] + Epoch uint64 `json:"epoch"` // Epoch length to reset votes and checkpoint + ProposerPolicy uint64 `json:"policy"` // The policy for proposer selection + Ceil2Nby3Block *big.Int `json:"ceil2Nby3Block,omitempty"` // Number of confirmations required to move from one state to next [2F + 1 to Ceil(2N/3)] + AllowedFutureBlockTime uint64 `json:"allowedFutureBlockTime"` // Max time (in seconds) from current time allowed for blocks, before they're considered future blocks } // String implements the stringer interface, returning the consensus engine details. @@ -319,6 +321,7 @@ func (c *ChainConfig) IsEWASM(num *big.Int) bool { func (c *ChainConfig) IsQIP714(num *big.Int) bool { return isForked(c.QIP714Block, num) } + // Quorum // // IsMaxCodeSizeChangeBlock returns whether num represents a block number max code size