Skip to content

Commit

Permalink
fix(rollapp): bring back num blocks in state info (#1276)
Browse files Browse the repository at this point in the history
  • Loading branch information
danwt authored Oct 3, 2024
1 parent 41267b2 commit 96d7ac7
Show file tree
Hide file tree
Showing 14 changed files with 128 additions and 77 deletions.
5 changes: 3 additions & 2 deletions ibctesting/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (s *utilSuite) updateRollappState(endHeight uint64) {
stateInfo, found := rollappKeeper.GetStateInfo(s.hubCtx(), rollappChainID(), latestStateInfoIndex.Index)
startHeight := uint64(1)
if found {
startHeight = stateInfo.LastHeight() + 1
startHeight = stateInfo.StartHeight + stateInfo.NumBlocks
}
numBlocks := endHeight - startHeight + 1
// populate the block descriptors
Expand Down Expand Up @@ -238,7 +238,8 @@ func (s *utilSuite) finalizeRollappState(index uint64, endHeight uint64) (sdk.Ev
stateInfo, found := rollappKeeper.GetStateInfo(ctx, rollappChainID(), stateInfoIdx.Index)
s.Require().True(found)
// this is a hack to increase the finalized height by modifying the last state info instead of submitting a new one
stateInfo = stateInfo.WithNumBlocks(endHeight - stateInfo.StartHeight + 1)
stateInfo.NumBlocks = endHeight - stateInfo.StartHeight + 1
stateInfo.BDs.BD = make([]rollapptypes.BlockDescriptor, stateInfo.NumBlocks)
stateInfo.Status = common.Status_FINALIZED
// update the status of the stateInfo
rollappKeeper.SetStateInfo(ctx, stateInfo)
Expand Down
3 changes: 2 additions & 1 deletion proto/dymensionxyz/dymension/rollapp/state_info.proto
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ message StateInfo {
string sequencer = 2;
// startHeight is the block height of the first block in the batch
uint64 startHeight = 3;
reserved 4; // used to be num blocks, deprecating in favour of counting len of BDs
// numBlocks is the number of blocks included in this batch update
uint64 numBlocks = 4;
// DAPath is the description of the location on the DA layer
string DAPath = 5;

Expand Down
6 changes: 4 additions & 2 deletions x/delayedack/keeper/finalize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ func (s *DelayedAckTestSuite) TestFinalizePacket() {
Index: 1,
},
StartHeight: 1,
NumBlocks: tc.rollappHeight,
Status: commontypes.Status_FINALIZED,
Sequencer: proposer,
}.WithNumBlocks(tc.rollappHeight)
}

// save state info
s.App.RollappKeeper.SetStateInfo(s.Ctx, stateInfo)
Expand Down Expand Up @@ -196,9 +197,10 @@ func (s *DelayedAckTestSuite) TestFinalizeRollappPacketsByReceiver() {
Index: 1,
},
StartHeight: 1,
NumBlocks: tc.rollappHeight,
Status: commontypes.Status_FINALIZED,
Sequencer: proposer,
}.WithNumBlocks(tc.rollappHeight)
}

// save state info
s.App.RollappKeeper.SetStateInfo(s.Ctx, stateInfo)
Expand Down
6 changes: 4 additions & 2 deletions x/delayedack/keeper/invariants_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,18 +236,20 @@ func (suite *DelayedAckTestSuite) TestRollappPacketsCasesInvariant() {
Index: 1,
},
StartHeight: 1,
NumBlocks: 10,
Status: commontypes.Status_FINALIZED,
Sequencer: proposer,
}.WithNumBlocks(10)
}
stateInfo2 := rollapptypes.StateInfo{
StateInfoIndex: rollapptypes.StateInfoIndex{
RollappId: rollapp,
Index: 2,
},
StartHeight: 11,
NumBlocks: 10,
Status: commontypes.Status_PENDING,
Sequencer: proposer,
}.WithNumBlocks(10)
}

// if nothingFinalized true, all the state infos submitted should be pending
if tc.nothingFinalized {
Expand Down
2 changes: 1 addition & 1 deletion x/delayedack/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (k Keeper) getRollappFinalizedHeight(ctx sdk.Context, chainID string) (uint
}

stateInfo := k.rollappKeeper.MustGetStateInfo(ctx, chainID, latestFinalizedStateIndex.Index)
return stateInfo.GetLatestHeight(), nil
return stateInfo.StartHeight + stateInfo.NumBlocks - 1, nil
}

/* -------------------------------------------------------------------------- */
Expand Down
4 changes: 4 additions & 0 deletions x/lightclient/ante/ibc_msg_update_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func TestHandleMsgUpdateClient(t *testing.T) {
Index: 3,
},
StartHeight: 3,
NumBlocks: 1,
BDs: rollapptypes.BlockDescriptors{
BD: []rollapptypes.BlockDescriptor{
{
Expand Down Expand Up @@ -177,6 +178,7 @@ func TestHandleMsgUpdateClient(t *testing.T) {
Index: 1,
},
StartHeight: 1,
NumBlocks: 1,
BDs: rollapptypes.BlockDescriptors{
BD: []rollapptypes.BlockDescriptor{
{
Expand All @@ -193,6 +195,7 @@ func TestHandleMsgUpdateClient(t *testing.T) {
Index: 2,
},
StartHeight: 2,
NumBlocks: 1,
BDs: rollapptypes.BlockDescriptors{
BD: []rollapptypes.BlockDescriptor{
{
Expand Down Expand Up @@ -265,6 +268,7 @@ func TestHandleMsgUpdateClient(t *testing.T) {
Index: 1,
},
StartHeight: 1,
NumBlocks: 2,
BDs: rollapptypes.BlockDescriptors{
BD: []rollapptypes.BlockDescriptor{
{
Expand Down
3 changes: 3 additions & 0 deletions x/lightclient/keeper/hook_listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func TestAfterUpdateState(t *testing.T) {
stateInfo: &rollapptypes.StateInfo{
Sequencer: keepertest.Alice,
StartHeight: 1,
NumBlocks: 1,
BDs: rollapptypes.BlockDescriptors{
BD: []rollapptypes.BlockDescriptor{
{
Expand All @@ -70,6 +71,7 @@ func TestAfterUpdateState(t *testing.T) {
stateInfo: &rollapptypes.StateInfo{
Sequencer: keepertest.Alice,
StartHeight: 1,
NumBlocks: 3,
BDs: rollapptypes.BlockDescriptors{
BD: []rollapptypes.BlockDescriptor{
{
Expand Down Expand Up @@ -106,6 +108,7 @@ func TestAfterUpdateState(t *testing.T) {
stateInfo: &rollapptypes.StateInfo{
Sequencer: keepertest.Alice,
StartHeight: 1,
NumBlocks: 3,
BDs: rollapptypes.BlockDescriptors{
BD: []rollapptypes.BlockDescriptor{
{
Expand Down
1 change: 1 addition & 0 deletions x/lightclient/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type SequencerKeeperExpected interface {
type RollappKeeperExpected interface {
GetRollapp(ctx sdk.Context, rollappId string) (val rollapptypes.Rollapp, found bool)
FindStateInfoByHeight(ctx sdk.Context, rollappId string, height uint64) (*rollapptypes.StateInfo, error)
GetStateInfo(ctx sdk.Context, rollappId string, index uint64) (val rollapptypes.StateInfo, found bool)
SetRollapp(ctx sdk.Context, rollapp rollapptypes.Rollapp)
HandleFraud(ctx sdk.Context, rollappID, clientId string, fraudHeight uint64, seqAddr string) error
}
Expand Down
23 changes: 14 additions & 9 deletions x/rollapp/keeper/grpc_query_get_state_info_by_height_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ func createNStateInfoAndIndex(keeper *keeper.Keeper, ctx sdk.Context, n int, rol
Index: uint64(i + 1),
},
StartHeight: StartHeight,
}.WithNumBlocks(numBlocks)
StartHeight += stateInfo.NumBlocks()
NumBlocks: numBlocks,
}
StartHeight += stateInfo.NumBlocks

keeper.SetStateInfo(ctx, stateInfo)
keeper.SetLatestStateInfoIndex(ctx, types.StateInfoIndex{
Expand Down Expand Up @@ -112,7 +113,8 @@ func TestStateInfoByHeightMissingStateInfo1(t *testing.T) {
k.SetStateInfo(ctx, types.StateInfo{
StateInfoIndex: types.StateInfoIndex{RollappId: rollappId, Index: 60},
StartHeight: 71,
}.WithNumBlocks(1))
NumBlocks: 1,
})
_, err := k.StateInfo(wctx, request)
errIndex := 1 + (60-1)/2 // Using binary search, the middle index is lookedup first and is missing.
require.EqualError(t, err, errorsmod.Wrapf(types.ErrNotFound,
Expand Down Expand Up @@ -140,7 +142,8 @@ func TestStateInfoByHeightErr(t *testing.T) {
response: &types.QueryGetStateInfoResponse{StateInfo: types.StateInfo{
StateInfoIndex: types.StateInfoIndex{RollappId: rollappID, Index: 4},
StartHeight: msgs[3].StartHeight,
}.WithNumBlocks(msgs[3].NumBlocks())},
NumBlocks: msgs[3].NumBlocks,
}},
},
{
desc: "StateInfoByHeight_firstBlockInBatch",
Expand All @@ -151,18 +154,20 @@ func TestStateInfoByHeightErr(t *testing.T) {
response: &types.QueryGetStateInfoResponse{StateInfo: types.StateInfo{
StateInfoIndex: types.StateInfoIndex{RollappId: rollappID, Index: 3},
StartHeight: msgs[2].StartHeight,
}.WithNumBlocks(msgs[2].NumBlocks())},
NumBlocks: msgs[2].NumBlocks,
}},
},
{
desc: "StateInfoByHeight_lastBlockInBatch",
request: &types.QueryGetStateInfoRequest{
RollappId: rollappID,
Height: msgs[2].LastHeight(),
Height: msgs[2].StartHeight + msgs[2].NumBlocks - 1,
},
response: &types.QueryGetStateInfoResponse{StateInfo: types.StateInfo{
StateInfoIndex: types.StateInfoIndex{RollappId: rollappID, Index: 3},
StartHeight: msgs[2].StartHeight,
}.WithNumBlocks(msgs[2].NumBlocks())},
NumBlocks: msgs[2].NumBlocks,
}},
},
{
desc: "StateInfoByHeight_unknownRollappId",
Expand Down Expand Up @@ -204,7 +209,7 @@ func TestStateInfoByHeightValidIncreasingBlockBatches(t *testing.T) {
msgs := createNStateInfoAndIndex(k, ctx, numOfMsg, rollappID)

for i := 0; i < numOfMsg; i += 1 {
for height := msgs[i].StartHeight; height <= msgs[i].LastHeight(); height += 1 {
for height := msgs[i].StartHeight; height < msgs[i].StartHeight+msgs[i].NumBlocks; height += 1 {
request := &types.QueryGetStateInfoRequest{
RollappId: rollappID,
Height: height,
Expand All @@ -227,7 +232,7 @@ func TestStateInfoByHeightValidDecreasingBlockBatches(t *testing.T) {
msgs := createNStateInfoAndIndex(k, ctx, numOfMsg, rollappID)

for i := 0; i < numOfMsg; i += 1 {
for height := msgs[i].StartHeight; height < msgs[i].LastHeight(); height += 1 {
for height := msgs[i].StartHeight; height < msgs[i].StartHeight+msgs[i].NumBlocks; height += 1 {
request := &types.QueryGetStateInfoRequest{
RollappId: rollappID,
Height: height,
Expand Down
9 changes: 6 additions & 3 deletions x/rollapp/keeper/grpc_query_state_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,18 @@ func TestFindStateInfoByHeight(t *testing.T) {
keeper.SetStateInfo(ctx, types.StateInfo{
StateInfoIndex: types.StateInfoIndex{RollappId: rollappID, Index: 1},
StartHeight: 1,
}.WithNumBlocks(2))
NumBlocks: 2,
})
keeper.SetStateInfo(ctx, types.StateInfo{
StateInfoIndex: types.StateInfoIndex{RollappId: rollappID, Index: 2},
StartHeight: 3,
}.WithNumBlocks(3))
NumBlocks: 3,
})
keeper.SetStateInfo(ctx, types.StateInfo{
StateInfoIndex: types.StateInfoIndex{RollappId: rollappID, Index: 3},
StartHeight: 6,
}.WithNumBlocks(4))
NumBlocks: 4,
})
keeper.SetLatestStateInfoIndex(ctx, types.StateInfoIndex{
RollappId: rollappID,
Index: 3,
Expand Down
3 changes: 2 additions & 1 deletion x/rollapp/keeper/msg_server_update_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (k msgServer) UpdateState(goCtx context.Context, msg *types.MsgUpdateState)
}

// check to see if received height is the one we expected
expectedStartHeight := stateInfo.LastHeight() + 1
expectedStartHeight := stateInfo.StartHeight + stateInfo.NumBlocks
if expectedStartHeight != msg.StartHeight {
return nil, errorsmod.Wrapf(types.ErrWrongBlockHeight,
"expected height (%d), but received (%d)",
Expand Down Expand Up @@ -102,6 +102,7 @@ func (k msgServer) UpdateState(goCtx context.Context, msg *types.MsgUpdateState)
newIndex,
msg.Creator,
msg.StartHeight,
msg.NumBlocks,
msg.DAPath,
creationHeight,
msg.BDs,
Expand Down
8 changes: 5 additions & 3 deletions x/rollapp/keeper/msg_server_update_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (suite *RollappTestSuite) TestUpdateState() {
}, "finalization queue", "i", i)

// update state
_, err := suite.PostStateUpdate(suite.Ctx, rollappId, proposer, expectedStateInfo.LastHeight()+1, uint64(2))
_, err := suite.PostStateUpdate(suite.Ctx, rollappId, proposer, expectedStateInfo.StartHeight+expectedStateInfo.NumBlocks, uint64(2))
suite.Require().Nil(err)

// end block
Expand Down Expand Up @@ -174,10 +174,10 @@ func (suite *RollappTestSuite) TestUpdateStateSequencerRollappMismatch() {
suite.SetupTest()

rollappId, _ := suite.CreateDefaultRollappAndProposer()
_, seq2 := suite.CreateDefaultRollappAndProposer()
_, seq_2 := suite.CreateDefaultRollappAndProposer()

// update state from proposer of rollapp2
_, err := suite.PostStateUpdate(suite.Ctx, rollappId, seq2, 1, uint64(3))
_, err := suite.PostStateUpdate(suite.Ctx, rollappId, seq_2, 1, uint64(3))
suite.ErrorIs(err, sequencertypes.ErrNotActiveSequencer)
}

Expand Down Expand Up @@ -237,6 +237,7 @@ func (suite *RollappTestSuite) TestUpdateStateErrWrongBlockHeight() {
StateInfoIndex: types.StateInfoIndex{RollappId: rollappId, Index: 1},
Sequencer: proposer,
StartHeight: 1,
NumBlocks: 3,
Status: common.Status_PENDING,
BDs: types.BlockDescriptors{BD: []types.BlockDescriptor{{Height: 1}, {Height: 2}, {Height: 3}}},
}
Expand Down Expand Up @@ -298,6 +299,7 @@ func (suite *RollappTestSuite) TestUpdateStateDowngradeTimestamp() {
StateInfoIndex: types.StateInfoIndex{RollappId: rollappId, Index: 1},
Sequencer: proposer,
StartHeight: 1,
NumBlocks: 1,
DAPath: "",
BDs: types.BlockDescriptors{BD: []types.BlockDescriptor{{Height: 1}}},
}
Expand Down
22 changes: 6 additions & 16 deletions x/rollapp/types/state_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"

common "github.com/dymensionxyz/dymension/v3/x/common/types"
)

Expand All @@ -13,6 +14,7 @@ func NewStateInfo(
newIndex uint64,
creator string,
startHeight uint64,
numBlocks uint64,
daPath string,
height uint64,
BDs BlockDescriptors,
Expand All @@ -25,6 +27,7 @@ func NewStateInfo(
StateInfoIndex: stateInfoIndex,
Sequencer: creator,
StartHeight: startHeight,
NumBlocks: numBlocks,
DAPath: daPath,
CreationHeight: height,
Status: status,
Expand All @@ -38,18 +41,12 @@ func (s *StateInfo) Finalize() {
s.Status = common.Status_FINALIZED
}

// WithNumBlocks is intended as utility in tests if a certain number of blocks need to be made, but the content is unimportant
func (s StateInfo) WithNumBlocks(n uint64) StateInfo {
s.BDs = BlockDescriptors{BD: make([]BlockDescriptor, n)}
return s
}

func (s *StateInfo) GetIndex() StateInfoIndex {
return s.StateInfoIndex
}

func (s *StateInfo) GetLatestHeight() uint64 {
return s.StartHeight + s.NumBlocks() - 1
return s.StartHeight + s.NumBlocks - 1
}

func (s *StateInfo) ContainsHeight(height uint64) bool {
Expand All @@ -64,23 +61,16 @@ func (s *StateInfo) GetBlockDescriptor(height uint64) (BlockDescriptor, bool) {
}

func (s *StateInfo) GetLatestBlockDescriptor() BlockDescriptor {
// return s.BDs.BD[s.NumBlocks-1] // todo: should it be this? or the one below? using this breaks ibctesting tests
return s.BDs.BD[len(s.BDs.BD)-1]
}

func (s *StateInfo) LastHeight() uint64 {
return s.StartHeight + s.NumBlocks() - 1
}

func (s *StateInfo) NumBlocks() uint64 {
return uint64(len(s.BDs.BD))
}

func (s *StateInfo) GetEvents() []sdk.Attribute {
eventAttributes := []sdk.Attribute{
sdk.NewAttribute(AttributeKeyRollappId, s.StateInfoIndex.RollappId),
sdk.NewAttribute(AttributeKeyStateInfoIndex, strconv.FormatUint(s.StateInfoIndex.Index, 10)),
sdk.NewAttribute(AttributeKeyStartHeight, strconv.FormatUint(s.StartHeight, 10)),
sdk.NewAttribute(AttributeKeyNumBlocks, strconv.FormatUint(s.NumBlocks(), 10)),
sdk.NewAttribute(AttributeKeyNumBlocks, strconv.FormatUint(s.NumBlocks, 10)),
sdk.NewAttribute(AttributeKeyDAPath, s.DAPath),
sdk.NewAttribute(AttributeKeyStatus, s.Status.String()),
sdk.NewAttribute(AttributeKeyCreatedAt, s.CreatedAt.Format(time.RFC3339)),
Expand Down
Loading

0 comments on commit 96d7ac7

Please sign in to comment.