From 2df1b936bc88a11826b6e3cfb54f4323091021ab Mon Sep 17 00:00:00 2001 From: Igor Crevar Date: Fri, 23 Jun 2023 12:03:46 +0200 Subject: [PATCH] EVM-713 Fork manager params (#1645) --- chain/params.go | 45 +++++++---- chain/params_test.go | 2 +- command/genesis/params.go | 2 +- command/genesis/polybft_params.go | 7 +- consensus/polybft/polybft.go | 15 ++++ forkmanager/fork.go | 33 +++++--- forkmanager/fork_manager.go | 122 ++++++++++++++++++++++++------ forkmanager/fork_manager_test.go | 61 ++++++++++++--- server/builtin.go | 6 ++ server/server.go | 9 +++ 10 files changed, 234 insertions(+), 68 deletions(-) diff --git a/chain/params.go b/chain/params.go index fc16f5842a..edb967e02f 100644 --- a/chain/params.go +++ b/chain/params.go @@ -2,9 +2,9 @@ package chain import ( "errors" - "math/big" "sort" + "github.com/0xPolygon/polygon-edge/helper/common" "github.com/0xPolygon/polygon-edge/types" ) @@ -88,20 +88,24 @@ const ( ) // Forks is map which contains all forks and their starting blocks from genesis -type Forks map[string]*Fork +type Forks map[string]Fork // IsActive returns true if fork defined by name exists and defined for the block func (f *Forks) IsActive(name string, block uint64) bool { - ff := (*f)[name] + ff, exists := (*f)[name] - return ff != nil && ff.Active(block) + return exists && ff.Active(block) } // SetFork adds/updates fork defined by name -func (f *Forks) SetFork(name string, value *Fork) { +func (f *Forks) SetFork(name string, value Fork) { (*f)[name] = value } +func (f *Forks) RemoveFork(name string) { + delete(*f, name) +} + // At returns ForksInTime instance that shows which supported forks are enabled for the block func (f *Forks) At(block uint64) ForksInTime { return ForksInTime{ @@ -117,20 +121,35 @@ func (f *Forks) At(block uint64) ForksInTime { } } -type Fork uint64 +// ForkParams hard-coded fork params +type ForkParams struct { + // MaxValidatorSetSize indicates the maximum size of validator set + MaxValidatorSetSize *uint64 `json:"maxValidatorSetSize,omitempty"` + + // EpochSize is size of epoch + EpochSize *uint64 `json:"epochSize,omitempty"` + + // SprintSize is size of sprint + SprintSize *uint64 `json:"sprintSize,omitempty"` -func NewFork(n uint64) *Fork { - f := Fork(n) + // BlockTime is target frequency of blocks production + BlockTime *common.Duration `json:"blockTime,omitempty"` - return &f + // BlockTimeDrift defines the time slot in which a new block can be created + BlockTimeDrift *uint64 `json:"blockTimeDrift,omitempty"` } -func (f Fork) Active(block uint64) bool { - return block >= uint64(f) +type Fork struct { + Block uint64 `json:"block"` + Params *ForkParams `json:"params,omitempty"` +} + +func NewFork(n uint64) Fork { + return Fork{Block: n} } -func (f Fork) Int() *big.Int { - return big.NewInt(int64(f)) +func (f Fork) Active(block uint64) bool { + return block >= f.Block } // ForksInTime should contain all supported forks by current edge version diff --git a/chain/params_test.go b/chain/params_test.go index 9c72d2764a..c7161ea2c9 100644 --- a/chain/params_test.go +++ b/chain/params_test.go @@ -17,7 +17,7 @@ func TestParamsForks(t *testing.T) { }{ { input: `{ - "homestead": 1000 + "homestead": { "block": 1000 } }`, output: &Forks{ Homestead: NewFork(1000), diff --git a/command/genesis/params.go b/command/genesis/params.go index 5039d4becb..3abf4da668 100644 --- a/command/genesis/params.go +++ b/command/genesis/params.go @@ -369,7 +369,7 @@ func (p *genesisParams) initGenesisConfig() error { // Disable london hardfork if burn contract address is not provided enabledForks := chain.AllForksEnabled if len(p.burnContracts) == 0 { - enabledForks.SetFork(chain.London, nil) + enabledForks.RemoveFork(chain.London) } chainConfig := &chain.Chain{ diff --git a/command/genesis/polybft_params.go b/command/genesis/polybft_params.go index 118e1f31a7..efc5652dca 100644 --- a/command/genesis/polybft_params.go +++ b/command/genesis/polybft_params.go @@ -19,7 +19,6 @@ import ( "github.com/0xPolygon/polygon-edge/consensus/polybft/contractsapi/artifact" "github.com/0xPolygon/polygon-edge/consensus/polybft/validator" "github.com/0xPolygon/polygon-edge/contracts" - "github.com/0xPolygon/polygon-edge/forkmanager" "github.com/0xPolygon/polygon-edge/helper/common" "github.com/0xPolygon/polygon-edge/server" "github.com/0xPolygon/polygon-edge/types" @@ -72,10 +71,6 @@ var ( // generatePolyBftChainConfig creates and persists polybft chain configuration to the provided file path func (p *genesisParams) generatePolyBftChainConfig(o command.OutputFormatter) error { - if err := forkmanager.ForkManagerInit(polybft.ForkManagerFactory, chain.AllForksEnabled); err != nil { - return err - } - // populate premine balance map premineBalances := make(map[types.Address]*premineInfo, len(p.premine)) @@ -164,7 +159,7 @@ func (p *genesisParams) generatePolyBftChainConfig(o command.OutputFormatter) er // Disable london hardfork if burn contract address is not provided enabledForks := chain.AllForksEnabled if len(p.burnContracts) == 0 { - enabledForks.SetFork(chain.London, nil) + enabledForks.RemoveFork(chain.London) } chainConfig := &chain.Chain{ diff --git a/consensus/polybft/polybft.go b/consensus/polybft/polybft.go index a082b443be..8d6eb794e3 100644 --- a/consensus/polybft/polybft.go +++ b/consensus/polybft/polybft.go @@ -435,6 +435,21 @@ func (p *Polybft) Initialize() error { return nil } +func ForkManagerInitialParamsFactory(config *chain.Chain) (*chain.ForkParams, error) { + pbftConfig, err := GetPolyBFTConfig(config) + if err != nil { + return nil, err + } + + return &chain.ForkParams{ + MaxValidatorSetSize: &pbftConfig.MaxValidatorSetSize, + EpochSize: &pbftConfig.EpochSize, + SprintSize: &pbftConfig.SprintSize, + BlockTime: &pbftConfig.BlockTime, + BlockTimeDrift: &pbftConfig.BlockTimeDrift, + }, nil +} + // Start starts the consensus and servers func (p *Polybft) Start() error { p.logger.Info("starting polybft consensus", "signer", p.key.String()) diff --git a/forkmanager/fork.go b/forkmanager/fork.go index 0556ca6c7e..51bae5019a 100644 --- a/forkmanager/fork.go +++ b/forkmanager/fork.go @@ -18,21 +18,34 @@ type Fork struct { Name string // after the fork is activated, `FromBlockNumber` shows from which block is enabled FromBlockNumber uint64 + // fork consensus parameters + Params *chain.ForkParams // this value is false if fork is registered but not activated IsActive bool // map of all handlers registered for this fork Handlers map[HandlerDesc]interface{} } -// Handler defines one custom handler -type Handler struct { +// forkHandler defines one custom handler +type forkHandler struct { // Handler should be active from block `FromBlockNumber`` FromBlockNumber uint64 // instance of some structure, function etc Handler interface{} } -func ForkManagerInit(factory func(*chain.Forks) error, forks *chain.Forks) error { +// forkParamsBlock encapsulates block and actual fork params +type forkParamsBlock struct { + // Params should be active from block `FromBlockNumber`` + FromBlockNumber uint64 + // pointer to fork params + Params *chain.ForkParams +} + +func ForkManagerInit( + initialParams *chain.ForkParams, + factory func(*chain.Forks) error, + forks *chain.Forks) error { if factory == nil { return nil } @@ -41,16 +54,16 @@ func ForkManagerInit(factory func(*chain.Forks) error, forks *chain.Forks) error fm.Clear() // register initial fork - fm.RegisterFork(InitialFork) + fm.RegisterFork(InitialFork, initialParams) // Register forks - for name := range *forks { + for name, f := range *forks { // check if fork is not supported by current edge version if _, found := (*chain.AllForksEnabled)[name]; !found { return fmt.Errorf("fork is not available: %s", name) } - fm.RegisterFork(name) + fm.RegisterFork(name, f.Params) } // Register handlers and additional forks here @@ -64,12 +77,8 @@ func ForkManagerInit(factory func(*chain.Forks) error, forks *chain.Forks) error } // Activate forks - for name, blockNumber := range *forks { - if blockNumber == nil { - continue - } - - if err := fm.ActivateFork(name, (uint64)(*blockNumber)); err != nil { + for name, f := range *forks { + if err := fm.ActivateFork(name, f.Block); err != nil { return err } } diff --git a/forkmanager/fork_manager.go b/forkmanager/fork_manager.go index 653bd860d1..0b7d7e2899 100644 --- a/forkmanager/fork_manager.go +++ b/forkmanager/fork_manager.go @@ -2,20 +2,12 @@ package forkmanager import ( "fmt" + "reflect" "sort" "sync" -) - -/* -Regarding whether it is okay to use the Singleton pattern in Go, it's a topic of some debate. -The Singleton pattern can introduce global state and make testing and concurrent programming more challenging. -It can also make code tightly coupled and harder to maintain. -In general, it's recommended to favor dependency injection and explicit collaboration over singletons. -However, there might be scenarios where the Singleton pattern is still useful, -such as managing shared resources or maintaining a global configuration. -Just be mindful of the potential drawbacks and consider alternative patterns when appropriate. -*/ + "github.com/0xPolygon/polygon-edge/chain" +) var ( forkManagerInstance *forkManager @@ -26,7 +18,8 @@ type forkManager struct { lock sync.Mutex forkMap map[string]*Fork - handlersMap map[HandlerDesc][]Handler + handlersMap map[HandlerDesc][]forkHandler + params []*forkParamsBlock } // GeInstance returns fork manager singleton instance. Thread safe @@ -47,11 +40,11 @@ func (fm *forkManager) Clear() { defer fm.lock.Unlock() fm.forkMap = map[string]*Fork{} - fm.handlersMap = map[HandlerDesc][]Handler{} + fm.handlersMap = map[HandlerDesc][]forkHandler{} } // RegisterFork registers fork by its name -func (fm *forkManager) RegisterFork(name string) { +func (fm *forkManager) RegisterFork(name string, forkParams *chain.ForkParams) { fm.lock.Lock() defer fm.lock.Unlock() @@ -59,6 +52,7 @@ func (fm *forkManager) RegisterFork(name string) { Name: name, FromBlockNumber: 0, IsActive: false, + Params: forkParams, Handlers: map[HandlerDesc]interface{}{}, } } @@ -79,7 +73,7 @@ func (fm *forkManager) RegisterHandler(forkName string, handlerName HandlerDesc, } // ActivateFork activates fork from some block number -// All handlers belong to this fork are also activated +// All handlers and parameters belonging to this fork are also activated func (fm *forkManager) ActivateFork(forkName string, blockNumber uint64) error { fm.lock.Lock() defer fm.lock.Unlock() @@ -96,15 +90,17 @@ func (fm *forkManager) ActivateFork(forkName string, blockNumber uint64) error { fork.IsActive = true fork.FromBlockNumber = blockNumber - for forkHandlerName, forkHandler := range fork.Handlers { - fm.addHandler(forkHandlerName, blockNumber, forkHandler) + for name, handler := range fork.Handlers { + fm.addHandler(name, blockNumber, handler) } + fm.addParams(blockNumber, fork.Params) + return nil } // DeactivateFork de-activates fork -// All handlers belong to this fork are also de-activated +// All handlers and parameters belong to this fork are also de-activated func (fm *forkManager) DeactivateFork(forkName string) error { fm.lock.Lock() defer fm.lock.Unlock() @@ -124,6 +120,8 @@ func (fm *forkManager) DeactivateFork(forkName string) error { fm.removeHandler(forkHandlerName, fork.FromBlockNumber) } + fm.removeParams(fork.FromBlockNumber) + return nil } @@ -139,7 +137,7 @@ func (fm *forkManager) GetHandler(name HandlerDesc, blockNumber uint64) interfac // binary search to find the latest handler defined for a specific block pos := sort.Search(len(handlers), func(i int) bool { - return blockNumber < handlers[i].FromBlockNumber + return handlers[i].FromBlockNumber > blockNumber }) - 1 if pos < 0 { return nil @@ -148,6 +146,22 @@ func (fm *forkManager) GetHandler(name HandlerDesc, blockNumber uint64) interfac return handlers[pos].Handler } +// GetParams retrieves chain.ForkParams for a block number +func (fm *forkManager) GetParams(blockNumber uint64) *chain.ForkParams { + fm.lock.Lock() + defer fm.lock.Unlock() + + // binary search to find the desired *chain.ForkParams + pos := sort.Search(len(fm.params), func(i int) bool { + return fm.params[i].FromBlockNumber > blockNumber + }) - 1 + if pos < 0 { + return nil + } + + return fm.params[pos].Params +} + // IsForkRegistered checks if fork is registered func (fm *forkManager) IsForkRegistered(name string) bool { fm.lock.Lock() @@ -190,7 +204,7 @@ func (fm *forkManager) GetForkBlock(name string) (uint64, error) { func (fm *forkManager) addHandler(handlerName HandlerDesc, blockNumber uint64, handler interface{}) { if handlers, exists := fm.handlersMap[handlerName]; !exists { - fm.handlersMap[handlerName] = []Handler{ + fm.handlersMap[handlerName] = []forkHandler{ { FromBlockNumber: blockNumber, Handler: handler, @@ -201,9 +215,9 @@ func (fm *forkManager) addHandler(handlerName HandlerDesc, blockNumber uint64, h index := sort.Search(len(handlers), func(i int) bool { return handlers[i].FromBlockNumber >= blockNumber }) - handlers = append(handlers, Handler{}) + handlers = append(handlers, forkHandler{}) copy(handlers[index+1:], handlers[index:]) - handlers[index] = Handler{ + handlers[index] = forkHandler{ FromBlockNumber: blockNumber, Handler: handler, } @@ -218,12 +232,70 @@ func (fm *forkManager) removeHandler(handlerName HandlerDesc, blockNumber uint64 } index := sort.Search(len(handlers), func(i int) bool { - return handlers[i].FromBlockNumber == blockNumber + return handlers[i].FromBlockNumber >= blockNumber }) - if index != -1 { + if index < len(handlers) && handlers[index].FromBlockNumber == blockNumber { copy(handlers[index:], handlers[index+1:]) - handlers[len(handlers)-1] = Handler{} + handlers[len(handlers)-1] = forkHandler{} fm.handlersMap[handlerName] = handlers[:len(handlers)-1] } } + +func (fm *forkManager) addParams(blockNumber uint64, params *chain.ForkParams) { + if params == nil { + return + } + + item := &forkParamsBlock{FromBlockNumber: blockNumber, Params: params} + + if len(fm.params) == 0 { + fm.params = append(fm.params, item) + } else { + // keep everything in sorted order + index := sort.Search(len(fm.params), func(i int) bool { + return fm.params[i].FromBlockNumber >= blockNumber + }) + + fm.params = append(fm.params, (*forkParamsBlock)(nil)) + copy(fm.params[index+1:], fm.params[index:]) + fm.params[index] = item + + if index > 0 { + // copy all nil parameters from previous + copyParams(item.Params, fm.params[index-1].Params) + } + + // update parameters for next + for i := index; i < len(fm.params)-1; i++ { + copyParams(fm.params[i+1].Params, fm.params[i].Params) + } + } +} + +func (fm *forkManager) removeParams(blockNumber uint64) { + index := sort.Search(len(fm.params), func(i int) bool { + return fm.params[i].FromBlockNumber >= blockNumber + }) + + if index < len(fm.params) && fm.params[index].FromBlockNumber == blockNumber { + copy(fm.params[index:], fm.params[index+1:]) + fm.params[len(fm.params)-1] = nil + fm.params = fm.params[:len(fm.params)-1] + } +} + +func copyParams(dest, src *chain.ForkParams) { + srcValue := reflect.ValueOf(src).Elem() + dstValue := reflect.ValueOf(dest).Elem() + + for i := 0; i < srcValue.NumField(); i++ { + dstField := dstValue.Field(i) + srcField := srcValue.Field(i) + + // copy if dst is nil, but src is not + if dstField.Kind() == reflect.Ptr && dstField.IsNil() && !srcField.IsNil() { + dstField.Set(srcField) + } + } +} diff --git a/forkmanager/fork_manager_test.go b/forkmanager/fork_manager_test.go index d3b91b1877..69a0bad964 100644 --- a/forkmanager/fork_manager_test.go +++ b/forkmanager/fork_manager_test.go @@ -2,7 +2,10 @@ package forkmanager import ( "testing" + "time" + "github.com/0xPolygon/polygon-edge/chain" + "github.com/0xPolygon/polygon-edge/helper/common" "github.com/stretchr/testify/assert" ) @@ -22,12 +25,16 @@ const ( func TestForkManager(t *testing.T) { t.Parallel() + es1, es2, es3 := uint64(100), uint64(300), uint64(200) + mss1 := uint64(10002) + bt1, bt2 := common.Duration{Duration: time.Second * 5}, common.Duration{Duration: time.Second * 12} + forkManager := GetInstance() - forkManager.RegisterFork(ForkA) - forkManager.RegisterFork(ForkB) - forkManager.RegisterFork(ForkC) - forkManager.RegisterFork(ForkD) + forkManager.RegisterFork(ForkA, &chain.ForkParams{EpochSize: &es1, BlockTime: &bt1}) + forkManager.RegisterFork(ForkB, &chain.ForkParams{EpochSize: &es2, MaxValidatorSetSize: &mss1}) + forkManager.RegisterFork(ForkC, nil) + forkManager.RegisterFork(ForkD, &chain.ForkParams{EpochSize: &es3, BlockTime: &bt2}) assert.NoError(t, forkManager.RegisterHandler(ForkA, HandlerA, func() string { return "AAH" })) assert.NoError(t, forkManager.RegisterHandler(ForkC, HandlerA, func() string { return "ACH" })) @@ -36,10 +43,10 @@ func TestForkManager(t *testing.T) { assert.NoError(t, forkManager.RegisterHandler(ForkD, HandlerB, func() string { return "BDH" })) assert.NoError(t, forkManager.RegisterHandler(ForkC, HandlerC, func() string { return "CCH" })) + assert.NoError(t, forkManager.ActivateFork(ForkD, 300)) assert.NoError(t, forkManager.ActivateFork(ForkA, 0)) - assert.NoError(t, forkManager.ActivateFork(ForkB, 100)) assert.NoError(t, forkManager.ActivateFork(ForkC, 200)) - assert.NoError(t, forkManager.ActivateFork(ForkD, 300)) + assert.NoError(t, forkManager.ActivateFork(ForkB, 100)) handlersACnt := len(forkManager.handlersMap[HandlerA]) handlersBCnt := len(forkManager.handlersMap[HandlerB]) @@ -156,6 +163,27 @@ func TestForkManager(t *testing.T) { assert.Nil(t, forkManager.GetHandler(HandlerD, 0)) }) + + t.Run("get params", func(t *testing.T) { + t.Parallel() + + for i := uint64(0); i < uint64(4); i++ { + assert.Equal(t, es1, *forkManager.GetParams(i).EpochSize) + assert.Equal(t, es2, *forkManager.GetParams(i + 100).EpochSize) + assert.Equal(t, es2, *forkManager.GetParams(i + 200).EpochSize) + assert.Equal(t, es3, *forkManager.GetParams(i + 300).EpochSize) + + assert.Nil(t, forkManager.GetParams(i).MaxValidatorSetSize) + assert.Equal(t, mss1, *forkManager.GetParams(i + 100).MaxValidatorSetSize) + assert.Equal(t, mss1, *forkManager.GetParams(i + 200).MaxValidatorSetSize) + assert.Equal(t, mss1, *forkManager.GetParams(i + 300).MaxValidatorSetSize) + + assert.Equal(t, bt1, *forkManager.GetParams(i).BlockTime) + assert.Equal(t, bt1, *forkManager.GetParams(i + 100).BlockTime) + assert.Equal(t, bt1, *forkManager.GetParams(i + 200).BlockTime) + assert.Equal(t, bt2, *forkManager.GetParams(i + 300).BlockTime) + } + }) } func TestForkManager_Deactivate(t *testing.T) { @@ -163,25 +191,38 @@ func TestForkManager_Deactivate(t *testing.T) { forkManager := &forkManager{ forkMap: map[string]*Fork{}, - handlersMap: map[HandlerDesc][]Handler{}, + handlersMap: map[HandlerDesc][]forkHandler{}, } + mvs1, mvs2 := uint64(1), uint64(2) - forkManager.RegisterFork(ForkA) - forkManager.RegisterFork(ForkB) + forkManager.RegisterFork(ForkA, &chain.ForkParams{MaxValidatorSetSize: &mvs1}) + forkManager.RegisterFork(ForkB, &chain.ForkParams{MaxValidatorSetSize: &mvs2}) + forkManager.RegisterFork(ForkC, &chain.ForkParams{}) assert.NoError(t, forkManager.RegisterHandler(ForkA, HandlerA, func() string { return "AAH" })) assert.NoError(t, forkManager.RegisterHandler(ForkB, HandlerA, func() string { return "ABH" })) + assert.NoError(t, forkManager.ActivateFork(ForkB, 10)) + assert.NoError(t, forkManager.ActivateFork(ForkC, 20)) assert.NoError(t, forkManager.ActivateFork(ForkA, 0)) - assert.NoError(t, forkManager.ActivateFork(ForkB, 0)) assert.Equal(t, 2, len(forkManager.handlersMap[HandlerA])) + assert.Equal(t, 3, len(forkManager.params)) + assert.Equal(t, mvs2, *forkManager.GetParams(30).MaxValidatorSetSize) assert.NoError(t, forkManager.DeactivateFork(ForkA)) assert.Equal(t, 1, len(forkManager.handlersMap[HandlerA])) + assert.Equal(t, 2, len(forkManager.params)) + assert.Nil(t, forkManager.GetParams(0)) + + assert.NoError(t, forkManager.DeactivateFork(ForkC)) + + assert.Equal(t, 1, len(forkManager.handlersMap[HandlerA])) + assert.Equal(t, 1, len(forkManager.params)) assert.NoError(t, forkManager.DeactivateFork(ForkB)) assert.Equal(t, 0, len(forkManager.handlersMap[HandlerA])) + assert.Equal(t, 0, len(forkManager.params)) } diff --git a/server/builtin.go b/server/builtin.go index c84e03e97c..f98c2a4eb4 100644 --- a/server/builtin.go +++ b/server/builtin.go @@ -21,6 +21,8 @@ type ConsensusType string type ForkManagerFactory func(forks *chain.Forks) error +type ForkManagerInitialParamsFactory func(config *chain.Chain) (*chain.ForkParams, error) + const ( DevConsensus ConsensusType = "dev" IBFTConsensus ConsensusType = "ibft" @@ -52,6 +54,10 @@ var forkManagerFactory = map[ConsensusType]ForkManagerFactory{ PolyBFTConsensus: consensusPolyBFT.ForkManagerFactory, } +var forkManagerInitialParamsFactory = map[ConsensusType]ForkManagerInitialParamsFactory{ + PolyBFTConsensus: consensusPolyBFT.ForkManagerInitialParamsFactory, +} + func ConsensusSupported(value string) bool { _, ok := consensusBackends[ConsensusType(value)] diff --git a/server/server.go b/server/server.go index 4da704a30f..c1872955e3 100644 --- a/server/server.go +++ b/server/server.go @@ -281,7 +281,16 @@ func NewServer(config *Config) (*Server, error) { return nil, err } + var initialParams *chain.ForkParams + + if pf := forkManagerInitialParamsFactory[ConsensusType(engineName)]; pf != nil { + if initialParams, err = pf(config.Chain); err != nil { + return nil, err + } + } + if err := forkmanager.ForkManagerInit( + initialParams, forkManagerFactory[ConsensusType(engineName)], config.Chain.Params.Forks); err != nil { return nil, err