Skip to content

Commit

Permalink
feat(rollapp): migrate drs from commit string to integer (#1362)
Browse files Browse the repository at this point in the history
  • Loading branch information
srene authored Oct 31, 2024
1 parent 084f756 commit ca94133
Show file tree
Hide file tree
Showing 15 changed files with 376 additions and 277 deletions.
4 changes: 2 additions & 2 deletions app/apptesting/test_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ func (s *KeeperTestHelper) CreateSequencerByPubkey(ctx sdk.Context, rollappId st
}

func (s *KeeperTestHelper) PostStateUpdate(ctx sdk.Context, rollappId, seqAddr string, startHeight, numOfBlocks uint64) (lastHeight uint64, err error) {
return s.PostStateUpdateWithDRSVersion(ctx, rollappId, seqAddr, startHeight, numOfBlocks, "")
return s.PostStateUpdateWithDRSVersion(ctx, rollappId, seqAddr, startHeight, numOfBlocks, 1)
}

func (s *KeeperTestHelper) PostStateUpdateWithDRSVersion(ctx sdk.Context, rollappId, seqAddr string, startHeight, numOfBlocks uint64, drsVersion string) (lastHeight uint64, err error) {
func (s *KeeperTestHelper) PostStateUpdateWithDRSVersion(ctx sdk.Context, rollappId, seqAddr string, startHeight, numOfBlocks uint64, drsVersion uint32) (lastHeight uint64, err error) {
var bds rollapptypes.BlockDescriptors
bds.BD = make([]rollapptypes.BlockDescriptor, numOfBlocks)
for k := uint64(0); k < numOfBlocks; k++ {
Expand Down
7 changes: 4 additions & 3 deletions ibctesting/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,10 @@ func (s *utilSuite) updateRollappState(endHeight uint64) {
blockDescriptors := &rollapptypes.BlockDescriptors{BD: make([]rollapptypes.BlockDescriptor, numBlocks)}
for i := uint64(0); i < numBlocks; i++ {
blockDescriptors.BD[i] = rollapptypes.BlockDescriptor{
Height: startHeight + i,
StateRoot: bytes.Repeat([]byte{byte(startHeight) + byte(i)}, 32),
Timestamp: time.Now().UTC(),
Height: startHeight + i,
StateRoot: bytes.Repeat([]byte{byte(startHeight) + byte(i)}, 32),
Timestamp: time.Now().UTC(),
DrsVersion: 1,
}
}
// Update the state
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ message BlockDescriptor {
// timestamp is the time from the block header
google.protobuf.Timestamp timestamp = 3 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
// DrsVersion is a DRS version used by the rollapp.
string drs_version = 4;
uint32 drs_version = 4;
}

// BlockDescriptors defines list of BlockDescriptor.
Expand Down
2 changes: 1 addition & 1 deletion proto/dymensionxyz/dymension/rollapp/events.proto
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ message EventMarkVulnerableRollapps {
// VulnerableRollappNum is a number of rollapps that were marked as vulnerable.
uint64 vulnerable_rollapp_num = 1;
// DrsVersions is a list of DRS versions that were marked as vulnerable.
repeated string drs_versions = 2;
repeated uint32 drs_versions = 2;
}
2 changes: 1 addition & 1 deletion proto/dymensionxyz/dymension/rollapp/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ message MsgMarkVulnerableRollapps {
// Authority is the authority address.
string authority = 1;
// DrsVersions is a list of DRS versions that will be marked vulnerable.
repeated string drs_versions = 2;
repeated uint32 drs_versions = 2;
}

message MsgMarkVulnerableRollappsResponse {}
4 changes: 2 additions & 2 deletions x/rollapp/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type Keeper struct {
sequencerKeeper SequencerKeeper
bankKeeper BankKeeper

vulnerableDRSVersions collections.KeySet[string]
vulnerableDRSVersions collections.KeySet[uint32]
registeredRollappDenoms collections.KeySet[collections.Pair[string, string]]

finalizePending func(ctx sdk.Context, stateInfoIndex types.StateInfoIndex) error
Expand Down Expand Up @@ -71,7 +71,7 @@ func NewKeeper(
collections.NewSchemaBuilder(collcompat.NewKVStoreService(storeKey)),
collections.NewPrefix(types.VulnerableDRSVersionsKeyPrefix),
"vulnerable_drs_versions",
collections.StringKey,
collections.Uint32Key,
),
registeredRollappDenoms: collections.NewKeySet[collections.Pair[string, string]](
collections.NewSchemaBuilder(collcompat.NewKVStoreService(storeKey)),
Expand Down
9 changes: 2 additions & 7 deletions x/rollapp/keeper/msg_server_mark_vulnerable_rollapps.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ func (k msgServer) MarkVulnerableRollapps(goCtx context.Context, msg *types.MsgM
return &types.MsgMarkVulnerableRollappsResponse{}, nil
}

func (k Keeper) MarkVulnerableRollapps(ctx sdk.Context, drsVersions []string) (int, error) {
vulnerableVersions := make(map[string]struct{})
func (k Keeper) MarkVulnerableRollapps(ctx sdk.Context, drsVersions []uint32) (int, error) {
vulnerableVersions := make(map[uint32]struct{})
for _, v := range drsVersions {
vulnerableVersions[v] = struct{}{}
// this also saves in the state the vulnerable version
Expand All @@ -65,11 +65,6 @@ func (k Keeper) MarkVulnerableRollapps(ctx sdk.Context, drsVersions []string) (i

// check only last block descriptor DRS, since if that last is not vulnerable it means the rollapp already upgraded and is not vulnerable anymore
bd := info.BDs.BD[len(info.BDs.BD)-1]
// TODO: this check may be deleted once empty DRS version is marked vulnerable
// https://github.com/dymensionxyz/dymension/issues/1233
if bd.DrsVersion == "" {
logger.With("rollapp_id", rollapp.RollappId).Info("no DRS version set for rollapp")
}

_, vulnerable := vulnerableVersions[bd.DrsVersion]
if vulnerable {
Expand Down
76 changes: 38 additions & 38 deletions x/rollapp/keeper/msg_server_mark_vulnerable_rollapps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,90 +17,90 @@ import (
func (s *RollappTestSuite) TestMarkVulnerableRollapps() {
type rollapp struct {
name string
drsVersion string
drsVersion uint32
}
govModule := authtypes.NewModuleAddress(govtypes.ModuleName).String()

tests := []struct {
name string
authority string
rollapps []rollapp
vulnVersions []string
vulnVersions []uint32
expError error
}{
{
name: "happy path 1",
authority: govModule,
rollapps: []rollapp{
{name: "rollappa_1-1", drsVersion: "drs_non_vuln_1"},
{name: "rollappb_2-1", drsVersion: "drs_non_vuln_1"},
{name: "rollappc_3-1", drsVersion: "drs_non_vuln_2"},
{name: "rollappd_4-1", drsVersion: "drs_vuln_1"},
{name: "rollappe_5-1", drsVersion: "drs_vuln_1"},
{name: "rollappf_6-1", drsVersion: "drs_vuln_2"},
{name: "rollappa_1-1", drsVersion: 3},
{name: "rollappb_2-1", drsVersion: 3},
{name: "rollappc_3-1", drsVersion: 4},
{name: "rollappd_4-1", drsVersion: 1},
{name: "rollappe_5-1", drsVersion: 1},
{name: "rollappf_6-1", drsVersion: 2},
},
vulnVersions: []string{
"drs_vuln_1",
"drs_vuln_2",
vulnVersions: []uint32{
1,
2,
},
expError: nil,
},
{
name: "happy path 2",
authority: govModule,
rollapps: []rollapp{
{name: "rollappa_1-1", drsVersion: "drs_non_vuln_1"},
{name: "rollappd_2-1", drsVersion: "drs_vuln_1"},
{name: "rollappa_1-1", drsVersion: 2},
{name: "rollappd_2-1", drsVersion: 1},
},
vulnVersions: []string{
"drs_vuln_1",
vulnVersions: []uint32{
1,
},
expError: nil,
},
{
name: "some legacy rollapps don't have a DRS version",
authority: govModule,
rollapps: []rollapp{
{name: "rollappa_1-1", drsVersion: "drs_non_vuln_1"},
{name: "rollappb_2-1", drsVersion: ""},
{name: "rollappc_3-1", drsVersion: "drs_non_vuln_2"},
{name: "rollappd_4-1", drsVersion: "drs_vuln_1"},
{name: "rollappe_5-1", drsVersion: ""},
{name: "rollappf_6-1", drsVersion: "drs_vuln_2"},
{name: "rollappa_1-1", drsVersion: 2},
{name: "rollappb_2-1", drsVersion: 0},
{name: "rollappc_3-1", drsVersion: 3},
{name: "rollappd_4-1", drsVersion: 1},
{name: "rollappe_5-1", drsVersion: 0},
{name: "rollappf_6-1", drsVersion: 2},
},
vulnVersions: []string{
"drs_vuln_1",
"drs_vuln_2",
vulnVersions: []uint32{
1,
2,
},
expError: nil,
},
{
name: "empty DRS version is also vulnerable",
authority: govModule,
rollapps: []rollapp{
{name: "rollappa_1-1", drsVersion: "drs_non_vuln_1"},
{name: "rollappb_2-1", drsVersion: ""},
{name: "rollappc_3-1", drsVersion: "drs_non_vuln_2"},
{name: "rollappd_4-1", drsVersion: "drs_vuln_1"},
{name: "rollappe_5-1", drsVersion: ""},
{name: "rollappf_6-1", drsVersion: "drs_vuln_2"},
{name: "rollappa_1-1", drsVersion: 3},
{name: "rollappb_2-1", drsVersion: 0},
{name: "rollappc_3-1", drsVersion: 4},
{name: "rollappd_4-1", drsVersion: 1},
{name: "rollappe_5-1", drsVersion: 0},
{name: "rollappf_6-1", drsVersion: 1},
},
vulnVersions: []string{
"",
"drs_vuln_1",
"drs_vuln_2",
vulnVersions: []uint32{
0,
1,
2,
},
expError: nil,
},
{
name: "invalid authority",
authority: apptesting.CreateRandomAccounts(1)[0].String(),
rollapps: []rollapp{
{name: "rollappa_1-1", drsVersion: "drs_non_vuln_1"},
{name: "rollappe_2-1", drsVersion: "drs_vuln_1"},
{name: "rollappa_1-1", drsVersion: 2},
{name: "rollappe_2-1", drsVersion: 1},
},
vulnVersions: []string{
"drs_vuln_1",
vulnVersions: []uint32{
1,
},
expError: gerrc.ErrInvalidArgument,
},
Expand Down
6 changes: 3 additions & 3 deletions x/rollapp/keeper/msg_server_update_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ func (suite *RollappTestSuite) TestUpdateState() {
func (s *RollappTestSuite) TestUpdateStateVulnerableRollapp() {
const (
raName = "rollapptest_1-1"
nonVulnerableVersion = "non_vulnerable_version"
vulnerableVersion = "vulnerable_version"
nonVulnerableVersion = 2
vulnerableVersion = 1
)

// create a rollapp
Expand All @@ -119,7 +119,7 @@ func (s *RollappTestSuite) TestUpdateStateVulnerableRollapp() {
s.Require().Equal(expectedLastHeight, actualLastHeight)

// mark a DRS version as vulnerable. note that the last state update of the rollapp wasn't vulnerable
vulnNum, err := s.App.RollappKeeper.MarkVulnerableRollapps(s.Ctx, []string{vulnerableVersion})
vulnNum, err := s.App.RollappKeeper.MarkVulnerableRollapps(s.Ctx, []uint32{vulnerableVersion})
s.Require().NoError(err)
s.Require().Equal(0, vulnNum)

Expand Down
6 changes: 3 additions & 3 deletions x/rollapp/keeper/rollapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,19 +346,19 @@ func FilterNonVulnerable(b types.Rollapp) bool {
return !b.IsVulnerable()
}

func (k Keeper) IsDRSVersionVulnerable(ctx sdk.Context, version string) bool {
func (k Keeper) IsDRSVersionVulnerable(ctx sdk.Context, version uint32) bool {
ok, err := k.vulnerableDRSVersions.Has(ctx, version)
if err != nil {
panic(fmt.Sprintf("checking if DRS version is vulnerable: %v", err))
}
return ok
}

func (k Keeper) SetVulnerableDRSVersion(ctx sdk.Context, version string) error {
func (k Keeper) SetVulnerableDRSVersion(ctx sdk.Context, version uint32) error {
return k.vulnerableDRSVersions.Set(ctx, version)
}

func (k Keeper) GetAllVulnerableDRSVersions(ctx sdk.Context) ([]string, error) {
func (k Keeper) GetAllVulnerableDRSVersions(ctx sdk.Context) ([]uint32, error) {
iter, err := k.vulnerableDRSVersions.Iterate(ctx, nil)
if err != nil {
return nil, err
Expand Down
Loading

0 comments on commit ca94133

Please sign in to comment.