Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: get revision for height instead of the latest #1249

Merged
merged 1 commit into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
})
}
}
Loading