Skip to content

Commit

Permalink
fix: get revision for height instead of the latest (#1249)
Browse files Browse the repository at this point in the history
  • Loading branch information
keruch authored Nov 21, 2024
1 parent aeec343 commit f503231
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 26 deletions.
32 changes: 18 additions & 14 deletions block/fork.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ import (
"time"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
sequencers "github.com/dymensionxyz/dymension-rdk/x/sequencers/types"
"github.com/dymensionxyz/gerr-cosmos/gerrc"
"github.com/gogo/protobuf/proto"

sequencers "github.com/dymensionxyz/dymension-rdk/x/sequencers/types"
"github.com/dymensionxyz/dymint/types"

"github.com/dymensionxyz/dymint/version"
)

Expand Down Expand Up @@ -50,29 +49,33 @@ func (m *Manager) checkForkUpdate(msg string) error {
return err
}

if m.shouldStopNode(rollapp, m.State.GetRevision()) {
err = m.createInstruction(rollapp)
var (
nextHeight = m.State.NextHeight()
actualRevision = m.State.GetRevision()
expectedRevision = rollapp.GetRevisionForHeight(nextHeight)
)
if shouldStopNode(expectedRevision, nextHeight, actualRevision) {
err = m.createInstruction(expectedRevision)
if err != nil {
return err
}

m.freezeNode(fmt.Errorf("%s local_block_height: %d rollapp_revision_start_height: %d local_revision: %d rollapp_revision: %d", msg, m.State.Height(), rollapp.LatestRevision().StartHeight, m.State.GetRevision(), rollapp.LatestRevision().Number))
m.freezeNode(fmt.Errorf("%s local_block_height: %d rollapp_revision_start_height: %d local_revision: %d rollapp_revision: %d", msg, m.State.Height(), expectedRevision.StartHeight, actualRevision, expectedRevision.Number))
}

return nil
}

// createInstruction writes file to disk with fork information
func (m *Manager) createInstruction(rollapp *types.Rollapp) error {
func (m *Manager) createInstruction(expectedRevision types.Revision) error {
obsoleteDrs, err := m.SLClient.GetObsoleteDrs()
if err != nil {
return err
}

revision := rollapp.LatestRevision()
instruction := types.Instruction{
Revision: revision.Number,
RevisionStartHeight: revision.StartHeight,
Revision: expectedRevision.Number,
RevisionStartHeight: expectedRevision.StartHeight,
FaultyDRS: obsoleteDrs,
}

Expand All @@ -89,11 +92,12 @@ func (m *Manager) createInstruction(rollapp *types.Rollapp) error {
// This method checks two conditions to decide if a node should be stopped:
// 1. If the next state height is greater than or equal to the rollapp's revision start height.
// 2. If the block's app version (equivalent to revision) is less than the rollapp's revision
func (m *Manager) shouldStopNode(rollapp *types.Rollapp, revision uint64) bool {
if m.State.NextHeight() >= rollapp.LatestRevision().StartHeight && revision < rollapp.LatestRevision().Number {
return true
}
return false
func shouldStopNode(
expectedRevision types.Revision,
nextHeight uint64,
actualRevisionNumber uint64,
) bool {
return nextHeight >= expectedRevision.StartHeight && actualRevisionNumber < expectedRevision.Number
}

// forkNeeded returns true if the fork file exists
Expand Down
16 changes: 4 additions & 12 deletions block/fork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,8 @@ func TestShouldStopNode(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
state := &types.State{}
state.LastBlockHeight.Store(tt.height)

logger := log.NewNopLogger()

manager := &Manager{
State: state,
logger: logger,
}

result := manager.shouldStopNode(tt.rollapp, tt.block.Header.Version.App)
expectedRevision := tt.rollapp.GetRevisionForHeight(tt.height)
result := shouldStopNode(expectedRevision, tt.height, tt.block.Header.Version.App)
assert.Equal(t, tt.expected, result)
})
}
Expand Down Expand Up @@ -257,7 +248,8 @@ func TestCreateInstruction(t *testing.T) {
}, nil)

manager.SLClient = mockSL
err := manager.createInstruction(tt.rollapp)
expectedRevision := tt.rollapp.GetRevisionForHeight(tt.block.Header.Height)
err := manager.createInstruction(expectedRevision)
if tt.expectedError {
assert.Error(t, err)
} else {
Expand Down
105 changes: 105 additions & 0 deletions types/rollapp_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package types_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/dymensionxyz/dymint/types"
)

func TestGetRevisionForHeight(t *testing.T) {
tests := []struct {
name string
rollapp types.Rollapp
height uint64
expected types.Revision
}{
{
name: "no revisions",
rollapp: types.Rollapp{
RollappID: "test",
Revisions: []types.Revision{},
},
height: 100,
expected: types.Revision{},
},
{
name: "single revision, height matches",
rollapp: types.Rollapp{
RollappID: "test",
Revisions: []types.Revision{
{Number: 1, StartHeight: 50},
},
},
height: 50,
expected: types.Revision{Number: 1, StartHeight: 50},
},
{
name: "single revision, height does not match",
rollapp: types.Rollapp{
RollappID: "test",
Revisions: []types.Revision{
{Number: 1, StartHeight: 50},
},
},
height: 49,
expected: types.Revision{},
},
{
name: "multiple revisions, height matches first",
rollapp: types.Rollapp{
RollappID: "test",
Revisions: []types.Revision{
{Number: 1, StartHeight: 50},
{Number: 2, StartHeight: 100},
},
},
height: 50,
expected: types.Revision{Number: 1, StartHeight: 50},
},
{
name: "multiple revisions, height matches second",
rollapp: types.Rollapp{
RollappID: "test",
Revisions: []types.Revision{
{Number: 1, StartHeight: 50},
{Number: 2, StartHeight: 100},
},
},
height: 100,
expected: types.Revision{Number: 2, StartHeight: 100},
},
{
name: "multiple revisions, height between revisions",
rollapp: types.Rollapp{
RollappID: "test",
Revisions: []types.Revision{
{Number: 1, StartHeight: 50},
{Number: 2, StartHeight: 100},
},
},
height: 75,
expected: types.Revision{Number: 1, StartHeight: 50},
},
{
name: "multiple revisions, height after last revision",
rollapp: types.Rollapp{
RollappID: "test",
Revisions: []types.Revision{
{Number: 1, StartHeight: 50},
{Number: 2, StartHeight: 100},
},
},
height: 150,
expected: types.Revision{Number: 2, StartHeight: 100},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := tt.rollapp.GetRevisionForHeight(tt.height)
require.Equal(t, tt.expected, result)
})
}
}

0 comments on commit f503231

Please sign in to comment.