diff --git a/forkmanager/fork.go b/forkmanager/fork.go index b55d40e882..8df46b729c 100644 --- a/forkmanager/fork.go +++ b/forkmanager/fork.go @@ -4,22 +4,29 @@ import "github.com/0xPolygon/polygon-edge/helper/common" const InitialFork = "initialfork" -// HandlerDesc gives description for the handler -// eq: "extra", "proposer_calculator", etc +// HandlerDesc gives description for the handler eq: "extra", "proposer_calculator", etc type HandlerDesc string +// HandlerContainer keeps id of a handler and actual handler +type HandlerContainer struct { + // ID is ID of a handler + ID uint + // Handler represents an actual event handler + Handler interface{} +} + // Fork structure defines one fork type Fork struct { - // name of the fork + // Name is name of the fork Name string - // after the fork is activated, `FromBlockNumber` shows from which block is enabled + // FromBlockNumber indicates the block from which fork becomes enabled FromBlockNumber uint64 - // fork consensus parameters + // Params are fork consensus parameters Params *ForkParams - // this value is false if fork is registered but not activated + // IsActive is false if fork is registered but not activated IsActive bool - // map of all handlers registered for this fork - Handlers map[HandlerDesc]interface{} + // Handlers is a map of all handlers registered for this fork + Handlers map[HandlerDesc]HandlerContainer } // ForkParams hard-coded fork params @@ -42,16 +49,20 @@ type ForkParams 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{} + // id - if two handlers start from the same block number, the one with the greater ID should take precedence. + id uint + // fromBlockNumber defines block number after handler should be active + fromBlockNumber uint64 + // handler represents an actual event handler - instance of some structure, function etc + handler interface{} } // forkParamsBlock encapsulates block and actual fork params type forkParamsBlock struct { - // Params should be active from block `FromBlockNumber`` - FromBlockNumber uint64 - // pointer to fork params - Params *ForkParams + // id - if two params start from the same block number, the one with the greater ID should take precedence. + id uint + // fromBlockNumber defines block number after params should be active + fromBlockNumber uint64 + // params is a pointer to fork params + params *ForkParams } diff --git a/forkmanager/fork_manager.go b/forkmanager/fork_manager.go index 8ecfb2b38d..fac754057a 100644 --- a/forkmanager/fork_manager.go +++ b/forkmanager/fork_manager.go @@ -18,6 +18,7 @@ type forkManager struct { forkMap map[string]*Fork handlersMap map[HandlerDesc][]forkHandler params []forkParamsBlock + handlersIds map[HandlerDesc]uint } // GeInstance returns fork manager singleton instance. Thread safe @@ -39,6 +40,7 @@ func (fm *forkManager) Clear() { fm.forkMap = map[string]*Fork{} fm.handlersMap = map[HandlerDesc][]forkHandler{} + fm.handlersIds = map[HandlerDesc]uint{} } // RegisterFork registers fork by its name @@ -51,7 +53,7 @@ func (fm *forkManager) RegisterFork(name string, forkParams *ForkParams) { FromBlockNumber: 0, IsActive: false, Params: forkParams, - Handlers: map[HandlerDesc]interface{}{}, + Handlers: map[HandlerDesc]HandlerContainer{}, } } @@ -65,7 +67,12 @@ func (fm *forkManager) RegisterHandler(forkName string, handlerName HandlerDesc, return fmt.Errorf("fork does not exist: %s", forkName) } - fork.Handlers[handlerName] = handler + fm.handlersIds[handlerName]++ + + fork.Handlers[handlerName] = HandlerContainer{ + ID: fm.handlersIds[handlerName], + Handler: handler, + } return nil } @@ -114,8 +121,8 @@ func (fm *forkManager) DeactivateFork(forkName string) error { fork.IsActive = false - for forkHandlerName := range fork.Handlers { - fm.removeHandler(forkHandlerName, fork.FromBlockNumber) + for handlerName, handlerCont := range fork.Handlers { + fm.removeHandler(handlerName, fork.FromBlockNumber, handlerCont.ID) } fm.removeParams(fork.FromBlockNumber) @@ -135,13 +142,13 @@ 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 handlers[i].FromBlockNumber > blockNumber + return handlers[i].fromBlockNumber > blockNumber }) - 1 if pos < 0 { return nil } - return handlers[pos].Handler + return handlers[pos].handler } // GetParams retrieves chain.ForkParams for a block number @@ -151,13 +158,13 @@ func (fm *forkManager) GetParams(blockNumber uint64) *ForkParams { // binary search to find the desired *chain.ForkParams pos := sort.Search(len(fm.params), func(i int) bool { - return fm.params[i].FromBlockNumber > blockNumber + return fm.params[i].fromBlockNumber > blockNumber }) - 1 if pos < 0 { return nil } - return fm.params[pos].Params + return fm.params[pos].params } // IsForkRegistered checks if fork is registered @@ -200,47 +207,47 @@ func (fm *forkManager) GetForkBlock(name string) (uint64, error) { return fork.FromBlockNumber, nil } -func (fm *forkManager) addHandler(handlerName HandlerDesc, blockNumber uint64, handler interface{}) { +func (fm *forkManager) addHandler(handlerName HandlerDesc, blockNumber uint64, handlerCont HandlerContainer) { if handlers, exists := fm.handlersMap[handlerName]; !exists { fm.handlersMap[handlerName] = []forkHandler{ { - FromBlockNumber: blockNumber, - Handler: handler, + id: handlerCont.ID, + fromBlockNumber: blockNumber, + handler: handlerCont.Handler, }, } } else { // keep everything in sorted order index := sort.Search(len(handlers), func(i int) bool { - return handlers[i].FromBlockNumber >= blockNumber - }) - // replace existing handler if on the same block as current one - if index < len(handlers) && handlers[index].FromBlockNumber == blockNumber { - handlers[index].Handler = handler + h := handlers[i] - return - } + return h.fromBlockNumber > blockNumber || (h.fromBlockNumber == blockNumber && h.id >= handlerCont.ID) + }) handlers = append(handlers, forkHandler{}) copy(handlers[index+1:], handlers[index:]) handlers[index] = forkHandler{ - FromBlockNumber: blockNumber, - Handler: handler, + id: handlerCont.ID, + fromBlockNumber: blockNumber, + handler: handlerCont.Handler, } fm.handlersMap[handlerName] = handlers } } -func (fm *forkManager) removeHandler(handlerName HandlerDesc, blockNumber uint64) { +func (fm *forkManager) removeHandler(handlerName HandlerDesc, blockNumber uint64, id uint) { handlers, exists := fm.handlersMap[handlerName] if !exists { return } index := sort.Search(len(handlers), func(i int) bool { - return handlers[i].FromBlockNumber >= blockNumber + h := handlers[i] + + return h.fromBlockNumber > blockNumber || (h.fromBlockNumber == blockNumber && h.id >= id) }) - if index < len(handlers) && handlers[index].FromBlockNumber == blockNumber { + if index < len(handlers) && handlers[index].fromBlockNumber == blockNumber && handlers[index].id == id { copy(handlers[index:], handlers[index+1:]) handlers[len(handlers)-1] = forkHandler{} fm.handlersMap[handlerName] = handlers[:len(handlers)-1] @@ -252,14 +259,14 @@ func (fm *forkManager) addParams(blockNumber uint64, params *ForkParams) { return } - item := forkParamsBlock{FromBlockNumber: blockNumber, Params: params} + 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 + return fm.params[i].fromBlockNumber >= blockNumber }) fm.params = append(fm.params, forkParamsBlock{}) @@ -268,22 +275,22 @@ func (fm *forkManager) addParams(blockNumber uint64, params *ForkParams) { if index > 0 { // copy all nil parameters from previous - copyParams(item.Params, fm.params[index-1].Params) + 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) + 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 + return fm.params[i].fromBlockNumber >= blockNumber }) - if index < len(fm.params) && fm.params[index].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] = forkParamsBlock{} fm.params = fm.params[:len(fm.params)-1] diff --git a/forkmanager/fork_manager_test.go b/forkmanager/fork_manager_test.go index 21499b79c1..7f38579cbc 100644 --- a/forkmanager/fork_manager_test.go +++ b/forkmanager/fork_manager_test.go @@ -54,6 +54,17 @@ func TestForkManager(t *testing.T) { assert.Equal(t, 2, handlersACnt) assert.Equal(t, 3, handlersBCnt) + // double deactivate should be same as single deactivate + assert.NoError(t, forkManager.DeactivateFork(ForkA)) + assert.NoError(t, forkManager.DeactivateFork(ForkA)) + assert.Equal(t, 1, len(forkManager.handlersMap[HandlerA])) + assert.Equal(t, 2, len(forkManager.handlersMap[HandlerB])) + + // activate fork again + assert.NoError(t, forkManager.ActivateFork(ForkA, 0)) + assert.Equal(t, handlersACnt, len(forkManager.handlersMap[HandlerA])) + assert.Equal(t, handlersBCnt, len(forkManager.handlersMap[HandlerB])) + t.Run("activate not registered fork", func(t *testing.T) { t.Parallel() @@ -70,6 +81,12 @@ func TestForkManager(t *testing.T) { assert.Equal(t, handlersBCnt, len(forkManager.handlersMap[HandlerB])) }) + t.Run("deactivate not registered fork", func(t *testing.T) { + t.Parallel() + + assert.Error(t, forkManager.DeactivateFork(ForkE)) + }) + t.Run("is fork enabled", func(t *testing.T) { t.Parallel() @@ -199,6 +216,7 @@ func TestForkManager_Deactivate(t *testing.T) { forkManager := &forkManager{ forkMap: map[string]*Fork{}, handlersMap: map[HandlerDesc][]forkHandler{}, + handlersIds: map[HandlerDesc]uint{}, } mvs1, mvs2 := uint64(1), uint64(2) @@ -244,6 +262,7 @@ func TestForkManager_HandlerReplacement(t *testing.T) { forkManager := &forkManager{ forkMap: map[string]*Fork{}, handlersMap: map[HandlerDesc][]forkHandler{}, + handlersIds: map[HandlerDesc]uint{}, } execute := func(name HandlerDesc, block uint64) string { @@ -274,3 +293,65 @@ func TestForkManager_HandlerReplacement(t *testing.T) { assert.Equal(t, "ADH", execute(HandlerA, i+10)) } } + +func TestForkManager_HandlerPrecedence(t *testing.T) { + t.Parallel() + + forkManager := &forkManager{ + forkMap: map[string]*Fork{}, + handlersMap: map[HandlerDesc][]forkHandler{}, + handlersIds: map[HandlerDesc]uint{}, + } + + execute := func(name HandlerDesc, block uint64) string { + //nolint:forcetypeassert + return forkManager.GetHandler(name, block).(func() string)() + } + + forkManager.RegisterFork(ForkA, nil) + forkManager.RegisterFork(ForkB, nil) + forkManager.RegisterFork(ForkC, nil) + forkManager.RegisterFork(ForkD, nil) + forkManager.RegisterFork(ForkE, nil) + + assert.NoError(t, forkManager.RegisterHandler(ForkA, HandlerA, func() string { return "A" })) + assert.NoError(t, forkManager.RegisterHandler(ForkB, HandlerA, func() string { return "B" })) + assert.NoError(t, forkManager.RegisterHandler(ForkC, HandlerA, func() string { return "C" })) + assert.NoError(t, forkManager.RegisterHandler(ForkD, HandlerA, func() string { return "D" })) + assert.NoError(t, forkManager.RegisterHandler(ForkE, HandlerA, func() string { return "E" })) + + assert.NoError(t, forkManager.ActivateFork(ForkE, 10)) + assert.NoError(t, forkManager.ActivateFork(ForkC, 0)) + assert.NoError(t, forkManager.ActivateFork(ForkA, 0)) + assert.NoError(t, forkManager.ActivateFork(ForkB, 0)) + assert.NoError(t, forkManager.ActivateFork(ForkD, 0)) + + assert.Equal(t, "D", execute(HandlerA, 0)) + assert.NoError(t, forkManager.DeactivateFork(ForkD)) + assert.Equal(t, "C", execute(HandlerA, 0)) + assert.NoError(t, forkManager.DeactivateFork(ForkC)) + assert.Equal(t, "B", execute(HandlerA, 0)) + assert.NoError(t, forkManager.DeactivateFork(ForkB)) + assert.Equal(t, "A", execute(HandlerA, 0)) + assert.NoError(t, forkManager.DeactivateFork(ForkA)) + assert.Nil(t, forkManager.GetHandler(HandlerA, 0)) + assert.Equal(t, "E", execute(HandlerA, 10)) + + assert.NoError(t, forkManager.ActivateFork(ForkA, 0)) + assert.NoError(t, forkManager.ActivateFork(ForkB, 0)) + assert.NoError(t, forkManager.ActivateFork(ForkC, 0)) + + assert.Equal(t, "C", execute(HandlerA, 0)) + assert.NoError(t, forkManager.DeactivateFork(ForkC)) + assert.Equal(t, "B", execute(HandlerA, 0)) + assert.NoError(t, forkManager.DeactivateFork(ForkB)) + assert.Equal(t, "A", execute(HandlerA, 0)) + assert.NoError(t, forkManager.DeactivateFork(ForkA)) + + assert.NoError(t, forkManager.ActivateFork(ForkB, 0)) + assert.NoError(t, forkManager.ActivateFork(ForkC, 0)) + assert.Equal(t, "C", execute(HandlerA, 0)) + assert.NoError(t, forkManager.DeactivateFork(ForkC)) + assert.Equal(t, "B", execute(HandlerA, 0)) + assert.NoError(t, forkManager.DeactivateFork(ForkB)) +}