Skip to content

Commit

Permalink
0.4.0 Migration Bug Fixes and Additional Unit Tests (#574)
Browse files Browse the repository at this point in the history 
## Purpose of Changes and their Description

Fix two bugs in the migration.go code for 0.4.0 and also adds unit tests
to actually cover the code that handles active topic migration and
keeping active topics active

## Are these changes tested and documented?

This PR adds unit tests in migration_test.go



**Note**: this was built off an already-squashed PR and so there are a
bunch of commits that you can ignore, they'll be squashed again in this
PR

---------

Co-authored-by: Kenny <[email protected]>
Co-authored-by: michael <[email protected]>
Co-authored-by: Kenny P <[email protected]>
Co-authored-by: Guilherme Brandão <[email protected]>
  • Loading branch information
@relyt29 @kpeluso
@RedBird96 @guilherme-brandao
5 people authored Sep 3, 2024
1 parent ab4b3ab commit b7252e0
Show file tree
Hide file tree
Showing 3 changed files with 287 additions and 27 deletions.
62 changes: 35 additions & 27 deletions x/emissions/migrations/v3/migrate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,9 @@ func MigrateTopics(
if err != nil {
return errorsmod.Wrapf(err, "failed to get params for active topic migration")
}
churningBlock := make(map[types.TopicId]types.BlockHeight, 0)
blockToActiveTopics := make(map[types.BlockHeight]types.TopicIds, 0)
lowestWeight := make(map[types.BlockHeight]types.TopicIdWeightPair, 0)

churningBlock := make(map[types.TopicId]types.BlockHeight, 0)
topicWeightData := make(map[types.TopicId]alloraMath.Dec, 0)

topicsToChange := make(map[string]types.Topic, 0)
Expand Down Expand Up @@ -192,6 +191,29 @@ func MigrateTopics(
continue
}

activeTopicIds := blockToActiveTopics[blockHeight]
activeTopicIds.TopicIds = append(activeTopicIds.TopicIds, oldMsg.Id)
// If number of active topic is over global param then remove lowest topic
if uint64(len(blockToActiveTopics[blockHeight].TopicIds)) >= params.MaxActiveTopicsPerBlock {
// If current weight is lower than lowest then skip
// Otherwise upgrade lowest weight
if weight.Lt(lowestWeight[blockHeight].Weight) {
continue
} else {
newActiveTopicIds := []types.TopicId{}
for i, id := range activeTopicIds.TopicIds {
if id == lowestWeight[blockHeight].TopicId {
delete(churningBlock, id)
newActiveTopicIds = append(activeTopicIds.TopicIds[:i],
activeTopicIds.TopicIds[i+1:]...)
break
}
}
activeTopicIds.TopicIds = newActiveTopicIds
lowestWeight[blockHeight] = getLowestTopicIdWeightPair(topicWeightData, activeTopicIds)
}
}
churningBlock[oldMsg.Id] = blockHeight
cuLowestWeight := lowestWeight[blockHeight]
// Update lowest weight of topic per block
if cuLowestWeight.Weight.Equal(alloraMath.ZeroDec()) ||
Expand All @@ -200,37 +222,14 @@ func MigrateTopics(
Weight: weight,
TopicId: oldMsg.Id,
}
lowestWeight[blockHeight] = cuLowestWeight
}

churningBlock[oldMsg.Id] = blockHeight

activeTopicIds := blockToActiveTopics[blockHeight]
activeTopicIds.TopicIds = append(activeTopicIds.TopicIds, oldMsg.Id)

// If number of active topic is over global param then remove lowest topic
if uint64(len(blockToActiveTopics[blockHeight].TopicIds)) > params.MaxActiveTopicsPerBlock {
// Remove from topicToNextPossibleChurningBlock
delete(churningBlock, lowestWeight[blockHeight].TopicId)
newActiveTopicIds := []types.TopicId{}
for i, id := range blockToActiveTopics[blockHeight].TopicIds {
if id == lowestWeight[blockHeight].TopicId {
newActiveTopicIds = append(blockToActiveTopics[blockHeight].TopicIds[:i],
blockToActiveTopics[blockHeight].TopicIds[i+1:]...)
break
}
}
// Reset active topics per block
activeTopicIds.TopicIds = newActiveTopicIds
//blockToActiveTopics[blockHeight] = types.TopicIds{TopicIds: newActiveTopicIds}
// Reset lowest weight per block
cuLowestWeight = getLowestTopicIdWeightPair(topicWeightData, blockToActiveTopics[blockHeight])
}
blockToActiveTopics[blockHeight] = activeTopicIds
blockHeightBytes, err := collections.Int64Value.Encode(blockHeight)
if err != nil {
return err
}
churningBlockStore.Set(idArray, blockHeightBytes)
activeTopicsBytes, err := activeTopicIds.Marshal()
if err != nil {
return err
Expand All @@ -245,6 +244,15 @@ func MigrateTopics(
topicsToChange[string(iterator.Key())] = getNewTopic(oldMsg)
}
_ = iterator.Close()
for key, value := range churningBlock {
blockHeightBytes, err := collections.Int64Value.Encode(value)
if err != nil {
return err
}
idArray := make([]byte, 8)
binary.BigEndian.PutUint64(idArray, key)
churningBlockStore.Set(idArray, blockHeightBytes)
}
for key, value := range topicsToChange {
topicStore.Set([]byte(key), cdc.MustMarshal(&value))
}
Expand All @@ -264,7 +272,7 @@ func getNewTopic(oldMsg oldtypes.Topic) types.Topic {
PNorm: oldMsg.PNorm,
AlphaRegret: oldMsg.AlphaRegret,
AllowNegative: oldMsg.AllowNegative,
Epsilon: alloraMath.MustNewDecFromString("0.01"),
Epsilon: oldMsg.Epsilon,
// InitialRegret is being reset to account for NaNs that were previously stored due to insufficient validation
InitialRegret: alloraMath.MustNewDecFromString("0"),
WorkerSubmissionWindow: oldMsg.WorkerSubmissionWindow,
Expand Down
251 changes: 251 additions & 0 deletions x/emissions/migrations/v3/migrate_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package v3_test

import (
"strconv"
"testing"

alloraMath "github.com/allora-network/allora-chain/math"

codecAddress "github.com/cosmos/cosmos-sdk/codec/address"

"cosmossdk.io/core/store"
cosmosMath "cosmossdk.io/math"
"github.com/allora-network/allora-chain/app/params"

"cosmossdk.io/store/prefix"
Expand Down Expand Up @@ -236,6 +238,255 @@ func (s *EmissionsV3MigrationTestSuite) TestMigrateTopics() {
s.Require().Equal("0", newMsg.InitialRegret.String())
}

func (s *EmissionsV3MigrationTestSuite) TestMigrateTopicsWithWeightSameEpoch() {
store := runtime.KVStoreAdapter(s.storeService.OpenKVStore(s.ctx))
cdc := s.emissionsKeeper.GetBinaryCodec()

oldTopics := []oldtypes.Topic{
{
Id: 1,
Creator: "creator",
Metadata: "metadata",
LossMethod: "lossmethod",
EpochLastEnded: 0,
EpochLength: 100,
GroundTruthLag: 10,
PNorm: alloraMath.NewDecFromInt64(3),
AlphaRegret: alloraMath.MustNewDecFromString("0.1"),
AllowNegative: false,
Epsilon: alloraMath.MustNewDecFromString("0.01"),
// InitialRegret is being reset to account for NaNs that were previously stored due to insufficient validation
InitialRegret: alloraMath.MustNewDecFromString("11"),
WorkerSubmissionWindow: 120,
},
{
Id: 2,
Creator: "creator2",
Metadata: "metadata2",
LossMethod: "lossmethod2",
EpochLastEnded: 0,
EpochLength: 100,
GroundTruthLag: 20,
PNorm: alloraMath.NewDecFromInt64(3),
AlphaRegret: alloraMath.MustNewDecFromString("0.1"),
AllowNegative: false,
Epsilon: alloraMath.MustNewDecFromString("0.01"),
InitialRegret: alloraMath.MustNewDecFromString("11"),
WorkerSubmissionWindow: 120,
},
{
Id: 3,
Creator: "creator3",
Metadata: "metadata3",
LossMethod: "lossmethod3",
EpochLastEnded: 0,
EpochLength: 100,
GroundTruthLag: 30,
PNorm: alloraMath.NewDecFromInt64(3),
AlphaRegret: alloraMath.MustNewDecFromString("0.1"),
AllowNegative: false,
Epsilon: alloraMath.MustNewDecFromString("0.01"),
InitialRegret: alloraMath.MustNewDecFromString("11"),
WorkerSubmissionWindow: 130,
},
}
err := s.emissionsKeeper.AddTopicFeeRevenue(s.ctx, 1, cosmosMath.NewInt(40000))
s.Require().NoError(err)
err = s.emissionsKeeper.AddTopicFeeRevenue(s.ctx, 2, cosmosMath.NewInt(70000))
s.Require().NoError(err)
err = s.emissionsKeeper.AddTopicFeeRevenue(s.ctx, 3, cosmosMath.NewInt(60000))
s.Require().NoError(err)

err = s.emissionsKeeper.SetTopicStake(s.ctx, 1, cosmosMath.NewInt(40000))
s.Require().NoError(err)
err = s.emissionsKeeper.SetTopicStake(s.ctx, 2, cosmosMath.NewInt(70000))
s.Require().NoError(err)
err = s.emissionsKeeper.SetTopicStake(s.ctx, 3, cosmosMath.NewInt(60000))
s.Require().NoError(err)

topicStore := prefix.NewStore(store, types.TopicsKey)
for i, oldTopic := range oldTopics {
bz, err := proto.Marshal(&oldTopic)
s.Require().NoError(err)

topicStore.Set([]byte("testKey"+strconv.Itoa(i+1)), bz)
}

err = v3.MigrateTopics(s.ctx, store, cdc, *s.emissionsKeeper)
s.Require().NoError(err)

// Verify the store has been updated correctly
iterator := topicStore.Iterator(nil, nil)
s.Require().True(iterator.Valid())
defer iterator.Close()

// this is from topic.BlockHeightEnded + topic.EpochLength
blockHeightEnded := int64(100)

churningBlock, inFuture, err := s.emissionsKeeper.GetNextPossibleChurningBlockByTopicId(s.ctx, 1)
s.Require().NoError(err)
s.Require().Equal(int64(0), churningBlock)
s.Require().False(inFuture)

churningBlock, inFuture, err = s.emissionsKeeper.GetNextPossibleChurningBlockByTopicId(s.ctx, 2)
s.Require().NoError(err)
s.Require().Equal(churningBlock, blockHeightEnded)
s.Require().True(inFuture)

churningBlock, inFuture, err = s.emissionsKeeper.GetNextPossibleChurningBlockByTopicId(s.ctx, 3)
s.Require().NoError(err)
s.Require().Equal(int64(0), churningBlock)
s.Require().False(inFuture)

// not the same as feeRev * stake because weight is EMAd with 0
lowestWeight, noPrior, err := s.emissionsKeeper.GetLowestActiveTopicWeightAtBlock(s.ctx, blockHeightEnded)
s.Require().False(noPrior)
s.Require().NoError(err)
s.Require().True(lowestWeight.Weight.Gt(alloraMath.ZeroDec()))

activeTopicIds, err := s.emissionsKeeper.GetActiveTopicIdsAtBlock(s.ctx, blockHeightEnded)
s.Require().NoError(err)
s.Require().Len(activeTopicIds.TopicIds, 1)
s.Require().NotContains(activeTopicIds.TopicIds, uint64(1))
s.Require().Contains(activeTopicIds.TopicIds, uint64(2))
s.Require().NotContains(activeTopicIds.TopicIds, uint64(3))
}

func (s *EmissionsV3MigrationTestSuite) TestMigrateTopicsWithWeightDifferentEpoch() {
store := runtime.KVStoreAdapter(s.storeService.OpenKVStore(s.ctx))
cdc := s.emissionsKeeper.GetBinaryCodec()

blockHeightEnded1 := int64(100)
blockHeightEnded2 := int64(200)
blockHeightEnded3 := int64(300)

oldTopics := []oldtypes.Topic{
{
Id: 1,
Creator: "creator",
Metadata: "metadata",
LossMethod: "lossmethod",
EpochLastEnded: 0,
EpochLength: blockHeightEnded1,
GroundTruthLag: 10,
PNorm: alloraMath.NewDecFromInt64(3),
AlphaRegret: alloraMath.MustNewDecFromString("0.1"),
AllowNegative: false,
Epsilon: alloraMath.MustNewDecFromString("0.01"),
// InitialRegret is being reset to account for NaNs that were previously stored due to insufficient validation
InitialRegret: alloraMath.MustNewDecFromString("11"),
WorkerSubmissionWindow: 120,
},
{
Id: 2,
Creator: "creator2",
Metadata: "metadata2",
LossMethod: "lossmethod2",
EpochLastEnded: 0,
EpochLength: blockHeightEnded2,
GroundTruthLag: 20,
PNorm: alloraMath.NewDecFromInt64(3),
AlphaRegret: alloraMath.MustNewDecFromString("0.1"),
AllowNegative: false,
Epsilon: alloraMath.MustNewDecFromString("0.01"),
InitialRegret: alloraMath.MustNewDecFromString("11"),
WorkerSubmissionWindow: 120,
},
{
Id: 3,
Creator: "creator3",
Metadata: "metadata3",
LossMethod: "lossmethod3",
EpochLastEnded: 0,
EpochLength: blockHeightEnded3,
GroundTruthLag: 30,
PNorm: alloraMath.NewDecFromInt64(3),
AlphaRegret: alloraMath.MustNewDecFromString("0.1"),
AllowNegative: false,
Epsilon: alloraMath.MustNewDecFromString("0.01"),
InitialRegret: alloraMath.MustNewDecFromString("11"),
WorkerSubmissionWindow: 130,
},
}
err := s.emissionsKeeper.AddTopicFeeRevenue(s.ctx, 1, cosmosMath.NewInt(20000))
s.Require().NoError(err)
err = s.emissionsKeeper.AddTopicFeeRevenue(s.ctx, 2, cosmosMath.NewInt(40000))
s.Require().NoError(err)
err = s.emissionsKeeper.AddTopicFeeRevenue(s.ctx, 3, cosmosMath.NewInt(60000))
s.Require().NoError(err)

err = s.emissionsKeeper.SetTopicStake(s.ctx, 1, cosmosMath.NewInt(20000))
s.Require().NoError(err)
err = s.emissionsKeeper.SetTopicStake(s.ctx, 2, cosmosMath.NewInt(40000))
s.Require().NoError(err)
err = s.emissionsKeeper.SetTopicStake(s.ctx, 3, cosmosMath.NewInt(60000))
s.Require().NoError(err)

topicStore := prefix.NewStore(store, types.TopicsKey)
for i, oldTopic := range oldTopics {
bz, err := proto.Marshal(&oldTopic)
s.Require().NoError(err)

topicStore.Set([]byte("testKey"+strconv.Itoa(i+1)), bz)
}

err = v3.MigrateTopics(s.ctx, store, cdc, *s.emissionsKeeper)
s.Require().NoError(err)

// Verify the store has been updated correctly
iterator := topicStore.Iterator(nil, nil)
s.Require().True(iterator.Valid())
defer iterator.Close()

// this is from topic.BlockHeightEnded + topic.EpochLength

churningBlock, inFuture, err := s.emissionsKeeper.GetNextPossibleChurningBlockByTopicId(s.ctx, 1)
s.Require().NoError(err)
s.Require().Equal(churningBlock, blockHeightEnded1)
s.Require().True(inFuture)

churningBlock, inFuture, err = s.emissionsKeeper.GetNextPossibleChurningBlockByTopicId(s.ctx, 2)
s.Require().NoError(err)
s.Require().Equal(churningBlock, blockHeightEnded2)
s.Require().True(inFuture)

churningBlock, inFuture, err = s.emissionsKeeper.GetNextPossibleChurningBlockByTopicId(s.ctx, 3)
s.Require().NoError(err)
s.Require().Equal(churningBlock, blockHeightEnded3)
s.Require().True(inFuture)

// not the same as feeRev * stake because weight is EMAd with 0
lowestWeight, noPrior, err := s.emissionsKeeper.GetLowestActiveTopicWeightAtBlock(s.ctx, blockHeightEnded1)
s.Require().False(noPrior)
s.Require().NoError(err)
s.Require().True(lowestWeight.Weight.Gt(alloraMath.ZeroDec()))

lowestWeight, noPrior, err = s.emissionsKeeper.GetLowestActiveTopicWeightAtBlock(s.ctx, blockHeightEnded2)
s.Require().False(noPrior)
s.Require().NoError(err)
s.Require().True(lowestWeight.Weight.Gt(alloraMath.ZeroDec()))

lowestWeight, noPrior, err = s.emissionsKeeper.GetLowestActiveTopicWeightAtBlock(s.ctx, blockHeightEnded3)
s.Require().False(noPrior)
s.Require().NoError(err)
s.Require().True(lowestWeight.Weight.Gt(alloraMath.ZeroDec()))

activeTopicIds, err := s.emissionsKeeper.GetActiveTopicIdsAtBlock(s.ctx, blockHeightEnded1)
s.Require().NoError(err)
s.Require().Len(activeTopicIds.TopicIds, 1)
s.Require().Contains(activeTopicIds.TopicIds, uint64(1))

activeTopicIds, err = s.emissionsKeeper.GetActiveTopicIdsAtBlock(s.ctx, blockHeightEnded2)
s.Require().NoError(err)
s.Require().Len(activeTopicIds.TopicIds, 1)
s.Require().Contains(activeTopicIds.TopicIds, uint64(2))

activeTopicIds, err = s.emissionsKeeper.GetActiveTopicIdsAtBlock(s.ctx, blockHeightEnded3)
s.Require().NoError(err)
s.Require().Len(activeTopicIds.TopicIds, 1)
s.Require().Contains(activeTopicIds.TopicIds, uint64(3))
}

func (s *EmissionsV3MigrationTestSuite) TestResetMapsWithNonNumericValues() {
store := runtime.KVStoreAdapter(s.storeService.OpenKVStore(s.ctx))
cdc := s.emissionsKeeper.GetBinaryCodec()
Expand Down
1 change: 1 addition & 0 deletions x/emissions/module/rewards/topic_rewards.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ func GetAndUpdateActiveTopicWeights(
if err != nil {
return nil, alloraMath.Dec{}, cosmosMath.Int{}, errors.Wrapf(err, "failed to inactivate topic")
}
ctx.Logger().Debug(fmt.Sprintf("Topic %d inactivated at block %d", topic.Id, block))
continue
}

Expand Down

0 comments on commit b7252e0

Please sign in to comment.