CIP-21 Governable "lookbackWindow" parameter

.circleci/config.yml

@@ -10,7 +10,7 @@ executors:
     working_directory: ~/repos/celo-monorepo/packages/celotool
     environment:
       GO_VERSION: "1.14.14"
-      CELO_MONOREPO_BRANCH_TO_TEST: master
+      CELO_MONOREPO_BRANCH_TO_TEST: andres-dg/loopbackwindow-refactor
       GITHUB_RSA_FINGERPRINT: SHA256:nThbg6kXUpJWGl7E1IGOCspRomTxdCARLviKw6E5SY8
 jobs:
   build-geth:
@@ -183,7 +183,7 @@ jobs:
 
   end-to-end-governance-test:
     executor: e2e
-    resource_class: large
+    resource_class: xlarge
     steps:
       - attach_workspace:
           at: ~/repos
@@ -235,7 +235,7 @@ jobs:
 
   end-to-end-validator-order-test:
     executor: e2e
-    resource_class: large
+    resource_class: xlarge
     steps:
       - attach_workspace:
           at: ~/repos

cmd/utils/flags.go

@@ -136,6 +136,7 @@ func printHelp(out io.Writer, templ string, data interface{}) {
 
 var (
 	// General settings
+
 	DataDirFlag = DirectoryFlag{
 		Name:  "datadir",
 		Usage: "Data directory for the databases and keystore",
@@ -237,7 +238,9 @@ var (
 		Usage: "Public address for block mining BLS signatures (default = first account created)",
 		Value: "0",
 	}
+
 	// Light server and client settings
+
 	LightServeFlag = cli.IntFlag{
 		Name:  "light.serve",
 		Usage: "Maximum percentage of time allowed for serving LES requests (multi-threaded processing allows values over 100)",
@@ -277,7 +280,9 @@ var (
 		Name:  "ulc.onlyannounce",
 		Usage: "Ultra light server sends announcements only",
 	}
+
 	// Transaction pool settings
+
 	TxPoolLocalsFlag = cli.StringFlag{
 		Name:  "txpool.locals",
 		Usage: "Comma separated accounts to treat as locals (no flush, priority inclusion)",
@@ -331,7 +336,9 @@ var (
 		Usage: "Maximum amount of time non-executable transaction are queued",
 		Value: eth.DefaultConfig.TxPool.Lifetime,
 	}
+
 	// Performance tuning settings
+
 	CacheFlag = cli.IntFlag{
 		Name:  "cache",
 		Usage: "Megabytes of memory allocated to internal caching (default = 4096 mainnet full node, 128 light mode)",
@@ -356,7 +363,9 @@ var (
 		Name:  "cache.noprefetch",
 		Usage: "Disable heuristic state prefetch during block import (less CPU and disk IO, more time waiting for data)",
 	}
+
 	// Miner settings
+
 	MiningEnabledFlag = cli.BoolFlag{
 		Name:  "mine",
 		Usage: "Enable mining",
@@ -427,7 +436,9 @@ var (
 		Name:  "miner.noverify",
 		Usage: "Disable remote sealing verification",
 	}
+
 	// Account settings
+
 	UnlockedAccountFlag = cli.StringFlag{
 		Name:  "unlock",
 		Usage: "Comma separated list of accounts to unlock",
@@ -455,7 +466,9 @@ var (
 		Name:  "rpc.gascap",
 		Usage: "Sets a cap on gas that can be used in eth_call/estimateGas",
 	}
+
 	// Logging and debug settings
+
 	EthStatsLegacyURLFlag = cli.StringFlag{
 		Name:  "ethstats",
 		Usage: "Reporting URL of a celostats service (nodename:secret@host:port) (deprecated, Use --celostats)",
@@ -473,6 +486,7 @@ var (
 		Usage: "Disables db compaction after import",
 	}
 	// RPC settings
+
 	IPCDisabledFlag = cli.BoolFlag{
 		Name:  "ipcdisable",
 		Usage: "Disable the IPC-RPC server",
@@ -568,6 +582,7 @@ var (
 	}
 
 	// Network Settings
+
 	MaxPeersFlag = cli.IntFlag{
 		Name:  "maxpeers",
 		Usage: "Maximum number of network peers (network disabled if set to 0)",
@@ -668,6 +683,7 @@ var (
 	}
 
 	// Metrics flags
+
 	MetricsEnabledFlag = cli.BoolFlag{
 		Name:  "metrics",
 		Usage: "Enable metrics collection and reporting",
@@ -722,32 +738,34 @@ var (
 	}
 
 	// Istanbul settings
+
 	IstanbulRequestTimeoutFlag = cli.Uint64Flag{
 		Name:  "istanbul.requesttimeout",
-		Usage: "Timeout for each Istanbul round in milliseconds",
-		Value: eth.DefaultConfig.Istanbul.RequestTimeout,
+		Usage: "Timeout for each Istanbul round in milliseconds (deprecated, value obtained from genesis config)",
+		Value: 0,
 	}
 	IstanbulBlockPeriodFlag = cli.Uint64Flag{
 		Name:  "istanbul.blockperiod",
-		Usage: "Default minimum difference between two consecutive block's timestamps in seconds",
-		Value: eth.DefaultConfig.Istanbul.BlockPeriod,
+		Usage: "Default minimum difference between two consecutive block's timestamps in seconds  (deprecated, value obtained from genesis config)",
+		Value: 0,
 	}
 	IstanbulProposerPolicyFlag = cli.Uint64Flag{
 		Name:  "istanbul.proposerpolicy",
-		Usage: "Default minimum difference between two consecutive block's timestamps in seconds",
-		Value: uint64(eth.DefaultConfig.Istanbul.ProposerPolicy),
+		Usage: "Default minimum difference between two consecutive block's timestamps in seconds (deprecated, value obtained from genesis config)",
+		Value: 0,
 	}
 	IstanbulLookbackWindowFlag = cli.Uint64Flag{
 		Name:  "istanbul.lookbackwindow",
-		Usage: "A validator's signature must be absent for this many consecutive blocks to be considered down for the uptime score",
-		Value: eth.DefaultConfig.Istanbul.LookbackWindow,
+		Usage: "A validator's signature must be absent for this many consecutive blocks to be considered down for the uptime score  (deprecated, value obtained from genesis config)",
+		Value: 0,
 	}
 	IstanbulReplicaFlag = cli.BoolFlag{
 		Name:  "istanbul.replica",
 		Usage: "Run this node as a validator replica. Must be paired with --mine. Use the RPCs to enable participation in consensus.",
 	}
 
 	// Announce settings
+
 	AnnounceQueryEnodeGossipPeriodFlag = cli.Uint64Flag{
 		Name:  "announce.queryenodegossipperiod",
 		Usage: "Time duration (in seconds) between gossiped query enode messages",
@@ -759,6 +777,7 @@ var (
 	}
 
 	// Proxy node settings
+
 	ProxyFlag = cli.BoolFlag{
 		Name:  "proxy.proxy",
 		Usage: "Specifies whether this node is a proxy",
@@ -774,6 +793,7 @@ var (
 	}
 
 	// Proxied validator settings
+
 	ProxiedFlag = cli.BoolFlag{
 		Name:  "proxy.proxied",
 		Usage: "Specifies whether this validator will be proxied by a proxy node",
@@ -1404,16 +1424,16 @@ func setWhitelist(ctx *cli.Context, cfg *eth.Config) {
 
 func setIstanbul(ctx *cli.Context, stack *node.Node, cfg *eth.Config) {
 	if ctx.GlobalIsSet(IstanbulRequestTimeoutFlag.Name) {
-		cfg.Istanbul.RequestTimeout = ctx.GlobalUint64(IstanbulRequestTimeoutFlag.Name)
+		log.Warn("Flag value is ignored, and obtained from genesis config", "flag", IstanbulRequestTimeoutFlag.Name)
 	}
 	if ctx.GlobalIsSet(IstanbulBlockPeriodFlag.Name) {
-		cfg.Istanbul.BlockPeriod = ctx.GlobalUint64(IstanbulBlockPeriodFlag.Name)
+		log.Warn("Flag value is ignored, and obtained from genesis config", "flag", IstanbulBlockPeriodFlag.Name)
 	}
 	if ctx.GlobalIsSet(IstanbulLookbackWindowFlag.Name) {
-		cfg.Istanbul.LookbackWindow = ctx.GlobalUint64(IstanbulLookbackWindowFlag.Name)
+		log.Warn("Flag value is ignored, and obtained from genesis config", "flag", IstanbulLookbackWindowFlag.Name)
 	}
 	if ctx.GlobalIsSet(IstanbulProposerPolicyFlag.Name) {
-		cfg.Istanbul.ProposerPolicy = istanbul.ProposerPolicy(ctx.GlobalUint64(IstanbulProposerPolicyFlag.Name))
+		log.Warn("Flag value is ignored, and obtained from genesis config", "flag", IstanbulProposerPolicyFlag.Name)
 	}
 	cfg.Istanbul.ReplicaStateDBPath = stack.ResolvePath(cfg.Istanbul.ReplicaStateDBPath)
 	cfg.Istanbul.ValidatorEnodeDBPath = stack.ResolvePath(cfg.Istanbul.ValidatorEnodeDBPath)

consensus/consensus.go

@@ -208,6 +208,9 @@ type Istanbul interface {
 	// IsLastBlockOfEpoch will check to see if the header is from the last block of an epoch
 	IsLastBlockOfEpoch(header *types.Header) bool
 
+	// LookbackWindow returns the size of the lookback window for calculating uptime (in blocks)
+	LookbackWindow(header *types.Header, state *state.StateDB) uint64
+
 	// ValidatorAddress will return the istanbul engine's validator address
 	ValidatorAddress() common.Address
 

consensus/istanbul/backend/engine.go

@@ -27,7 +27,9 @@ import (
 	"github.com/ethereum/go-ethereum/consensus"
 	"github.com/ethereum/go-ethereum/consensus/istanbul"
 	istanbulCore "github.com/ethereum/go-ethereum/consensus/istanbul/core"
+	"github.com/ethereum/go-ethereum/consensus/istanbul/uptime"
 	"github.com/ethereum/go-ethereum/consensus/istanbul/validator"
+	"github.com/ethereum/go-ethereum/contract_comm/blockchain_parameters"
 	gpm "github.com/ethereum/go-ethereum/contract_comm/gasprice_minimum"
 	ethCore "github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/state"
@@ -416,19 +418,30 @@ func (sb *Backend) UpdateValSetDiff(chain consensus.ChainReader, header *types.H
 	return writeValidatorSetDiff(header, []istanbul.ValidatorData{}, []istanbul.ValidatorData{})
 }
 
-// Returns whether or not a particular header represents the last block in the epoch.
+// IsLastBlockOfEpoch returns whether or not a particular header represents the last block in the epoch.
 func (sb *Backend) IsLastBlockOfEpoch(header *types.Header) bool {
 	return istanbul.IsLastBlockOfEpoch(header.Number.Uint64(), sb.config.Epoch)
 }
 
-// Returns the size of epochs in blocks.
+// EpochSize returns the size of epochs in blocks.
 func (sb *Backend) EpochSize() uint64 {
 	return sb.config.Epoch
 }
 
-// Returns the size of the lookback window for calculating uptime (in blocks)
-func (sb *Backend) LookbackWindow() uint64 {
-	return sb.config.LookbackWindow
+// LookbackWindow returns the size of the lookback window for calculating uptime (in blocks)
+// Value is constant during an epoch
+func (sb *Backend) LookbackWindow(header *types.Header, state *state.StateDB) uint64 {
+	// Check if donut was already active at the beginning of the epoch
+	// as we want to activate the change at epoch change
+	firstBlockOfEpoch := istanbul.MustGetEpochFirstBlockGivenBlockNumber(header.Number.Uint64(), sb.config.Epoch)
+	cip21Activated := sb.chain.Config().IsDonut(new(big.Int).SetUint64(firstBlockOfEpoch))
+
+	return uptime.ComputeLookbackWindow(
+		sb.config.Epoch,
+		sb.config.DefaultLookbackWindow,
+		cip21Activated,
+		func() (uint64, error) { return blockchain_parameters.GetLookbackWindow(header, state) },
+	)
 }
 
 // Finalize runs any post-transaction state modifications (e.g. block rewards)

consensus/istanbul/backend/handler.go

@@ -213,7 +213,7 @@ func (sb *Backend) NewWork() error {
 	return nil
 }
 
-// Maintain metrics around the *parent* of the supplied block.
+// UpdateMetricsForParentOfBlock maintains metrics around the *parent* of the supplied block.
 // To figure out if this validator signed the parent block:
 // * First check the grandparent's validator set. If not elected, it didn't.
 // * Then, check the parent seal on the supplied (child) block.
@@ -303,8 +303,17 @@ func (sb *Backend) UpdateMetricsForParentOfBlock(child *types.Block) {
 		}
 	}
 
-	// Report downtime events.
-	if sb.blocksElectedButNotSignedGauge.Value() >= int64(sb.config.LookbackWindow) {
+	parentState, err := sb.stateAt(childHeader.ParentHash)
+	if err != nil {
+		sb.logger.Error("Error obtaining block state", "block_number", parentHeader.Number, "err", err.Error())
+		return
+	}
+	// The parents lookback window at the time will be used.
+	// However, the value used for updating the validator scores is the one set at the last epoch block.
+	lookbackWindow := sb.LookbackWindow(parentHeader, parentState)
+
+	// Report downtime events
+	if sb.blocksElectedButNotSignedGauge.Value() >= int64(lookbackWindow) {
 		sb.blocksDowntimeEventMeter.Mark(1)
 		sb.logger.Error("Elected but getting marked as down", "missed block count", sb.blocksElectedButNotSignedGauge.Value(), "number", number-1, "address", sb.Address())
 	}

consensus/istanbul/backend/pos.go

@@ -132,10 +132,18 @@ func (sb *Backend) distributeEpochRewards(header *types.Header, state *state.Sta
 
 func (sb *Backend) updateValidatorScores(header *types.Header, state *state.StateDB, valSet []istanbul.Validator) ([]*big.Int, error) {
 	epoch := istanbul.GetEpochNumber(header.Number.Uint64(), sb.EpochSize())
-	logger := sb.logger.New("func", "Backend.updateValidatorScores", "blocknum", header.Number.Uint64(), "epoch", epoch, "epochsize", sb.EpochSize(), "window", sb.LookbackWindow())
+	logger := sb.logger.New("func", "Backend.updateValidatorScores", "blocknum", header.Number.Uint64(), "epoch", epoch, "epochsize", sb.EpochSize())
+
+	// header (&state) == lastBlockOfEpoch
+	// sb.LookbackWindow(header, state) => value at the end of epoch
+	// It doesn't matter which was the value at the beginning but how it ends.
+	// Notice that exposed metrics compute based on current block (not last of epoch) so if lookback window changed during the epoch, metric uptime score might differ
+	lookbackWindow := sb.LookbackWindow(header, state)
+
+	logger = logger.New("window", lookbackWindow)
 	logger.Trace("Updating validator scores")
 
-	monitor := uptime.NewMonitor(store.New(sb.db), sb.EpochSize(), sb.LookbackWindow())
+	monitor := uptime.NewMonitor(store.New(sb.db), sb.EpochSize(), lookbackWindow)
 	uptimes, err := monitor.ComputeValidatorsUptime(epoch, len(valSet))
 	if err != nil {
 		return nil, err

consensus/istanbul/backend/test_utils.go

@@ -47,6 +47,7 @@ func newBlockChainWithKeys(isProxy bool, proxiedValAddress common.Address, isPro
 	config.ProxiedValidatorAddress = proxiedValAddress
 	config.Proxied = isProxied
 	config.Validator = !isProxy
+	istanbul.ApplyParamsChainConfigToConfig(genesis.Config, &config)
 
 	b, _ := New(&config, memDB).(*Backend)
 
@@ -134,7 +135,7 @@ func getGenesisAndKeys(n int, isFullChain bool) (*core.Genesis, []*ecdsa.Private
 	// force enable Istanbul engine
 	genesis.Config.Istanbul = &params.IstanbulConfig{
 		Epoch:          10,
-		LookbackWindow: 2,
+		LookbackWindow: 3,
 	}
 
 	AppendValidatorsToGenesisBlock(genesis, validators)