From 96098ccd399026d38888135e71d78aeef9044bd3 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Fri, 8 Jan 2021 16:43:02 -0300
Subject: [PATCH 01/16] Deprecate unused flags from istanbul

These flags are actually ignored lated in the code; and generate
confussion.

Hence, this commit deprecrated them and clearle states that they are
ignored.
---
 cmd/utils/flags.go | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go
index 2cf221177e..d080f81eb3 100644
--- a/cmd/utils/flags.go
+++ b/cmd/utils/flags.go
@@ -724,22 +724,22 @@ var (
 	// Istanbul settings
 	IstanbulRequestTimeoutFlag = cli.Uint64Flag{
 		Name:  "istanbul.requesttimeout",
-		Usage: "Timeout for each Istanbul round in milliseconds",
+		Usage: "Timeout for each Istanbul round in milliseconds (deprecated, value obtained from genesis config)",
 		Value: eth.DefaultConfig.Istanbul.RequestTimeout,
 	}
 	IstanbulBlockPeriodFlag = cli.Uint64Flag{
 		Name:  "istanbul.blockperiod",
-		Usage: "Default minimum difference between two consecutive block's timestamps in seconds",
+		Usage: "Default minimum difference between two consecutive block's timestamps in seconds  (deprecated, value obtained from genesis config)",
 		Value: eth.DefaultConfig.Istanbul.BlockPeriod,
 	}
 	IstanbulProposerPolicyFlag = cli.Uint64Flag{
 		Name:  "istanbul.proposerpolicy",
-		Usage: "Default minimum difference between two consecutive block's timestamps in seconds",
+		Usage: "Default minimum difference between two consecutive block's timestamps in seconds (deprecated, value obtained from genesis config)",
 		Value: uint64(eth.DefaultConfig.Istanbul.ProposerPolicy),
 	}
 	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",
+		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: eth.DefaultConfig.Istanbul.LookbackWindow,
 	}
 	IstanbulReplicaFlag = cli.BoolFlag{
@@ -1404,16 +1404,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)

From ebb7e73856dd40d01c36b06b85bbdbab3153c31b Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Fri, 8 Jan 2021 18:31:21 -0300
Subject: [PATCH 02/16] Add values blockPeriod & requestTimeout for known
 testnets

---
 params/config.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/params/config.go b/params/config.go
index 6ad0385785..71fe0cf35f 100644
--- a/params/config.go
+++ b/params/config.go
@@ -67,6 +67,8 @@ var (
 		Istanbul: &IstanbulConfig{
 			Epoch:          17280,
 			ProposerPolicy: 2,
+			BlockPeriod:    5,
+			RequestTimeout: 3000,
 			LookbackWindow: 12,
 		},
 	}
@@ -111,6 +113,8 @@ var (
 		Istanbul: &IstanbulConfig{
 			Epoch:          17280,
 			ProposerPolicy: 2,
+			BlockPeriod:    5,
+			RequestTimeout: 10000,
 			LookbackWindow: 12,
 		},
 	}

From e9b8bae1145b1e6b3cad666e50acb817548d2ffd Mon Sep 17 00:00:00 2001
From: andres-dg <adigiovanni1997@gmail.com>
Date: Thu, 13 Aug 2020 18:23:41 -0300
Subject: [PATCH 03/16] Initial implementation of CIP-21

Governable lookback window
---
 .circleci/config.yml                          |  2 +-
 consensus/istanbul/backend/engine.go          | 14 ++++---
 consensus/istanbul/backend/handler.go         | 19 ++++++++--
 consensus/istanbul/backend/pos.go             | 16 +++++++-
 .../blockchain_parameters.go                  | 37 +++++++++++++++++++
 5 files changed, 77 insertions(+), 11 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index e2d4930ef2..be8b5819ff 100644
--- a/.circleci/config.yml
+++ b/.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:
diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go
index 8d3aefa2cf..e289665be5 100644
--- a/consensus/istanbul/backend/engine.go
+++ b/consensus/istanbul/backend/engine.go
@@ -28,6 +28,7 @@ import (
 	"github.com/ethereum/go-ethereum/consensus/istanbul"
 	istanbulCore "github.com/ethereum/go-ethereum/consensus/istanbul/core"
 	"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 +417,22 @@ 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)
+func (sb *Backend) LookbackWindow(header *types.Header, state *state.StateDB) (uint64, error) {
+	if !sb.chain.Config().IsDonut(header.Number) {
+		return sb.config.LookbackWindow, nil
+	}
+	return blockchain_parameters.GetLookbackWindow(header, state)
 }
 
 // Finalize runs any post-transaction state modifications (e.g. block rewards)
diff --git a/consensus/istanbul/backend/handler.go b/consensus/istanbul/backend/handler.go
index 3cc29997a2..5090bab58b 100644
--- a/consensus/istanbul/backend/handler.go
+++ b/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,21 @@ func (sb *Backend) UpdateMetricsForParentOfBlock(child *types.Block) {
 		}
 	}
 
-	// Report downtime events.
-	if sb.blocksElectedButNotSignedGauge.Value() >= int64(sb.config.LookbackWindow) {
+	parentState, err := sb.stateAt(parentHeader.Hash())
+	if err != nil {
+		sb.logger.Error(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, err := sb.LookbackWindow(parentHeader, parentState)
+	if err != nil {
+		sb.logger.Error(err.Error())
+		return
+	}
+
+	// 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())
 	}
diff --git a/consensus/istanbul/backend/pos.go b/consensus/istanbul/backend/pos.go
index dcd99632a9..562133d966 100644
--- a/consensus/istanbul/backend/pos.go
+++ b/consensus/istanbul/backend/pos.go
@@ -132,10 +132,22 @@ 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, err := sb.LookbackWindow(header, state)
+	if err != nil {
+		logger.Error("Error getting lookbackWindow", err.Error())
+		return nil, err
+	}
+
+	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
diff --git a/contract_comm/blockchain_parameters/blockchain_parameters.go b/contract_comm/blockchain_parameters/blockchain_parameters.go
index 18a1942898..ec3539ca83 100644
--- a/contract_comm/blockchain_parameters/blockchain_parameters.go
+++ b/contract_comm/blockchain_parameters/blockchain_parameters.go
@@ -68,6 +68,20 @@ const (
 		"stateMutability": "view",
 		"type": "function"
 	  },
+	{
+		"constant": true,
+		"inputs": [],
+		"name": "uptimeLookbackWindow",
+		"outputs": [
+		  {
+			"name": "",
+			"type": "uint256"
+		  }
+		],
+		"payable": false,
+		"stateMutability": "view",
+		"type": "function"
+	  },
 	  {
 		"constant": true,
 		"inputs": [],
@@ -189,3 +203,26 @@ func GetBlockGasLimit(header *types.Header, state vm.StateDB) (uint64, error) {
 	}
 	return gasLimit.Uint64(), nil
 }
+
+func GetLookbackWindow(header *types.Header, state vm.StateDB) (uint64, error) {
+	var lookbackWindow *big.Int
+	_, err := contract_comm.MakeStaticCall(
+		params.BlockchainParametersRegistryId,
+		blockchainParametersABI,
+		"uptimeLookbackWindow",
+		[]interface{}{},
+		&lookbackWindow,
+		params.MaxGasForReadBlockchainParameter,
+		header,
+		state,
+	)
+	if err != nil {
+		if err == errors.ErrRegistryContractNotDeployed {
+			log.Debug("Error obtaining lookback window", "err", err, "contract", hexutil.Encode(params.BlockchainParametersRegistryId[:]))
+		} else {
+			log.Warn("Error obtaining lookback window", "err", err, "contract", hexutil.Encode(params.BlockchainParametersRegistryId[:]))
+		}
+		return 0, err
+	}
+	return lookbackWindow.Uint64(), nil
+}

From f2fb6e6fb013d801b79522a293528d3749f4b328 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Fri, 8 Jan 2021 17:44:25 -0300
Subject: [PATCH 04/16] Cleaner use of lookbackWindow

* Move logic to uptime model, for cohesiveness
* Move istanbul config processing to istanbul module
* Verify lookbackWindow is within safe boundaries
* Verify EpochSize is at least MinEpochSize

Nits & clean ups
---
 cmd/utils/flags.go                            | 28 ++++++-
 consensus/consensus.go                        |  3 +
 consensus/istanbul/backend/engine.go          |  7 +-
 consensus/istanbul/backend/handler.go         |  6 +-
 consensus/istanbul/config.go                  | 48 ++++++++++--
 consensus/istanbul/uptime/config.go           | 74 +++++++++++++++++++
 consensus/istanbul/uptime/config_test.go      | 43 +++++++++++
 .../istanbul/uptime/{uptime.go => monitor.go} | 25 -------
 .../{uptime_test.go => monitor_test.go}       | 30 --------
 core/blockchain.go                            |  9 ++-
 eth/backend.go                                | 18 +----
 11 files changed, 202 insertions(+), 89 deletions(-)
 create mode 100644 consensus/istanbul/uptime/config.go
 create mode 100644 consensus/istanbul/uptime/config_test.go
 rename consensus/istanbul/uptime/{uptime.go => monitor.go} (85%)
 rename consensus/istanbul/uptime/{uptime_test.go => monitor_test.go} (72%)

diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go
index d080f81eb3..d866166092 100644
--- a/cmd/utils/flags.go
+++ b/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,25 +738,26 @@ var (
 	}
 
 	// Istanbul settings
+
 	IstanbulRequestTimeoutFlag = cli.Uint64Flag{
 		Name:  "istanbul.requesttimeout",
 		Usage: "Timeout for each Istanbul round in milliseconds (deprecated, value obtained from genesis config)",
-		Value: eth.DefaultConfig.Istanbul.RequestTimeout,
+		Value: 0,
 	}
 	IstanbulBlockPeriodFlag = cli.Uint64Flag{
 		Name:  "istanbul.blockperiod",
 		Usage: "Default minimum difference between two consecutive block's timestamps in seconds  (deprecated, value obtained from genesis config)",
-		Value: eth.DefaultConfig.Istanbul.BlockPeriod,
+		Value: 0,
 	}
 	IstanbulProposerPolicyFlag = cli.Uint64Flag{
 		Name:  "istanbul.proposerpolicy",
 		Usage: "Default minimum difference between two consecutive block's timestamps in seconds (deprecated, value obtained from genesis config)",
-		Value: uint64(eth.DefaultConfig.Istanbul.ProposerPolicy),
+		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  (deprecated, value obtained from genesis config)",
-		Value: eth.DefaultConfig.Istanbul.LookbackWindow,
+		Value: 0,
 	}
 	IstanbulReplicaFlag = cli.BoolFlag{
 		Name:  "istanbul.replica",
@@ -748,6 +765,7 @@ var (
 	}
 
 	// 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",
diff --git a/consensus/consensus.go b/consensus/consensus.go
index 41c6e86ae2..543613d7ff 100644
--- a/consensus/consensus.go
+++ b/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, error)
+
 	// ValidatorAddress will return the istanbul engine's validator address
 	ValidatorAddress() common.Address
 
diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go
index e289665be5..35c5363d66 100644
--- a/consensus/istanbul/backend/engine.go
+++ b/consensus/istanbul/backend/engine.go
@@ -27,8 +27,8 @@ 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"
@@ -429,10 +429,7 @@ func (sb *Backend) EpochSize() uint64 {
 
 // LookbackWindow returns the size of the lookback window for calculating uptime (in blocks)
 func (sb *Backend) LookbackWindow(header *types.Header, state *state.StateDB) (uint64, error) {
-	if !sb.chain.Config().IsDonut(header.Number) {
-		return sb.config.LookbackWindow, nil
-	}
-	return blockchain_parameters.GetLookbackWindow(header, state)
+	return uptime.LookbackWindow(sb.chain.Config(), sb.config, header, state)
 }
 
 // Finalize runs any post-transaction state modifications (e.g. block rewards)
diff --git a/consensus/istanbul/backend/handler.go b/consensus/istanbul/backend/handler.go
index 5090bab58b..fb672b4c67 100644
--- a/consensus/istanbul/backend/handler.go
+++ b/consensus/istanbul/backend/handler.go
@@ -303,16 +303,16 @@ func (sb *Backend) UpdateMetricsForParentOfBlock(child *types.Block) {
 		}
 	}
 
-	parentState, err := sb.stateAt(parentHeader.Hash())
+	parentState, err := sb.stateAt(childHeader.ParentHash)
 	if err != nil {
-		sb.logger.Error(err.Error())
+		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, err := sb.LookbackWindow(parentHeader, parentState)
 	if err != nil {
-		sb.logger.Error(err.Error())
+		sb.logger.Error("Error obtaining lookbackWindow", "block_number", parentHeader.Number, "err", err.Error())
 		return
 	}
 
diff --git a/consensus/istanbul/config.go b/consensus/istanbul/config.go
index 5c5874acce..3c4401d0c9 100644
--- a/consensus/istanbul/config.go
+++ b/consensus/istanbul/config.go
@@ -17,10 +17,19 @@
 package istanbul
 
 import (
+	"fmt"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/p2p/enode"
+	"github.com/ethereum/go-ethereum/params"
+)
+
+const (
+	//MinEpochSize represents minimun block that can conform an epoch
+	MinEpochSize = 3
 )
 
+// ProposerPolicy represents the policy used to order elected validators within an epoch
 type ProposerPolicy uint64
 
 const (
@@ -29,6 +38,7 @@ const (
 	ShuffledRoundRobin
 )
 
+// Config represents the istanbul consensus engine
 type Config struct {
 	RequestTimeout              uint64         `toml:",omitempty"` // The timeout for each Istanbul round in milliseconds.
 	TimeoutBackoffFactor        uint64         `toml:",omitempty"` // Timeout at subsequent rounds is: RequestTimeout + 2**round * TimeoutBackoffFactor (in milliseconds)
@@ -37,7 +47,7 @@ type Config struct {
 	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
-	LookbackWindow              uint64         `toml:",omitempty"` // The window of blocks in which a validator is forgived from voting
+	DefaultLookbackWindow       uint64         `toml:",omitempty"` // The default value for the window of blocks in which a validator is forgived from voting
 	ReplicaStateDBPath          string         `toml:",omitempty"` // The location for the validator replica state DB
 	ValidatorEnodeDBPath        string         `toml:",omitempty"` // The location for the validator enodes DB
 	VersionCertificateDBPath    string         `toml:",omitempty"` // The location for the signed announce version DB
@@ -59,6 +69,13 @@ type Config struct {
 	AnnounceAdditionalValidatorsToGossip           int64  `toml:",omitempty"` // Specifies the number of additional non-elected validators to gossip an announce
 }
 
+// ProxyConfig represents the configuration for validator's proxies
+type ProxyConfig struct {
+	InternalNode *enode.Node `toml:",omitempty"` // The internal facing node of the proxy that this proxied validator will peer with
+	ExternalNode *enode.Node `toml:",omitempty"` // The external facing node of the proxy that the proxied validator will broadcast via the announce message
+}
+
+// DefaultConfig for istanbul consensus engine
 var DefaultConfig = &Config{
 	RequestTimeout:                 3000,
 	TimeoutBackoffFactor:           1000,
@@ -67,7 +84,7 @@ var DefaultConfig = &Config{
 	BlockPeriod:                    5,
 	ProposerPolicy:                 ShuffledRoundRobin,
 	Epoch:                          30000,
-	LookbackWindow:                 12,
+	DefaultLookbackWindow:          12,
 	ReplicaStateDBPath:             "replicastate",
 	ValidatorEnodeDBPath:           "validatorenodes",
 	VersionCertificateDBPath:       "versioncertificates",
@@ -81,7 +98,28 @@ var DefaultConfig = &Config{
 	AnnounceAdditionalValidatorsToGossip:           10,
 }
 
-type ProxyConfig struct {
-	InternalNode *enode.Node `toml:",omitempty"` // The internal facing node of the proxy that this proxied validator will peer with
-	ExternalNode *enode.Node `toml:",omitempty"` // The external facing node of the proxy that the proxied validator will broadcast via the announce message
+//ApplyChainConfigToConfig applies chainConfig values to Config
+func ApplyChainConfigToConfig(chainConfig *params.ChainConfig, config *Config) error {
+	if chainConfig.Istanbul.Epoch != 0 {
+		if chainConfig.Istanbul.Epoch < MinEpochSize {
+			return fmt.Errorf("istanbul.Epoch must be greater than %d", MinEpochSize-1)
+		}
+
+		config.Epoch = chainConfig.Istanbul.Epoch
+	}
+	if chainConfig.Istanbul.RequestTimeout != 0 {
+		config.RequestTimeout = chainConfig.Istanbul.RequestTimeout
+	}
+	if chainConfig.Istanbul.BlockPeriod != 0 {
+		config.BlockPeriod = chainConfig.Istanbul.BlockPeriod
+	}
+	if chainConfig.Istanbul.LookbackWindow != 0 {
+		config.DefaultLookbackWindow = chainConfig.Istanbul.LookbackWindow
+	}
+	if chainConfig.Istanbul.LookbackWindow >= chainConfig.Istanbul.Epoch-2 {
+		return fmt.Errorf("istanbul.lookbackwindow must be less than istanbul.epoch-2")
+	}
+	config.ProposerPolicy = ProposerPolicy(chainConfig.Istanbul.ProposerPolicy)
+
+	return nil
 }
diff --git a/consensus/istanbul/uptime/config.go b/consensus/istanbul/uptime/config.go
new file mode 100644
index 0000000000..e378c3e7fa
--- /dev/null
+++ b/consensus/istanbul/uptime/config.go
@@ -0,0 +1,74 @@
+package uptime
+
+import (
+	"github.com/ethereum/go-ethereum/consensus/istanbul"
+	"github.com/ethereum/go-ethereum/contract_comm/blockchain_parameters"
+	"github.com/ethereum/go-ethereum/core/state"
+	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/params"
+)
+
+// Check CIP-21 Spec (https://github.com/celo-org/celo-proposals/blob/master/CIPs/cip-0021.md)
+const (
+	// MinSafeLookbackWindow is the minimum number allowed for lookbackWindow size
+	MinSafeLookbackWindow = 12
+	// MaxSafeLookbackWindow is the maximum number allowed for lookbackWindow size
+	MaxSafeLookbackWindow = 720
+	// BlocksToSkipAtEpochEnd represents the number of blocks to skip on the monitoring window from the end of the epoch
+	// Currently we skip blocks:
+	// lastBlock     => its parentSeal is on firstBlock of next epoch
+	// lastBlock - 1 => parentSeal is on lastBlockOfEpoch, but validatorScore is computed with lastBlockOfEpoch and before updating scores
+	// (lastBlock-1 could be counted, but much harder to implement)
+	BlocksToSkipAtEpochEnd = 2
+)
+
+// LookbackWindow returns the size of the lookback window for calculating uptime (in blocks)
+func LookbackWindow(config *params.ChainConfig, istConfig *istanbul.Config, header *types.Header, state *state.StateDB) (uint64, error) {
+	if !config.IsDonut(header.Number) {
+		return istConfig.DefaultLookbackWindow, nil
+	}
+
+	if istConfig.Epoch <= istanbul.MinEpochSize {
+		panic("Invalid epoch value")
+	}
+
+	value, err := blockchain_parameters.GetLookbackWindow(header, state)
+	if err != nil {
+		value = MinSafeLookbackWindow
+	}
+
+	// Adjust to safe range
+	if value < MinSafeLookbackWindow {
+		value = MinSafeLookbackWindow
+	} else if value > MaxSafeLookbackWindow {
+		value = MaxSafeLookbackWindow
+	}
+
+	// Ensure it's sensible to given chain params
+	if value > (istConfig.Epoch - BlocksToSkipAtEpochEnd) {
+		value = istConfig.Epoch - BlocksToSkipAtEpochEnd
+	}
+
+	return value, nil
+}
+
+// MonitoringWindow retrieves the block window where uptime is to be monitored
+// for a given epoch.
+func MonitoringWindow(epochNumber uint64, epochSize uint64, lookbackWindowSize uint64) Window {
+	if epochNumber == 0 {
+		panic("no monitoring window for epoch 0")
+	}
+
+	epochFirstBlock, _ := istanbul.GetEpochFirstBlockNumber(epochNumber, epochSize)
+	epochLastBlock := istanbul.GetEpochLastBlockNumber(epochNumber, epochSize)
+
+	// first block to monitor:
+	// we can't monitor uptime when current lookbackWindow crosses the epoch boundary
+	// thus, first block to monitor is the final block of the lookbackwindow that starts at firstBlockOfEpoch
+	firstBlockToMonitor := newWindowStartingAt(epochFirstBlock, lookbackWindowSize).End
+
+	return Window{
+		Start: firstBlockToMonitor,
+		End:   epochLastBlock - BlocksToSkipAtEpochEnd,
+	}
+}
diff --git a/consensus/istanbul/uptime/config_test.go b/consensus/istanbul/uptime/config_test.go
new file mode 100644
index 0000000000..4c75e27cb2
--- /dev/null
+++ b/consensus/istanbul/uptime/config_test.go
@@ -0,0 +1,43 @@
+package uptime
+
+import (
+	"testing"
+
+	"github.com/ethereum/go-ethereum/consensus/istanbul"
+)
+
+func TestEpochSizeIsConsistentWithSkippedBlock(t *testing.T) {
+	if istanbul.MinEpochSize <= BlocksToSkipAtEpochEnd {
+		t.Fatalf("Constant MinEpochSize MUST BE greater than BlocksToSkipAtEpochEnd (%d, %d) ", istanbul.MinEpochSize, BlocksToSkipAtEpochEnd)
+	}
+}
+
+func TestMonitoringWindow(t *testing.T) {
+	type args struct {
+		epochNumber        uint64
+		epochSize          uint64
+		lookbackWindowSize uint64
+	}
+	tests := []struct {
+		name      string
+		args      args
+		wantStart uint64
+		wantEnd   uint64
+	}{
+		{"monitoringWindow on first epoch", args{1, 10, 2}, 2, 8},
+		{"monitoringWindow on second epoch", args{2, 10, 2}, 12, 18},
+		{"lookback window too big", args{1, 10, 10}, 10, 8},
+		// TODO: Add test cases.
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			w := MonitoringWindow(tt.args.epochNumber, tt.args.epochSize, tt.args.lookbackWindowSize)
+			if w.Start != tt.wantStart {
+				t.Errorf("MonitoringWindow() got = %v, wantStart %v", w.Start, tt.wantStart)
+			}
+			if w.End != tt.wantEnd {
+				t.Errorf("MonitoringWindow() got1 = %v, wantEnd %v", w.End, tt.wantEnd)
+			}
+		})
+	}
+}
diff --git a/consensus/istanbul/uptime/uptime.go b/consensus/istanbul/uptime/monitor.go
similarity index 85%
rename from consensus/istanbul/uptime/uptime.go
rename to consensus/istanbul/uptime/monitor.go
index abf3d35777..a83f659d2b 100644
--- a/consensus/istanbul/uptime/uptime.go
+++ b/consensus/istanbul/uptime/monitor.go
@@ -39,31 +39,6 @@ func (u *UptimeEntry) String() string {
 	return fmt.Sprintf("UptimeEntry { upBlocks: %v, lastBlock: %v}", u.UpBlocks, u.LastSignedBlock)
 }
 
-// MonitoringWindow retrieves the block window where uptime is to be monitored
-// for a given epoch.
-func MonitoringWindow(epochNumber uint64, epochSize uint64, lookbackWindowSize uint64) Window {
-	if epochNumber == 0 {
-		panic("no monitoring window for epoch 0")
-	}
-
-	epochFirstBlock, _ := istanbul.GetEpochFirstBlockNumber(epochNumber, epochSize)
-	epochLastBlock := istanbul.GetEpochLastBlockNumber(epochNumber, epochSize)
-
-	// first block to monitor:
-	// we can't monitor uptime when current lookbackWindow crosses the epoch boundary
-	// thus, first block to monitor is the final block of the lookbackwindow that starts at firstBlockOfEpoch
-	firstBlockToMonitor := newWindowStartingAt(epochFirstBlock, lookbackWindowSize).End
-
-	// last block to monitor:
-	// Last 2 blocks from the epoch are removed from the window
-	// lastBlock     => its parentSeal is on firstBlock of next epoch
-	// lastBlock - 1 => parentSeal is on lastBlockOfEpoch, but validatorScore is computed with lastBlockOfEpoch and before updating scores
-	// (lastBlock-1 could be counted, but much harder to implement)
-	lastBlockToMonitor := epochLastBlock - 2
-
-	return Window{Start: firstBlockToMonitor, End: lastBlockToMonitor}
-}
-
 // Monitor is responsible for monitoring uptime by processing blocks
 type Monitor struct {
 	epochSize      uint64
diff --git a/consensus/istanbul/uptime/uptime_test.go b/consensus/istanbul/uptime/monitor_test.go
similarity index 72%
rename from consensus/istanbul/uptime/uptime_test.go
rename to consensus/istanbul/uptime/monitor_test.go
index 5c1fcad22d..59d10f91dd 100644
--- a/consensus/istanbul/uptime/uptime_test.go
+++ b/consensus/istanbul/uptime/monitor_test.go
@@ -6,36 +6,6 @@ import (
 	"testing"
 )
 
-func TestGetUptimeMonitoringWindow(t *testing.T) {
-	type args struct {
-		epochNumber        uint64
-		epochSize          uint64
-		lookbackWindowSize uint64
-	}
-	tests := []struct {
-		name      string
-		args      args
-		wantStart uint64
-		wantEnd   uint64
-	}{
-		{"monitoringWindow on first epoch", args{1, 10, 2}, 2, 8},
-		{"monitoringWindow on second epoch", args{2, 10, 2}, 12, 18},
-		{"lookback window too big", args{1, 10, 10}, 10, 8},
-		// TODO: Add test cases.
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			w := MonitoringWindow(tt.args.epochNumber, tt.args.epochSize, tt.args.lookbackWindowSize)
-			if w.Start != tt.wantStart {
-				t.Errorf("MonitoringWindow() got = %v, wantStart %v", w.Start, tt.wantStart)
-			}
-			if w.End != tt.wantEnd {
-				t.Errorf("MonitoringWindow() got1 = %v, wantEnd %v", w.End, tt.wantEnd)
-			}
-		})
-	}
-}
-
 func TestUptime(t *testing.T) {
 	var uptimes *Uptime
 	// (there can't be less than 2/3rds of validators sigs in a valid bitmap)
diff --git a/core/blockchain.go b/core/blockchain.go
index 3185965862..5030cccd28 100644
--- a/core/blockchain.go
+++ b/core/blockchain.go
@@ -1263,8 +1263,13 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types.
 			log.Error("Found two blocks with same height", "old", hash, "new", block.Hash())
 		}
 
-		// We are going to update the uptime tally.
-		uptimeMonitor := uptime.NewMonitor(store.New(bc.db), bc.chainConfig.Istanbul.Epoch, bc.chainConfig.Istanbul.LookbackWindow)
+		lookbackWindow, err := istEngine.LookbackWindow(block.Header(), state)
+		if err != nil {
+			log.Error("Error obtaining lookbackWindow", "block_number", block.Number, "err", err)
+			return NonStatTy, err
+		}
+
+		uptimeMonitor := uptime.NewMonitor(store.New(bc.db), bc.chainConfig.Istanbul.Epoch, lookbackWindow)
 		err = uptimeMonitor.ProcessBlock(block)
 		if err != nil {
 			return NonStatTy, err
diff --git a/eth/backend.go b/eth/backend.go
index a5b1c48d36..4e17f3a5e6 100644
--- a/eth/backend.go
+++ b/eth/backend.go
@@ -276,22 +276,10 @@ func CreateConsensusEngine(ctx *node.ServiceContext, chainConfig *params.ChainCo
 	// If Istanbul is requested, set it up
 	if chainConfig.Istanbul != nil {
 		log.Debug("Setting up Istanbul consensus engine")
-		if chainConfig.Istanbul.Epoch != 0 {
-			config.Istanbul.Epoch = chainConfig.Istanbul.Epoch
+		if err := istanbul.ApplyChainConfigToConfig(chainConfig, &config.Istanbul); err != nil {
+			log.Crit("Invalid Configuration for Istanbul Engine", "err", err)
 		}
-		if chainConfig.Istanbul.RequestTimeout != 0 {
-			config.Istanbul.RequestTimeout = chainConfig.Istanbul.RequestTimeout
-		}
-		if chainConfig.Istanbul.BlockPeriod != 0 {
-			config.Istanbul.BlockPeriod = chainConfig.Istanbul.BlockPeriod
-		}
-		if chainConfig.Istanbul.LookbackWindow != 0 {
-			config.Istanbul.LookbackWindow = chainConfig.Istanbul.LookbackWindow
-		}
-		if chainConfig.Istanbul.LookbackWindow >= chainConfig.Istanbul.Epoch-1 {
-			log.Crit("istanbul.lookbackwindow must be less than istanbul.epoch-1")
-		}
-		config.Istanbul.ProposerPolicy = istanbul.ProposerPolicy(chainConfig.Istanbul.ProposerPolicy)
+
 		return istanbulBackend.New(&config.Istanbul, db)
 	}
 	log.Error(fmt.Sprintf("Only Istanbul Consensus is supported: %v", chainConfig))

From d1b3e5b1eac7df969e7bf5c73e73c0cbdea1eb02 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Tue, 12 Jan 2021 14:21:19 -0300
Subject: [PATCH 05/16] Add comment

---
 consensus/istanbul/backend/engine.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go
index 35c5363d66..26dee55bcc 100644
--- a/consensus/istanbul/backend/engine.go
+++ b/consensus/istanbul/backend/engine.go
@@ -428,6 +428,7 @@ func (sb *Backend) EpochSize() uint64 {
 }
 
 // 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, error) {
 	return uptime.LookbackWindow(sb.chain.Config(), sb.config, header, state)
 }

From df9ccb327de502162455e7734cc69357f8c8cced Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Tue, 12 Jan 2021 16:49:51 -0300
Subject: [PATCH 06/16] Change function used to obtain lookbackWindow

---
 .../blockchain_parameters.go                  | 45 ++++++++++---------
 params/protocol_params.go                     |  2 +-
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/contract_comm/blockchain_parameters/blockchain_parameters.go b/contract_comm/blockchain_parameters/blockchain_parameters.go
index ec3539ca83..5a9bb32e07 100644
--- a/contract_comm/blockchain_parameters/blockchain_parameters.go
+++ b/contract_comm/blockchain_parameters/blockchain_parameters.go
@@ -67,35 +67,36 @@ const (
 		"payable": false,
 		"stateMutability": "view",
 		"type": "function"
-	  },
+	},
 	{
 		"constant": true,
 		"inputs": [],
-		"name": "uptimeLookbackWindow",
+		"name": "getUptimeLookbackWindow",
 		"outputs": [
-		  {
-			"name": "",
-			"type": "uint256"
-		  }
-		],
-		"payable": false,
-		"stateMutability": "view",
-		"type": "function"
-	  },
-	  {
-		"constant": true,
-		"inputs": [],
-		"name": "intrinsicGasForAlternativeFeeCurrency",
-		"outputs": [
-		  {
-			"name": "",
-			"type": "uint256"
-		  }
+			{
+				"internalType": "uint256",
+				"name": "lookbackWindow",
+				"type": "uint256"
+			}
 		],
 		"payable": false,
 		"stateMutability": "view",
 		"type": "function"
-	  }
+	},
+	{
+	"constant": true,
+	"inputs": [],
+	"name": "intrinsicGasForAlternativeFeeCurrency",
+	"outputs": [
+		{
+		"name": "",
+		"type": "uint256"
+		}
+	],
+	"payable": false,
+	"stateMutability": "view",
+	"type": "function"
+	}
 ]`
 )
 
@@ -209,7 +210,7 @@ func GetLookbackWindow(header *types.Header, state vm.StateDB) (uint64, error) {
 	_, err := contract_comm.MakeStaticCall(
 		params.BlockchainParametersRegistryId,
 		blockchainParametersABI,
-		"uptimeLookbackWindow",
+		"getUptimeLookbackWindow",
 		[]interface{}{},
 		&lookbackWindow,
 		params.MaxGasForReadBlockchainParameter,
diff --git a/params/protocol_params.go b/params/protocol_params.go
index f0fd816005..4ee9f7d273 100644
--- a/params/protocol_params.go
+++ b/params/protocol_params.go
@@ -220,7 +220,7 @@ const (
 	MaxGasForIncreaseSupply                        uint64 = 50 * thousand
 	MaxGasForIsFrozen                              uint64 = 20 * thousand
 	MaxGasForMedianRate                            uint64 = 100 * thousand
-	MaxGasForReadBlockchainParameter               uint64 = 20 * thousand
+	MaxGasForReadBlockchainParameter               uint64 = 40 * thousand // ad-hoc measurement is ~26k
 	MaxGasForRevealAndCommit                       uint64 = 2 * million
 	MaxGasForUpdateGasPriceMinimum                 uint64 = 2 * million
 	MaxGasForUpdateTargetVotingYield               uint64 = 2 * million

From 82761b2937acbb41d04b88102e30e0554f62c722 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Tue, 12 Jan 2021 18:58:37 -0300
Subject: [PATCH 07/16] Fix cyclic dependency. Remove error return from
 lookbackWindow function

# Conflicts:
#	consensus/istanbul/backend/engine.go
---
 consensus/consensus.go                   |  2 +-
 consensus/istanbul/backend/engine.go     | 10 ++++++--
 consensus/istanbul/backend/handler.go    |  6 +----
 consensus/istanbul/backend/pos.go        |  6 +----
 consensus/istanbul/uptime/config.go      | 23 ++++++++----------
 consensus/istanbul/uptime/config_test.go | 31 ++++++++++++++++++++++++
 core/blockchain.go                       |  6 +----
 7 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/consensus/consensus.go b/consensus/consensus.go
index 543613d7ff..3a5cf888a4 100644
--- a/consensus/consensus.go
+++ b/consensus/consensus.go
@@ -209,7 +209,7 @@ type Istanbul interface {
 	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, error)
+	LookbackWindow(header *types.Header, state *state.StateDB) uint64
 
 	// ValidatorAddress will return the istanbul engine's validator address
 	ValidatorAddress() common.Address
diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go
index 26dee55bcc..dd27f29ea0 100644
--- a/consensus/istanbul/backend/engine.go
+++ b/consensus/istanbul/backend/engine.go
@@ -29,6 +29,7 @@ import (
 	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"
@@ -429,8 +430,13 @@ func (sb *Backend) EpochSize() uint64 {
 
 // 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, error) {
-	return uptime.LookbackWindow(sb.chain.Config(), sb.config, header, state)
+func (sb *Backend) LookbackWindow(header *types.Header, state *state.StateDB) uint64 {
+	return uptime.ComputeLookbackWindow(
+		sb.config.Epoch,
+		sb.config.DefaultLookbackWindow,
+		sb.chain.Config().IsDonut(header.Number),
+		func() (uint64, error) { return blockchain_parameters.GetLookbackWindow(header, state) },
+	)
 }
 
 // Finalize runs any post-transaction state modifications (e.g. block rewards)
diff --git a/consensus/istanbul/backend/handler.go b/consensus/istanbul/backend/handler.go
index fb672b4c67..6e9fe93118 100644
--- a/consensus/istanbul/backend/handler.go
+++ b/consensus/istanbul/backend/handler.go
@@ -310,11 +310,7 @@ func (sb *Backend) UpdateMetricsForParentOfBlock(child *types.Block) {
 	}
 	// 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, err := sb.LookbackWindow(parentHeader, parentState)
-	if err != nil {
-		sb.logger.Error("Error obtaining lookbackWindow", "block_number", parentHeader.Number, "err", err.Error())
-		return
-	}
+	lookbackWindow := sb.LookbackWindow(parentHeader, parentState)
 
 	// Report downtime events
 	if sb.blocksElectedButNotSignedGauge.Value() >= int64(lookbackWindow) {
diff --git a/consensus/istanbul/backend/pos.go b/consensus/istanbul/backend/pos.go
index 562133d966..cf0e8d7317 100644
--- a/consensus/istanbul/backend/pos.go
+++ b/consensus/istanbul/backend/pos.go
@@ -138,11 +138,7 @@ func (sb *Backend) updateValidatorScores(header *types.Header, state *state.Stat
 	// 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, err := sb.LookbackWindow(header, state)
-	if err != nil {
-		logger.Error("Error getting lookbackWindow", err.Error())
-		return nil, err
-	}
+	lookbackWindow := sb.LookbackWindow(header, state)
 
 	logger = logger.New("window", lookbackWindow)
 	logger.Trace("Updating validator scores")
diff --git a/consensus/istanbul/uptime/config.go b/consensus/istanbul/uptime/config.go
index e378c3e7fa..df2bbc7a40 100644
--- a/consensus/istanbul/uptime/config.go
+++ b/consensus/istanbul/uptime/config.go
@@ -2,10 +2,6 @@ package uptime
 
 import (
 	"github.com/ethereum/go-ethereum/consensus/istanbul"
-	"github.com/ethereum/go-ethereum/contract_comm/blockchain_parameters"
-	"github.com/ethereum/go-ethereum/core/state"
-	"github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/params"
 )
 
 // Check CIP-21 Spec (https://github.com/celo-org/celo-proposals/blob/master/CIPs/cip-0021.md)
@@ -22,17 +18,18 @@ const (
 	BlocksToSkipAtEpochEnd = 2
 )
 
-// LookbackWindow returns the size of the lookback window for calculating uptime (in blocks)
-func LookbackWindow(config *params.ChainConfig, istConfig *istanbul.Config, header *types.Header, state *state.StateDB) (uint64, error) {
-	if !config.IsDonut(header.Number) {
-		return istConfig.DefaultLookbackWindow, nil
+// ComputeLookbackWindow computes the lookbackWindow based on different required parameters.
+// getLookbackWindow represents the way to obtain lookbackWindow from the smart contract
+func ComputeLookbackWindow(epochSize uint64, defaultLookbackWindow uint64, isDonut bool, getLookbackWindow func() (uint64, error)) uint64 {
+	if !isDonut {
+		return defaultLookbackWindow
 	}
 
-	if istConfig.Epoch <= istanbul.MinEpochSize {
+	if epochSize <= istanbul.MinEpochSize {
 		panic("Invalid epoch value")
 	}
 
-	value, err := blockchain_parameters.GetLookbackWindow(header, state)
+	value, err := getLookbackWindow()
 	if err != nil {
 		value = MinSafeLookbackWindow
 	}
@@ -45,11 +42,11 @@ func LookbackWindow(config *params.ChainConfig, istConfig *istanbul.Config, head
 	}
 
 	// Ensure it's sensible to given chain params
-	if value > (istConfig.Epoch - BlocksToSkipAtEpochEnd) {
-		value = istConfig.Epoch - BlocksToSkipAtEpochEnd
+	if value > (epochSize - BlocksToSkipAtEpochEnd) {
+		value = epochSize - BlocksToSkipAtEpochEnd
 	}
 
-	return value, nil
+	return value
 }
 
 // MonitoringWindow retrieves the block window where uptime is to be monitored
diff --git a/consensus/istanbul/uptime/config_test.go b/consensus/istanbul/uptime/config_test.go
index 4c75e27cb2..762e0260e7 100644
--- a/consensus/istanbul/uptime/config_test.go
+++ b/consensus/istanbul/uptime/config_test.go
@@ -41,3 +41,34 @@ func TestMonitoringWindow(t *testing.T) {
 		})
 	}
 }
+
+func TestComputeLookbackWindow(t *testing.T) {
+	constant := func(value uint64) func() (uint64, error) {
+		return func() (uint64, error) { return value, nil }
+	}
+
+	type args struct {
+		epochSize             uint64
+		defaultLookbackWindow uint64
+		isDonut               bool
+		getLookbackWindow     func() (uint64, error)
+	}
+	tests := []struct {
+		name string
+		args args
+		want uint64
+	}{
+		{"returns default if Donut is not active", args{100, 20, false, constant(24)}, 20},
+		{"returns safe minimun if configured is below", args{100, 20, true, constant(10)}, 12},
+		{"returns safe maximum if configured is above", args{1000, 20, true, constant(800)}, 720},
+		{"returns epochSize-2 if configured is above", args{100, 20, true, constant(99)}, 98},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ComputeLookbackWindow(tt.args.epochSize, tt.args.defaultLookbackWindow, tt.args.isDonut, tt.args.getLookbackWindow)
+			if got != tt.want {
+				t.Errorf("ComputeLookbackWindow() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/core/blockchain.go b/core/blockchain.go
index 5030cccd28..9589df5873 100644
--- a/core/blockchain.go
+++ b/core/blockchain.go
@@ -1263,11 +1263,7 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types.
 			log.Error("Found two blocks with same height", "old", hash, "new", block.Hash())
 		}
 
-		lookbackWindow, err := istEngine.LookbackWindow(block.Header(), state)
-		if err != nil {
-			log.Error("Error obtaining lookbackWindow", "block_number", block.Number, "err", err)
-			return NonStatTy, err
-		}
+		lookbackWindow := istEngine.LookbackWindow(block.Header(), state)
 
 		uptimeMonitor := uptime.NewMonitor(store.New(bc.db), bc.chainConfig.Istanbul.Epoch, lookbackWindow)
 		err = uptimeMonitor.ProcessBlock(block)

From 26b754759a2a2a5140ee186252c113c20da9990b Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Wed, 13 Jan 2021 16:10:43 -0300
Subject: [PATCH 08/16] change minLookbackWindow safe value

---
 consensus/istanbul/uptime/config.go      | 2 +-
 consensus/istanbul/uptime/config_test.go | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/consensus/istanbul/uptime/config.go b/consensus/istanbul/uptime/config.go
index df2bbc7a40..3c29185858 100644
--- a/consensus/istanbul/uptime/config.go
+++ b/consensus/istanbul/uptime/config.go
@@ -7,7 +7,7 @@ import (
 // Check CIP-21 Spec (https://github.com/celo-org/celo-proposals/blob/master/CIPs/cip-0021.md)
 const (
 	// MinSafeLookbackWindow is the minimum number allowed for lookbackWindow size
-	MinSafeLookbackWindow = 12
+	MinSafeLookbackWindow = 3
 	// MaxSafeLookbackWindow is the maximum number allowed for lookbackWindow size
 	MaxSafeLookbackWindow = 720
 	// BlocksToSkipAtEpochEnd represents the number of blocks to skip on the monitoring window from the end of the epoch
diff --git a/consensus/istanbul/uptime/config_test.go b/consensus/istanbul/uptime/config_test.go
index 762e0260e7..9ed54d2a2b 100644
--- a/consensus/istanbul/uptime/config_test.go
+++ b/consensus/istanbul/uptime/config_test.go
@@ -59,7 +59,7 @@ func TestComputeLookbackWindow(t *testing.T) {
 		want uint64
 	}{
 		{"returns default if Donut is not active", args{100, 20, false, constant(24)}, 20},
-		{"returns safe minimun if configured is below", args{100, 20, true, constant(10)}, 12},
+		{"returns safe minimun if configured is below", args{100, 20, true, constant(2)}, 12},
 		{"returns safe maximum if configured is above", args{1000, 20, true, constant(800)}, 720},
 		{"returns epochSize-2 if configured is above", args{100, 20, true, constant(99)}, 98},
 	}

From 66ea7cbe2245ac23cddeba8d815947061b7c794c Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Thu, 14 Jan 2021 11:49:21 -0300
Subject: [PATCH 09/16] fix

---
 consensus/istanbul/uptime/config_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/consensus/istanbul/uptime/config_test.go b/consensus/istanbul/uptime/config_test.go
index 9ed54d2a2b..fec0afc867 100644
--- a/consensus/istanbul/uptime/config_test.go
+++ b/consensus/istanbul/uptime/config_test.go
@@ -59,7 +59,7 @@ func TestComputeLookbackWindow(t *testing.T) {
 		want uint64
 	}{
 		{"returns default if Donut is not active", args{100, 20, false, constant(24)}, 20},
-		{"returns safe minimun if configured is below", args{100, 20, true, constant(2)}, 12},
+		{"returns safe minimun if configured is below", args{100, 20, true, constant(2)}, 3},
 		{"returns safe maximum if configured is above", args{1000, 20, true, constant(800)}, 720},
 		{"returns epochSize-2 if configured is above", args{100, 20, true, constant(99)}, 98},
 	}

From e45670f196b76d9e89e9485c69aa8b5dc3ea2cb3 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Fri, 15 Jan 2021 17:57:06 -0300
Subject: [PATCH 10/16] Address PR comments

---
 consensus/istanbul/backend/engine.go          |  7 ++-
 consensus/istanbul/config.go                  |  8 ++--
 consensus/istanbul/uptime/config.go           | 24 +++++++++--
 consensus/istanbul/uptime/config_test.go      | 43 ++++++++++++-------
 consensus/istanbul/uptime/monitor.go          |  2 +-
 consensus/istanbul/uptime/monitor_test.go     |  4 +-
 consensus/istanbul/utils.go                   | 17 ++++++++
 .../blockchain_parameters.go                  | 24 +++++------
 eth/backend.go                                |  2 +-
 params/config.go                              |  2 +-
 10 files changed, 92 insertions(+), 41 deletions(-)

diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go
index dd27f29ea0..a03950c889 100644
--- a/consensus/istanbul/backend/engine.go
+++ b/consensus/istanbul/backend/engine.go
@@ -431,10 +431,15 @@ func (sb *Backend) EpochSize() uint64 {
 // 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)
+	isDonutActivated := sb.chain.Config().IsDonut(new(big.Int).SetUint64(firstBlockOfEpoch))
+
 	return uptime.ComputeLookbackWindow(
 		sb.config.Epoch,
 		sb.config.DefaultLookbackWindow,
-		sb.chain.Config().IsDonut(header.Number),
+		isDonutActivated,
 		func() (uint64, error) { return blockchain_parameters.GetLookbackWindow(header, state) },
 	)
 }
diff --git a/consensus/istanbul/config.go b/consensus/istanbul/config.go
index 3c4401d0c9..d81a1f01c5 100644
--- a/consensus/istanbul/config.go
+++ b/consensus/istanbul/config.go
@@ -25,7 +25,7 @@ import (
 )
 
 const (
-	//MinEpochSize represents minimun block that can conform an epoch
+	//MinEpochSize represents the minimum permissible epoch size
 	MinEpochSize = 3
 )
 
@@ -47,7 +47,7 @@ type Config struct {
 	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
-	DefaultLookbackWindow       uint64         `toml:",omitempty"` // The default value for the window of blocks in which a validator is forgived from voting
+	DefaultLookbackWindow       uint64         `toml:",omitempty"` // The default value for how many blocks in a row a validator must miss to be considered "down"
 	ReplicaStateDBPath          string         `toml:",omitempty"` // The location for the validator replica state DB
 	ValidatorEnodeDBPath        string         `toml:",omitempty"` // The location for the validator enodes DB
 	VersionCertificateDBPath    string         `toml:",omitempty"` // The location for the signed announce version DB
@@ -98,8 +98,8 @@ var DefaultConfig = &Config{
 	AnnounceAdditionalValidatorsToGossip:           10,
 }
 
-//ApplyChainConfigToConfig applies chainConfig values to Config
-func ApplyChainConfigToConfig(chainConfig *params.ChainConfig, config *Config) error {
+//ApplyParamsChainConfigToConfig applies the istanbul config values from params.chainConfig to the istanbul.Config config
+func ApplyParamsChainConfigToConfig(chainConfig *params.ChainConfig, config *Config) error {
 	if chainConfig.Istanbul.Epoch != 0 {
 		if chainConfig.Istanbul.Epoch < MinEpochSize {
 			return fmt.Errorf("istanbul.Epoch must be greater than %d", MinEpochSize-1)
diff --git a/consensus/istanbul/uptime/config.go b/consensus/istanbul/uptime/config.go
index 3c29185858..cefae82657 100644
--- a/consensus/istanbul/uptime/config.go
+++ b/consensus/istanbul/uptime/config.go
@@ -1,6 +1,9 @@
 package uptime
 
 import (
+	"errors"
+	"fmt"
+
 	"github.com/ethereum/go-ethereum/consensus/istanbul"
 )
 
@@ -49,11 +52,26 @@ func ComputeLookbackWindow(epochSize uint64, defaultLookbackWindow uint64, isDon
 	return value
 }
 
+// MustMonitoringWindow is a MonitoringWindow variant that panics on error
+func MustMonitoringWindow(epochNumber uint64, epochSize uint64, lookbackWindowSize uint64) Window {
+	w, err := MonitoringWindow(epochNumber, epochSize, lookbackWindowSize)
+	if err != nil {
+		panic(err)
+	}
+	return w
+}
+
 // MonitoringWindow retrieves the block window where uptime is to be monitored
 // for a given epoch.
-func MonitoringWindow(epochNumber uint64, epochSize uint64, lookbackWindowSize uint64) Window {
+func MonitoringWindow(epochNumber uint64, epochSize uint64, lookbackWindowSize uint64) (Window, error) {
 	if epochNumber == 0 {
-		panic("no monitoring window for epoch 0")
+		return Window{}, errors.New("no monitoring window for epoch 0")
+	}
+	if epochSize < istanbul.MinEpochSize {
+		return Window{}, errors.New("Invalid epoch value")
+	}
+	if epochSize < lookbackWindowSize+BlocksToSkipAtEpochEnd {
+		return Window{}, fmt.Errorf("LookbackWindow (%d) too big for epochSize (%d)", lookbackWindowSize, epochSize)
 	}
 
 	epochFirstBlock, _ := istanbul.GetEpochFirstBlockNumber(epochNumber, epochSize)
@@ -67,5 +85,5 @@ func MonitoringWindow(epochNumber uint64, epochSize uint64, lookbackWindowSize u
 	return Window{
 		Start: firstBlockToMonitor,
 		End:   epochLastBlock - BlocksToSkipAtEpochEnd,
-	}
+	}, nil
 }
diff --git a/consensus/istanbul/uptime/config_test.go b/consensus/istanbul/uptime/config_test.go
index fec0afc867..baaac49218 100644
--- a/consensus/istanbul/uptime/config_test.go
+++ b/consensus/istanbul/uptime/config_test.go
@@ -19,24 +19,33 @@ func TestMonitoringWindow(t *testing.T) {
 		lookbackWindowSize uint64
 	}
 	tests := []struct {
-		name      string
-		args      args
-		wantStart uint64
-		wantEnd   uint64
+		name  string
+		args  args
+		want  Window
+		fails bool
 	}{
-		{"monitoringWindow on first epoch", args{1, 10, 2}, 2, 8},
-		{"monitoringWindow on second epoch", args{2, 10, 2}, 12, 18},
-		{"lookback window too big", args{1, 10, 10}, 10, 8},
-		// TODO: Add test cases.
+		{"monitoringWindow on first epoch", args{1, 10, 2}, Window{2, 8}, false},
+		{"monitoringWindow on second epoch", args{2, 10, 2}, Window{12, 18}, false},
+		{"largest possible lookback window", args{1, 10, 8}, Window{8, 8}, false},
+		{"fails when epochSize is too small", args{1, istanbul.MinEpochSize - 1, 1}, Window{0, 0}, true},
+		{"fails for epoch 0 (genesis)", args{0, 10, 2}, Window{2, 8}, true},
+		{"fails when lookbackWindow too big for current epoch size", args{1, 10, 9}, Window{2, 8}, true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			w := MonitoringWindow(tt.args.epochNumber, tt.args.epochSize, tt.args.lookbackWindowSize)
-			if w.Start != tt.wantStart {
-				t.Errorf("MonitoringWindow() got = %v, wantStart %v", w.Start, tt.wantStart)
-			}
-			if w.End != tt.wantEnd {
-				t.Errorf("MonitoringWindow() got1 = %v, wantEnd %v", w.End, tt.wantEnd)
+			got, err := MonitoringWindow(tt.args.epochNumber, tt.args.epochSize, tt.args.lookbackWindowSize)
+
+			if err == nil && tt.fails {
+				t.Errorf("MonitoringWindow() got %v, expected to fail", got)
+			} else if err != nil && !tt.fails {
+				t.Errorf("MonitoringWindow() panicked, wanted: %v", tt.want)
+			} else if err == nil {
+				if got.Start != tt.want.Start {
+					t.Errorf("MonitoringWindow().Start got = %v, want = %v", got.Start, tt.want.Start)
+				}
+				if got.End != tt.want.End {
+					t.Errorf("MonitoringWindow().End got = %v, want = %v", got.End, tt.want.End)
+				}
 			}
 		})
 	}
@@ -59,9 +68,11 @@ func TestComputeLookbackWindow(t *testing.T) {
 		want uint64
 	}{
 		{"returns default if Donut is not active", args{100, 20, false, constant(24)}, 20},
-		{"returns safe minimun if configured is below", args{100, 20, true, constant(2)}, 3},
-		{"returns safe maximum if configured is above", args{1000, 20, true, constant(800)}, 720},
+		{"returns safe minimum if configured is below", args{100, 20, true, constant(2)}, MinSafeLookbackWindow},
+		{"returns safe maximum if configured is above", args{1000, 20, true, constant(800)}, MaxSafeLookbackWindow},
 		{"returns epochSize-2 if configured is above", args{100, 20, true, constant(99)}, 98},
+		{"mainet config before donut activation", args{17280, 12, false, constant(64)}, 12},
+		{"mainet config after donut activation", args{17280, 12, true, constant(64)}, 64},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/consensus/istanbul/uptime/monitor.go b/consensus/istanbul/uptime/monitor.go
index a83f659d2b..bdb5373403 100644
--- a/consensus/istanbul/uptime/monitor.go
+++ b/consensus/istanbul/uptime/monitor.go
@@ -61,7 +61,7 @@ func NewMonitor(store Store, epochSize, lookbackWindow uint64) *Monitor {
 // MonitoringWindow returns the monitoring window for the given epoch in the format
 // [firstBlock, lastBlock] both inclusive
 func (um *Monitor) MonitoringWindow(epoch uint64) Window {
-	return MonitoringWindow(epoch, um.epochSize, um.lookbackWindow)
+	return MustMonitoringWindow(epoch, um.epochSize, um.lookbackWindow)
 }
 
 // ComputeValidatorsUptime retrieves the uptime score for each validator for a given epoch
diff --git a/consensus/istanbul/uptime/monitor_test.go b/consensus/istanbul/uptime/monitor_test.go
index 59d10f91dd..d2c4568c01 100644
--- a/consensus/istanbul/uptime/monitor_test.go
+++ b/consensus/istanbul/uptime/monitor_test.go
@@ -20,7 +20,7 @@ func TestUptime(t *testing.T) {
 	// start on first block of window
 	block := uint64(1)
 	// use a window of 2 blocks - ideally we want to expand
-	monitoringWindow := MonitoringWindow(1, 10, 2) // [2,8]
+	monitoringWindow := MustMonitoringWindow(1, 10, 2) // [2,8]
 
 	for _, bitmap := range bitmaps {
 		// these tests to increase our confidence
@@ -64,7 +64,7 @@ func TestUptime(t *testing.T) {
 
 func TestUptimeSingle(t *testing.T) {
 	var uptimes *Uptime
-	monitoringWindow := MonitoringWindow(2, 211, 3)
+	monitoringWindow := MustMonitoringWindow(2, 211, 3)
 	uptimes = updateUptime(uptimes, 211, big.NewInt(7), 3, monitoringWindow)
 	// the first 2 uptime updates do not get scored since they're within the
 	// first window after the epoch block
diff --git a/consensus/istanbul/utils.go b/consensus/istanbul/utils.go
index fc2645d3bb..9b99c9f48c 100644
--- a/consensus/istanbul/utils.go
+++ b/consensus/istanbul/utils.go
@@ -99,6 +99,23 @@ func GetEpochNumber(number uint64, epochSize uint64) uint64 {
 	}
 }
 
+// MustGetEpochFirstBlockGivenBlockNumber is a variant of GetEpochFirstBlockGivenBlockNumber
+// that panics if called for epoch 0 (genesis)
+func MustGetEpochFirstBlockGivenBlockNumber(blockNumber uint64, epochSize uint64) uint64 {
+	firstBlock, err := GetEpochFirstBlockGivenBlockNumber(blockNumber, epochSize)
+	if err != nil {
+		panic(err)
+	}
+	return firstBlock
+}
+
+// GetEpochFirstBlockGivenBlockNumber retrieves first block of a given block's epoch
+// Fails when try to obtain first block of epoch 0 (genesis)
+func GetEpochFirstBlockGivenBlockNumber(blockNumber uint64, epochSize uint64) (uint64, error) {
+	epochNumber := GetEpochNumber(blockNumber, epochSize)
+	return GetEpochFirstBlockNumber(epochNumber, epochSize)
+}
+
 // GetEpochFirstBlockNumber retrieves first block of epoch.
 func GetEpochFirstBlockNumber(epochNumber uint64, epochSize uint64) (uint64, error) {
 	// Epoch 0 is just the genesis block, it doesn't have a first block (only last)
diff --git a/contract_comm/blockchain_parameters/blockchain_parameters.go b/contract_comm/blockchain_parameters/blockchain_parameters.go
index 5a9bb32e07..24576f9002 100644
--- a/contract_comm/blockchain_parameters/blockchain_parameters.go
+++ b/contract_comm/blockchain_parameters/blockchain_parameters.go
@@ -84,18 +84,18 @@ const (
 		"type": "function"
 	},
 	{
-	"constant": true,
-	"inputs": [],
-	"name": "intrinsicGasForAlternativeFeeCurrency",
-	"outputs": [
-		{
-		"name": "",
-		"type": "uint256"
-		}
-	],
-	"payable": false,
-	"stateMutability": "view",
-	"type": "function"
+		"constant": true,
+		"inputs": [],
+		"name": "intrinsicGasForAlternativeFeeCurrency",
+		"outputs": [
+			{
+				"name": "",
+				"type": "uint256"
+			}
+		],
+		"payable": false,
+		"stateMutability": "view",
+		"type": "function"
 	}
 ]`
 )
diff --git a/eth/backend.go b/eth/backend.go
index 4e17f3a5e6..de631c6a42 100644
--- a/eth/backend.go
+++ b/eth/backend.go
@@ -276,7 +276,7 @@ func CreateConsensusEngine(ctx *node.ServiceContext, chainConfig *params.ChainCo
 	// If Istanbul is requested, set it up
 	if chainConfig.Istanbul != nil {
 		log.Debug("Setting up Istanbul consensus engine")
-		if err := istanbul.ApplyChainConfigToConfig(chainConfig, &config.Istanbul); err != nil {
+		if err := istanbul.ApplyParamsChainConfigToConfig(chainConfig, &config.Istanbul); err != nil {
 			log.Crit("Invalid Configuration for Istanbul Engine", "err", err)
 		}
 
diff --git a/params/config.go b/params/config.go
index 71fe0cf35f..2328c7ab69 100644
--- a/params/config.go
+++ b/params/config.go
@@ -114,7 +114,7 @@ var (
 			Epoch:          17280,
 			ProposerPolicy: 2,
 			BlockPeriod:    5,
-			RequestTimeout: 10000,
+			RequestTimeout: 3000,
 			LookbackWindow: 12,
 		},
 	}

From 41afab4e1ccb1018144d8f806687728d0af01b39 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Fri, 15 Jan 2021 18:59:57 -0300
Subject: [PATCH 11/16] fix test

---
 consensus/istanbul/backend/test_utils.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/consensus/istanbul/backend/test_utils.go b/consensus/istanbul/backend/test_utils.go
index 4917bf1d4f..7af58c5b87 100644
--- a/consensus/istanbul/backend/test_utils.go
+++ b/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)

From 55ca39b1dbe2d349559409d11b3210f47f90c0e8 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Mon, 18 Jan 2021 17:39:44 -0300
Subject: [PATCH 12/16] increase docker size for some e2e tests

---
 .circleci/config.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index be8b5819ff..c756d228d8 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -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

From 7619ff4db6ab839da9c4dd5b37ecab7b55645ef1 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Tue, 19 Jan 2021 16:55:21 -0300
Subject: [PATCH 13/16] On SmartContract error use defaultLookbackWindow

This commits also changes the behaviour pre-hardfork, but it it
considered safe.
---
 consensus/istanbul/uptime/config.go      | 32 ++++++++++++++----------
 consensus/istanbul/uptime/config_test.go |  2 ++
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/consensus/istanbul/uptime/config.go b/consensus/istanbul/uptime/config.go
index cefae82657..c2087a4c1a 100644
--- a/consensus/istanbul/uptime/config.go
+++ b/consensus/istanbul/uptime/config.go
@@ -24,32 +24,38 @@ const (
 // ComputeLookbackWindow computes the lookbackWindow based on different required parameters.
 // getLookbackWindow represents the way to obtain lookbackWindow from the smart contract
 func ComputeLookbackWindow(epochSize uint64, defaultLookbackWindow uint64, isDonut bool, getLookbackWindow func() (uint64, error)) uint64 {
-	if !isDonut {
-		return defaultLookbackWindow
-	}
+	var returnValue uint64
 
 	if epochSize <= istanbul.MinEpochSize {
 		panic("Invalid epoch value")
 	}
 
-	value, err := getLookbackWindow()
-	if err != nil {
-		value = MinSafeLookbackWindow
+	if !isDonut {
+		returnValue = defaultLookbackWindow
+	} else {
+		var err error
+		returnValue, err = getLookbackWindow()
+		if err != nil {
+			// It can fail because smart contract it's not present
+			// or it's not initialized
+			// in both cases, we use the old value => defaultLookbackWindow
+			returnValue = defaultLookbackWindow
+		}
 	}
 
 	// Adjust to safe range
-	if value < MinSafeLookbackWindow {
-		value = MinSafeLookbackWindow
-	} else if value > MaxSafeLookbackWindow {
-		value = MaxSafeLookbackWindow
+	if returnValue < MinSafeLookbackWindow {
+		returnValue = MinSafeLookbackWindow
+	} else if returnValue > MaxSafeLookbackWindow {
+		returnValue = MaxSafeLookbackWindow
 	}
 
 	// Ensure it's sensible to given chain params
-	if value > (epochSize - BlocksToSkipAtEpochEnd) {
-		value = epochSize - BlocksToSkipAtEpochEnd
+	if returnValue > (epochSize - BlocksToSkipAtEpochEnd) {
+		returnValue = epochSize - BlocksToSkipAtEpochEnd
 	}
 
-	return value
+	return returnValue
 }
 
 // MustMonitoringWindow is a MonitoringWindow variant that panics on error
diff --git a/consensus/istanbul/uptime/config_test.go b/consensus/istanbul/uptime/config_test.go
index baaac49218..ce33c86115 100644
--- a/consensus/istanbul/uptime/config_test.go
+++ b/consensus/istanbul/uptime/config_test.go
@@ -1,6 +1,7 @@
 package uptime
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/ethereum/go-ethereum/consensus/istanbul"
@@ -68,6 +69,7 @@ func TestComputeLookbackWindow(t *testing.T) {
 		want uint64
 	}{
 		{"returns default if Donut is not active", args{100, 20, false, constant(24)}, 20},
+		{"returns default if call fails", args{100, 20, true, func() (uint64, error) { return 10, errors.New("some error") }}, 64},
 		{"returns safe minimum if configured is below", args{100, 20, true, constant(2)}, MinSafeLookbackWindow},
 		{"returns safe maximum if configured is above", args{1000, 20, true, constant(800)}, MaxSafeLookbackWindow},
 		{"returns epochSize-2 if configured is above", args{100, 20, true, constant(99)}, 98},

From a1898fa8fcf83aa4931374d9114467f958bf19de Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Tue, 19 Jan 2021 17:21:58 -0300
Subject: [PATCH 14/16] Revert behaviour pre-donut

---
 consensus/istanbul/uptime/config.go | 34 +++++++++++++----------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/consensus/istanbul/uptime/config.go b/consensus/istanbul/uptime/config.go
index c2087a4c1a..4be60d0b06 100644
--- a/consensus/istanbul/uptime/config.go
+++ b/consensus/istanbul/uptime/config.go
@@ -24,38 +24,34 @@ const (
 // ComputeLookbackWindow computes the lookbackWindow based on different required parameters.
 // getLookbackWindow represents the way to obtain lookbackWindow from the smart contract
 func ComputeLookbackWindow(epochSize uint64, defaultLookbackWindow uint64, isDonut bool, getLookbackWindow func() (uint64, error)) uint64 {
-	var returnValue uint64
+	if !isDonut {
+		return defaultLookbackWindow
+	}
 
 	if epochSize <= istanbul.MinEpochSize {
 		panic("Invalid epoch value")
 	}
 
-	if !isDonut {
-		returnValue = defaultLookbackWindow
-	} else {
-		var err error
-		returnValue, err = getLookbackWindow()
-		if err != nil {
-			// It can fail because smart contract it's not present
-			// or it's not initialized
-			// in both cases, we use the old value => defaultLookbackWindow
-			returnValue = defaultLookbackWindow
-		}
+	value, err := getLookbackWindow()
+	if err != nil {
+		// It can fail because smart contract it's not present or it's not initialized
+		// in both cases, we use the old value => defaultLookbackWindow
+		value = defaultLookbackWindow
 	}
 
 	// Adjust to safe range
-	if returnValue < MinSafeLookbackWindow {
-		returnValue = MinSafeLookbackWindow
-	} else if returnValue > MaxSafeLookbackWindow {
-		returnValue = MaxSafeLookbackWindow
+	if value < MinSafeLookbackWindow {
+		value = MinSafeLookbackWindow
+	} else if value > MaxSafeLookbackWindow {
+		value = MaxSafeLookbackWindow
 	}
 
 	// Ensure it's sensible to given chain params
-	if returnValue > (epochSize - BlocksToSkipAtEpochEnd) {
-		returnValue = epochSize - BlocksToSkipAtEpochEnd
+	if value > (epochSize - BlocksToSkipAtEpochEnd) {
+		value = epochSize - BlocksToSkipAtEpochEnd
 	}
 
-	return returnValue
+	return value
 }
 
 // MustMonitoringWindow is a MonitoringWindow variant that panics on error

From c598ae0898257d0a7351c2ff143fca58ee177997 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Tue, 19 Jan 2021 17:23:37 -0300
Subject: [PATCH 15/16] Prefer cip21 to isDonut for variables

---
 consensus/istanbul/backend/engine.go     | 4 ++--
 consensus/istanbul/uptime/config.go      | 4 ++--
 consensus/istanbul/uptime/config_test.go | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go
index a03950c889..419e163195 100644
--- a/consensus/istanbul/backend/engine.go
+++ b/consensus/istanbul/backend/engine.go
@@ -434,12 +434,12 @@ func (sb *Backend) LookbackWindow(header *types.Header, state *state.StateDB) ui
 	// 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)
-	isDonutActivated := sb.chain.Config().IsDonut(new(big.Int).SetUint64(firstBlockOfEpoch))
+	cip21Activated := sb.chain.Config().IsDonut(new(big.Int).SetUint64(firstBlockOfEpoch))
 
 	return uptime.ComputeLookbackWindow(
 		sb.config.Epoch,
 		sb.config.DefaultLookbackWindow,
-		isDonutActivated,
+		cip21Activated,
 		func() (uint64, error) { return blockchain_parameters.GetLookbackWindow(header, state) },
 	)
 }
diff --git a/consensus/istanbul/uptime/config.go b/consensus/istanbul/uptime/config.go
index 4be60d0b06..a1542945ec 100644
--- a/consensus/istanbul/uptime/config.go
+++ b/consensus/istanbul/uptime/config.go
@@ -23,8 +23,8 @@ const (
 
 // ComputeLookbackWindow computes the lookbackWindow based on different required parameters.
 // getLookbackWindow represents the way to obtain lookbackWindow from the smart contract
-func ComputeLookbackWindow(epochSize uint64, defaultLookbackWindow uint64, isDonut bool, getLookbackWindow func() (uint64, error)) uint64 {
-	if !isDonut {
+func ComputeLookbackWindow(epochSize uint64, defaultLookbackWindow uint64, cip21 bool, getLookbackWindow func() (uint64, error)) uint64 {
+	if !cip21 {
 		return defaultLookbackWindow
 	}
 
diff --git a/consensus/istanbul/uptime/config_test.go b/consensus/istanbul/uptime/config_test.go
index ce33c86115..8e3888cf41 100644
--- a/consensus/istanbul/uptime/config_test.go
+++ b/consensus/istanbul/uptime/config_test.go
@@ -60,7 +60,7 @@ func TestComputeLookbackWindow(t *testing.T) {
 	type args struct {
 		epochSize             uint64
 		defaultLookbackWindow uint64
-		isDonut               bool
+		cip21                 bool
 		getLookbackWindow     func() (uint64, error)
 	}
 	tests := []struct {
@@ -78,7 +78,7 @@ func TestComputeLookbackWindow(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := ComputeLookbackWindow(tt.args.epochSize, tt.args.defaultLookbackWindow, tt.args.isDonut, tt.args.getLookbackWindow)
+			got := ComputeLookbackWindow(tt.args.epochSize, tt.args.defaultLookbackWindow, tt.args.cip21, tt.args.getLookbackWindow)
 			if got != tt.want {
 				t.Errorf("ComputeLookbackWindow() = %v, want %v", got, tt.want)
 			}

From 375f5650f7cde0f9ecfab1ed0298a5001e05bd25 Mon Sep 17 00:00:00 2001
From: Mariano Cortesi <mariano@clabs.co>
Date: Wed, 20 Jan 2021 14:53:59 -0300
Subject: [PATCH 16/16] fix tests

---
 consensus/istanbul/uptime/config_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/consensus/istanbul/uptime/config_test.go b/consensus/istanbul/uptime/config_test.go
index 8e3888cf41..da8704ddcd 100644
--- a/consensus/istanbul/uptime/config_test.go
+++ b/consensus/istanbul/uptime/config_test.go
@@ -69,7 +69,7 @@ func TestComputeLookbackWindow(t *testing.T) {
 		want uint64
 	}{
 		{"returns default if Donut is not active", args{100, 20, false, constant(24)}, 20},
-		{"returns default if call fails", args{100, 20, true, func() (uint64, error) { return 10, errors.New("some error") }}, 64},
+		{"returns default if call fails", args{100, 20, true, func() (uint64, error) { return 10, errors.New("some error") }}, 20},
 		{"returns safe minimum if configured is below", args{100, 20, true, constant(2)}, MinSafeLookbackWindow},
 		{"returns safe maximum if configured is above", args{1000, 20, true, constant(800)}, MaxSafeLookbackWindow},
 		{"returns epochSize-2 if configured is above", args{100, 20, true, constant(99)}, 98},