From 641f4ba5aa2b01f9e12bc1b9911ccfcead2e301d Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Wed, 4 Dec 2024 14:37:07 +0100 Subject: [PATCH 01/11] fix --- block/fork.go | 13 ++++++++----- block/fork_test.go | 2 +- block/manager.go | 13 ++++++------- block/modes.go | 2 ++ 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/block/fork.go b/block/fork.go index 5788d27b7..b9d104d07 100644 --- a/block/fork.go +++ b/block/fork.go @@ -39,6 +39,9 @@ func (m *Manager) MonitorForkUpdateLoop(ctx context.Context) error { // checkForkUpdate checks if the hub has a fork update func (m *Manager) checkForkUpdate(msg string) error { + defer m.forkMu.Unlock() + m.forkMu.Lock() + rollapp, err := m.SLClient.GetRollapp() if err != nil { return err @@ -50,7 +53,7 @@ func (m *Manager) checkForkUpdate(msg string) error { expectedRevision = rollapp.GetRevisionForHeight(nextHeight) ) - if shouldStopNode(expectedRevision, nextHeight, actualRevision, m.RunMode) { + if shouldStopNode(expectedRevision, nextHeight, actualRevision) { instruction, err := m.createInstruction(expectedRevision) if err != nil { return err @@ -91,11 +94,7 @@ func shouldStopNode( expectedRevision types.Revision, nextHeight uint64, actualRevisionNumber uint64, - nodeMode uint, ) bool { - if nodeMode == RunModeFullNode { - return nextHeight > expectedRevision.StartHeight && actualRevisionNumber < expectedRevision.Number - } return nextHeight >= expectedRevision.StartHeight && actualRevisionNumber < expectedRevision.Number } @@ -251,6 +250,10 @@ func (m *Manager) updateStateWhenFork() error { // checkRevisionAndFork checks if fork is needed after syncing, and performs fork actions func (m *Manager) checkRevisionAndFork() error { + + defer m.forkMu.Unlock() + m.forkMu.Lock() + // it is checked again whether the node is the active proposer, since this could have changed after syncing. amIProposerOnSL, err := m.AmIProposerOnSL() if err != nil { diff --git a/block/fork_test.go b/block/fork_test.go index f9d25d00f..9af089df5 100644 --- a/block/fork_test.go +++ b/block/fork_test.go @@ -87,7 +87,7 @@ func TestShouldStopNode(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { expectedRevision := tt.rollapp.GetRevisionForHeight(tt.height) - result := shouldStopNode(expectedRevision, tt.height, tt.block.Header.Version.App, tt.runMode) + result := shouldStopNode(expectedRevision, tt.height, tt.block.Header.Version.App) assert.Equal(t, tt.expected, result) }) } diff --git a/block/manager.go b/block/manager.go index 735aab53a..6342d01d5 100644 --- a/block/manager.go +++ b/block/manager.go @@ -78,6 +78,9 @@ type Manager struct { // LastBlockTime is the time of last produced block, used to measure batch skew time LastBlockTime atomic.Int64 + + // mutex used to avoid stopping node when fork is detected but proposer is creating/sending fork batch + forkMu sync.Mutex /* Sequencer and full-node */ @@ -247,13 +250,9 @@ func (m *Manager) Start(ctx context.Context) error { return fmt.Errorf("am i proposer on SL: %w", err) } - if amIProposerOnSL || m.AmIProposerOnRollapp() { - m.RunMode = RunModeProposer - } else { - m.RunMode = RunModeFullNode - } + amIProposer := amIProposerOnSL || m.AmIProposerOnRollapp() - m.logger.Info("starting block manager", "mode", map[bool]string{true: "proposer", false: "full node"}[m.RunMode == RunModeProposer]) + m.logger.Info("starting block manager", "mode", map[bool]string{true: "proposer", false: "full node"}[amIProposer]) // update local state from latest state in settlement err = m.updateFromLastSettlementState() @@ -292,7 +291,7 @@ func (m *Manager) Start(ctx context.Context) error { }) // run based on the node role - if m.RunMode == RunModeFullNode { + if !amIProposer { return m.runAsFullNode(ctx, eg) } diff --git a/block/modes.go b/block/modes.go index b8d2835b6..7c5b3114c 100644 --- a/block/modes.go +++ b/block/modes.go @@ -23,6 +23,7 @@ const ( // setFraudHandler sets the fraud handler for the block manager. func (m *Manager) runAsFullNode(ctx context.Context, eg *errgroup.Group) error { m.logger.Info("starting block manager", "mode", "full node") + m.RunMode = RunModeFullNode // update latest finalized height err := m.updateLastFinalizedHeightFromSettlement() if err != nil { @@ -49,6 +50,7 @@ func (m *Manager) runAsFullNode(ctx context.Context, eg *errgroup.Group) error { func (m *Manager) runAsProposer(ctx context.Context, eg *errgroup.Group) error { m.logger.Info("starting block manager", "mode", "proposer") + m.RunMode = RunModeProposer // Subscribe to batch events, to update last submitted height in case batch confirmation was lost. This could happen if the sequencer crash/restarted just after submitting a batch to the settlement and by the time we query the last batch, this batch wasn't accepted yet. go uevent.MustSubscribe(ctx, m.Pubsub, "updateSubmittedHeightLoop", settlement.EventQueryNewSettlementBatchAccepted, m.UpdateLastSubmittedHeight, m.logger) // Subscribe to P2P received blocks events (used for P2P syncing). From 177e182669f5b8fe5bd0071ef201635346caeb08 Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Wed, 4 Dec 2024 22:22:05 +0100 Subject: [PATCH 02/11] remove instruction load --- block/fork.go | 40 ++++++++++++++++++++++------------------ block/modes.go | 10 ++-------- types/instruction.go | 10 ++++++---- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/block/fork.go b/block/fork.go index b9d104d07..8ef872a8f 100644 --- a/block/fork.go +++ b/block/fork.go @@ -100,7 +100,19 @@ func shouldStopNode( // forkNeeded returns true if the fork file exists func (m *Manager) forkNeeded() (types.Instruction, bool) { - if instruction, err := types.LoadInstructionFromDisk(m.RootDir); err == nil { + rollapp, err := m.SLClient.GetRollapp() + if err != nil { + return types.Instruction{}, false + } + + nextHeight := m.State.NextHeight() + expectedRevision := rollapp.GetRevisionForHeight(nextHeight) + + if nextHeight != expectedRevision.StartHeight { + return types.Instruction{}, false + } + + if instruction, err := m.createInstruction(expectedRevision); err == nil { return instruction, true } @@ -234,15 +246,13 @@ func (m *Manager) updateStateWhenFork() error { // Upgrade revision on state m.State.RevisionStartHeight = instruction.RevisionStartHeight // this is necessary to pass ValidateConfigWithRollappParams when DRS upgrade is required - if instruction.RevisionStartHeight == m.State.NextHeight() { - m.State.SetRevision(instruction.Revision) - drsVersion, err := version.GetDRSVersion() - if err != nil { - return err - } - m.State.RollappParams.DrsVersion = drsVersion + m.State.SetRevision(instruction.Revision) + drsVersion, err := version.GetDRSVersion() + if err != nil { + return err } - _, err := m.Store.SaveState(m.State, nil) + m.State.RollappParams.DrsVersion = drsVersion + _, err = m.Store.SaveState(m.State, nil) return err } return nil @@ -274,6 +284,7 @@ func (m *Manager) checkRevisionAndFork() error { if err != nil { return err } + expectedRevision := rollapp.GetRevisionForHeight(m.State.NextHeight()) // create fork batch in case it has not been submitted yet @@ -296,13 +307,6 @@ func (m *Manager) checkRevisionAndFork() error { panic("Inconsistent expected revision number from Hub. Unable to fork") } - // remove instruction file after fork to avoid enter fork loop again - if _, instructionExists := m.forkNeeded(); instructionExists { - err := types.DeleteInstructionFromDisk(m.RootDir) - if err != nil { - return fmt.Errorf("deleting instruction file: %w", err) - } - } - - return nil + // remove instruction file after fork + return types.DeleteInstructionFromDisk(m.RootDir) } diff --git a/block/modes.go b/block/modes.go index 7c5b3114c..075c52705 100644 --- a/block/modes.go +++ b/block/modes.go @@ -37,15 +37,9 @@ func (m *Manager) runAsFullNode(ctx context.Context, eg *errgroup.Group) error { m.subscribeFullNodeEvents(ctx) - if _, instructionExists := m.forkNeeded(); instructionExists { - // remove instruction file after fork to avoid enter fork loop again - err := types.DeleteInstructionFromDisk(m.RootDir) - if err != nil { - return fmt.Errorf("deleting instruction file: %w", err) - } - } + // remove instruction file after fork to avoid enter fork loop again + return types.DeleteInstructionFromDisk(m.RootDir) - return nil } func (m *Manager) runAsProposer(ctx context.Context, eg *errgroup.Group) error { diff --git a/types/instruction.go b/types/instruction.go index 7c093518b..ebae50aa5 100644 --- a/types/instruction.go +++ b/types/instruction.go @@ -2,6 +2,7 @@ package types import ( "encoding/json" + "fmt" "os" "path/filepath" ) @@ -52,11 +53,12 @@ func InstructionExists(dir string) bool { } func DeleteInstructionFromDisk(dir string) error { + if !InstructionExists(dir) { + return nil + } filePath := filepath.Join(dir, instructionFileName) - err := os.Remove(filePath) - if err != nil { - return err + if err := os.Remove(filePath); err != nil { + return fmt.Errorf("deleting instruction file: %w", err) } - return nil } From 3b525a9b13d54e63196021c7a802235bfd6b09c0 Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Wed, 4 Dec 2024 22:47:35 +0100 Subject: [PATCH 03/11] clean --- block/fork.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/block/fork.go b/block/fork.go index 8ef872a8f..16d189c09 100644 --- a/block/fork.go +++ b/block/fork.go @@ -98,7 +98,7 @@ func shouldStopNode( return nextHeight >= expectedRevision.StartHeight && actualRevisionNumber < expectedRevision.Number } -// forkNeeded returns true if the fork file exists +// forkNeeded returns true if fork action is required func (m *Manager) forkNeeded() (types.Instruction, bool) { rollapp, err := m.SLClient.GetRollapp() if err != nil { @@ -280,19 +280,13 @@ func (m *Manager) checkRevisionAndFork() error { } // get the revision for the current height to check against local state - rollapp, err := m.SLClient.GetRollapp() - if err != nil { - return err + instruction, forkNeeded := m.forkNeeded() + if !forkNeeded { + return nil } - expectedRevision := rollapp.GetRevisionForHeight(m.State.NextHeight()) - // create fork batch in case it has not been submitted yet - if m.LastSettlementHeight.Load() < expectedRevision.StartHeight { - instruction, err := m.createInstruction(expectedRevision) - if err != nil { - return err - } + if m.LastSettlementHeight.Load() < instruction.RevisionStartHeight { // update revision with revision after fork m.State.SetRevision(instruction.Revision) // create and submit fork batch @@ -303,7 +297,7 @@ func (m *Manager) checkRevisionAndFork() error { } // this cannot happen. it means the revision number obtained is not the same or the next revision. unable to fork. - if expectedRevision.Number != m.State.GetRevision() { + if instruction.Revision != m.State.GetRevision() { panic("Inconsistent expected revision number from Hub. Unable to fork") } From a2cccee8ebc1c13583d4cc7e17ee32bdaf05b2ba Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Wed, 4 Dec 2024 22:48:21 +0100 Subject: [PATCH 04/11] comment --- block/fork.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/fork.go b/block/fork.go index 16d189c09..5fec50e39 100644 --- a/block/fork.go +++ b/block/fork.go @@ -98,7 +98,7 @@ func shouldStopNode( return nextHeight >= expectedRevision.StartHeight && actualRevisionNumber < expectedRevision.Number } -// forkNeeded returns true if fork action is required +// forkNeeded returns true + fork data if fork action is required func (m *Manager) forkNeeded() (types.Instruction, bool) { rollapp, err := m.SLClient.GetRollapp() if err != nil { From 3e496b0bbe087793809555724409f10a5c80d2cd Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Wed, 4 Dec 2024 23:14:43 +0100 Subject: [PATCH 05/11] godoc --- block/fork.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/fork.go b/block/fork.go index 5fec50e39..970ac36e7 100644 --- a/block/fork.go +++ b/block/fork.go @@ -69,7 +69,7 @@ func (m *Manager) checkForkUpdate(msg string) error { return nil } -// createInstruction writes file to disk with fork information +// createInstruction returns instruction with fork information func (m *Manager) createInstruction(expectedRevision types.Revision) (types.Instruction, error) { obsoleteDrs, err := m.SLClient.GetObsoleteDrs() if err != nil { From 8cf9efd0d9336fe6a6069a1db3dd6ddf141d0b1a Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Thu, 5 Dec 2024 09:29:07 +0100 Subject: [PATCH 06/11] test fix --- block/fork.go | 1 - block/modes.go | 1 - block/production_test.go | 2 ++ block/submit_test.go | 1 + 4 files changed, 3 insertions(+), 2 deletions(-) diff --git a/block/fork.go b/block/fork.go index 970ac36e7..1517b4bfc 100644 --- a/block/fork.go +++ b/block/fork.go @@ -260,7 +260,6 @@ func (m *Manager) updateStateWhenFork() error { // checkRevisionAndFork checks if fork is needed after syncing, and performs fork actions func (m *Manager) checkRevisionAndFork() error { - defer m.forkMu.Unlock() m.forkMu.Lock() diff --git a/block/modes.go b/block/modes.go index 075c52705..ada69a22d 100644 --- a/block/modes.go +++ b/block/modes.go @@ -39,7 +39,6 @@ func (m *Manager) runAsFullNode(ctx context.Context, eg *errgroup.Group) error { // remove instruction file after fork to avoid enter fork loop again return types.DeleteInstructionFromDisk(m.RootDir) - } func (m *Manager) runAsProposer(ctx context.Context, eg *errgroup.Group) error { diff --git a/block/production_test.go b/block/production_test.go index ac5fd12fa..4edb11934 100644 --- a/block/production_test.go +++ b/block/production_test.go @@ -329,6 +329,7 @@ func TestUpdateInitialSequencerSet(t *testing.T) { slmock.On("Start").Return(nil) slmock.On("GetProposer").Return(proposer) slmock.On("GetAllSequencers").Return([]types.Sequencer{proposer, sequencer}, nil) + slmock.On("GetRollapp").Return(&types.Rollapp{}, nil) manager, err := testutil.GetManagerWithProposerKey(testutil.GetManagerConfig(), lib2pPrivKey, slmock, 1, 1, 0, proxyApp, nil) require.NoError(err) @@ -459,6 +460,7 @@ func TestUpdateExistingSequencerSet(t *testing.T) { slmock.On("Init", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) slmock.On("Start").Return(nil) slmock.On("GetProposer").Return(proposer) + slmock.On("GetRollapp").Return(&types.Rollapp{}, nil) manager, err := testutil.GetManagerWithProposerKey(testutil.GetManagerConfig(), lib2pPrivKey, slmock, 1, 1, 0, proxyApp, nil) require.NoError(err) diff --git a/block/submit_test.go b/block/submit_test.go index c6511785e..59017b00f 100644 --- a/block/submit_test.go +++ b/block/submit_test.go @@ -193,6 +193,7 @@ func TestBatchSubmissionFailedSubmission(t *testing.T) { slmock.On("Init", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) slmock.On("Start").Return(nil) slmock.On("GetProposer").Return(proposer) + slmock.On("GetRollapp").Return(&types.Rollapp{}, nil) manager, err := testutil.GetManagerWithProposerKey(testutil.GetManagerConfig(), lib2pPrivKey, slmock, 1, 1, 0, proxyApp, nil) require.NoError(err) From d7867ce279f1c4918d1991e76b112a49a98e6865 Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Thu, 5 Dec 2024 12:13:03 +0100 Subject: [PATCH 07/11] drying --- block/fork.go | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/block/fork.go b/block/fork.go index 1517b4bfc..1d3020cdf 100644 --- a/block/fork.go +++ b/block/fork.go @@ -98,25 +98,13 @@ func shouldStopNode( return nextHeight >= expectedRevision.StartHeight && actualRevisionNumber < expectedRevision.Number } -// forkNeeded returns true + fork data if fork action is required -func (m *Manager) forkNeeded() (types.Instruction, bool) { +// getRevision returns revision data for the specific height +func (m *Manager) getRevision(height uint64) (types.Revision, error) { rollapp, err := m.SLClient.GetRollapp() if err != nil { - return types.Instruction{}, false + return types.Revision{}, err } - - nextHeight := m.State.NextHeight() - expectedRevision := rollapp.GetRevisionForHeight(nextHeight) - - if nextHeight != expectedRevision.StartHeight { - return types.Instruction{}, false - } - - if instruction, err := m.createInstruction(expectedRevision); err == nil { - return instruction, true - } - - return types.Instruction{}, false + return rollapp.GetRevisionForHeight(height), nil } // doFork creates fork blocks and submits a new batch with them @@ -240,13 +228,21 @@ func (m *Manager) submitForkBatch(height uint64) error { // updateStateWhenFork updates dymint state in case fork is detected func (m *Manager) updateStateWhenFork() error { // in case fork is detected dymint state needs to be updated - if instruction, forkNeeded := m.forkNeeded(); forkNeeded { + + // get last revision + lastRevision, err := m.getRevision(m.State.NextHeight()) + if err != nil { + return err + } + + // if next height is revision start height, update local state + if lastRevision.StartHeight == m.State.NextHeight() { // Set proposer to nil to force updating it from SL m.State.SetProposer(nil) // Upgrade revision on state - m.State.RevisionStartHeight = instruction.RevisionStartHeight + m.State.RevisionStartHeight = lastRevision.StartHeight // this is necessary to pass ValidateConfigWithRollappParams when DRS upgrade is required - m.State.SetRevision(instruction.Revision) + m.State.SetRevision(lastRevision.Number) drsVersion, err := version.GetDRSVersion() if err != nil { return err @@ -278,14 +274,18 @@ func (m *Manager) checkRevisionAndFork() error { return err } - // get the revision for the current height to check against local state - instruction, forkNeeded := m.forkNeeded() - if !forkNeeded { - return nil + // get revision next height + expectedRevision, err := m.getRevision(m.State.NextHeight()) + if err != nil { + return err } // create fork batch in case it has not been submitted yet - if m.LastSettlementHeight.Load() < instruction.RevisionStartHeight { + if m.LastSettlementHeight.Load() < expectedRevision.StartHeight { + instruction, err := m.createInstruction(expectedRevision) + if err != nil { + return err + } // update revision with revision after fork m.State.SetRevision(instruction.Revision) // create and submit fork batch @@ -296,7 +296,7 @@ func (m *Manager) checkRevisionAndFork() error { } // this cannot happen. it means the revision number obtained is not the same or the next revision. unable to fork. - if instruction.Revision != m.State.GetRevision() { + if expectedRevision.Number != m.State.GetRevision() { panic("Inconsistent expected revision number from Hub. Unable to fork") } From c757cc04d631bd7f7b24e107e7afbbd61b449609 Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Sun, 8 Dec 2024 09:49:46 +0100 Subject: [PATCH 08/11] func rename --- block/fork.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block/fork.go b/block/fork.go index 1d3020cdf..581e74dd4 100644 --- a/block/fork.go +++ b/block/fork.go @@ -98,8 +98,8 @@ func shouldStopNode( return nextHeight >= expectedRevision.StartHeight && actualRevisionNumber < expectedRevision.Number } -// getRevision returns revision data for the specific height -func (m *Manager) getRevision(height uint64) (types.Revision, error) { +// getRevisionFromSL returns revision data for the specific height +func (m *Manager) getRevisionFromSL(height uint64) (types.Revision, error) { rollapp, err := m.SLClient.GetRollapp() if err != nil { return types.Revision{}, err @@ -230,7 +230,7 @@ func (m *Manager) updateStateWhenFork() error { // in case fork is detected dymint state needs to be updated // get last revision - lastRevision, err := m.getRevision(m.State.NextHeight()) + lastRevision, err := m.getRevisionFromSL(m.State.NextHeight()) if err != nil { return err } @@ -275,7 +275,7 @@ func (m *Manager) checkRevisionAndFork() error { } // get revision next height - expectedRevision, err := m.getRevision(m.State.NextHeight()) + expectedRevision, err := m.getRevisionFromSL(m.State.NextHeight()) if err != nil { return err } From cc8836a22ad2e41300a19f10a8cc06aebfdb2b2f Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Sun, 8 Dec 2024 09:58:22 +0100 Subject: [PATCH 09/11] minor edit --- block/fork.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/block/fork.go b/block/fork.go index 581e74dd4..4c4507422 100644 --- a/block/fork.go +++ b/block/fork.go @@ -229,25 +229,28 @@ func (m *Manager) submitForkBatch(height uint64) error { func (m *Manager) updateStateWhenFork() error { // in case fork is detected dymint state needs to be updated - // get last revision - lastRevision, err := m.getRevisionFromSL(m.State.NextHeight()) + // get next revision according to node height + nextRevision, err := m.getRevisionFromSL(m.State.NextHeight()) if err != nil { return err } // if next height is revision start height, update local state - if lastRevision.StartHeight == m.State.NextHeight() { + if nextRevision.StartHeight == m.State.NextHeight() { // Set proposer to nil to force updating it from SL m.State.SetProposer(nil) // Upgrade revision on state - m.State.RevisionStartHeight = lastRevision.StartHeight - // this is necessary to pass ValidateConfigWithRollappParams when DRS upgrade is required - m.State.SetRevision(lastRevision.Number) + m.State.RevisionStartHeight = nextRevision.StartHeight + m.State.SetRevision(nextRevision.Number) + + // we set rollappparam to node drs version to pass ValidateConfigWithRollappParams check, when drs upgrade is necessary. + // if the node starts with the wrong version at revision start height, it will stop after applyBlock. drsVersion, err := version.GetDRSVersion() if err != nil { return err } m.State.RollappParams.DrsVersion = drsVersion + // update stored state _, err = m.Store.SaveState(m.State, nil) return err } From 688d0f78c453ddce7a60985bbf85becdd4cb82c7 Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Sun, 8 Dec 2024 12:05:04 +0100 Subject: [PATCH 10/11] addressing comments --- block/fork.go | 29 +++++++---------------------- block/manager.go | 4 ++-- block/modes.go | 19 +++++++++++++++++-- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/block/fork.go b/block/fork.go index 4c4507422..a731a0c63 100644 --- a/block/fork.go +++ b/block/fork.go @@ -114,7 +114,7 @@ func (m *Manager) doFork(instruction types.Instruction) error { // add consensus msgs for upgrade DRS only if current DRS is obsolete consensusMsgs, err := m.prepareDRSUpgradeMessages(instruction.FaultyDRS) if err != nil { - panic(fmt.Sprintf("prepare DRS upgrade messages: %v", err)) + return fmt.Errorf("prepare DRS upgrade messages: %v", err) } // add consensus msg to bump the account sequences in all fork cases consensusMsgs = append(consensusMsgs, &sequencers.MsgBumpAccountSequences{Authority: authtypes.NewModuleAddress("sequencers").String()}) @@ -122,13 +122,13 @@ func (m *Manager) doFork(instruction types.Instruction) error { // create fork blocks err = m.createForkBlocks(instruction, consensusMsgs) if err != nil { - panic(fmt.Sprintf("validate existing blocks: %v", err)) + return fmt.Errorf("validate fork blocks: %v", err) } } // submit fork batch including two fork blocks if err := m.submitForkBatch(instruction.RevisionStartHeight); err != nil { - panic(fmt.Sprintf("ensure batch exists: %v", err)) + return fmt.Errorf("submit fork batch: %v", err) } return nil @@ -225,8 +225,8 @@ func (m *Manager) submitForkBatch(height uint64) error { return nil } -// updateStateWhenFork updates dymint state in case fork is detected -func (m *Manager) updateStateWhenFork() error { +// updateStateForNextRevision updates dymint stored state in case next height corresponds to a new revision, to enable syncing (and validation) for rollapps with multiple revisions. +func (m *Manager) updateStateForNextRevision() error { // in case fork is detected dymint state needs to be updated // get next revision according to node height @@ -257,26 +257,11 @@ func (m *Manager) updateStateWhenFork() error { return nil } -// checkRevisionAndFork checks if fork is needed after syncing, and performs fork actions -func (m *Manager) checkRevisionAndFork() error { +// doForkWhenNewRevision creates and submit to SL fork blocks according to next revision start height. +func (m *Manager) doForkWhenNewRevision() error { defer m.forkMu.Unlock() m.forkMu.Lock() - // it is checked again whether the node is the active proposer, since this could have changed after syncing. - amIProposerOnSL, err := m.AmIProposerOnSL() - if err != nil { - return fmt.Errorf("am i proposer on SL: %w", err) - } - if !amIProposerOnSL { - return fmt.Errorf("the node is no longer the proposer. please restart.") - } - - // update sequencer in case it changed after syncing - err = m.UpdateProposerFromSL() - if err != nil { - return err - } - // get revision next height expectedRevision, err := m.getRevisionFromSL(m.State.NextHeight()) if err != nil { diff --git a/block/manager.go b/block/manager.go index 6342d01d5..da2b46ae8 100644 --- a/block/manager.go +++ b/block/manager.go @@ -196,8 +196,8 @@ func NewManager( return nil, err } - // update dymint state with fork info - err = m.updateStateWhenFork() + // update dymint state with next revision info + err = m.updateStateForNextRevision() if err != nil { return nil, err } diff --git a/block/modes.go b/block/modes.go index ada69a22d..537cef5f8 100644 --- a/block/modes.go +++ b/block/modes.go @@ -56,8 +56,23 @@ func (m *Manager) runAsProposer(ctx context.Context, eg *errgroup.Group) error { // Sequencer must wait till node is synced till last submittedHeight, in case it is not m.waitForSettlementSyncing() - // checkRevisionAndFork executes fork if necessary - err := m.checkRevisionAndFork() + // it is checked again whether the node is the active proposer, since this could have changed after syncing. + amIProposerOnSL, err := m.AmIProposerOnSL() + if err != nil { + return fmt.Errorf("am i proposer on SL: %w", err) + } + if !amIProposerOnSL { + return fmt.Errorf("the node is no longer the proposer. please restart.") + } + + // update sequencer in case it changed after syncing + err = m.UpdateProposerFromSL() + if err != nil { + return err + } + + // doForkWhenNewRevision executes fork if necessary + err = m.doForkWhenNewRevision() if err != nil { return err } From 50960a8936fe478c308672faf8605bb423944572 Mon Sep 17 00:00:00 2001 From: Sergi Rene Date: Sun, 8 Dec 2024 12:22:20 +0100 Subject: [PATCH 11/11] comments --- block/fork.go | 2 +- block/modes.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/block/fork.go b/block/fork.go index a731a0c63..ee37f7aa4 100644 --- a/block/fork.go +++ b/block/fork.go @@ -111,7 +111,7 @@ func (m *Manager) getRevisionFromSL(height uint64) (types.Revision, error) { func (m *Manager) doFork(instruction types.Instruction) error { // if fork (two) blocks are not produced and applied yet, produce them if m.State.Height() < instruction.RevisionStartHeight+1 { - // add consensus msgs for upgrade DRS only if current DRS is obsolete + // add consensus msgs to upgrade DRS to running node version (msg is created in all cases and RDK will upgrade if necessary). If returns error if running version is deprecated. consensusMsgs, err := m.prepareDRSUpgradeMessages(instruction.FaultyDRS) if err != nil { return fmt.Errorf("prepare DRS upgrade messages: %v", err) diff --git a/block/modes.go b/block/modes.go index 537cef5f8..adfd56432 100644 --- a/block/modes.go +++ b/block/modes.go @@ -65,7 +65,7 @@ func (m *Manager) runAsProposer(ctx context.Context, eg *errgroup.Group) error { return fmt.Errorf("the node is no longer the proposer. please restart.") } - // update sequencer in case it changed after syncing + // update l2 proposer from SL in case it changed after syncing err = m.UpdateProposerFromSL() if err != nil { return err