Skip to content

Commit

Permalink
ensure every score blockheight is built with nonce not current height (
Browse files Browse the repository at this point in the history
…#610)

<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v           ✰  Thanks for creating a PR! You're awesome! ✰
v Please note that maintainers will only review those PRs with a
completed PR template.
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Purpose of Changes and their Description

Scores were inconsistently using current height or nonce. I modified to
just use nonces.

I used this to also modify how we were checking for idempotent
submissions (1 submission per actor type per topic per epoch).

This is a simpler solution than
#609

## Link(s) to Ticket(s) or Issue(s) resolved by this PR

[From
conversation](#609)
with @xmariachi and @guilherme-brandao

## Are these changes tested and documented?

- [ ] If tested, please describe how. If not, why tests are not needed.
- [ ] If documented, please describe where. If not, describe why docs
are not needed.
- [ ] Added to `Unreleased` section of `CHANGELOG.md`?

## Still Left Todo

*Fill this out if this is a Draft PR so others can help.*
  • Loading branch information
kpeluso authored Sep 19, 2024
1 parent fcdc125 commit 7223eb4
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Security

* [#588](https://github.com/allora-network/allora-chain/pull/588) Add String Max Length Module Parameter, Enforce Max String Length on Creation of New Topics
* [#610](https://github.com/allora-network/allora-chain/pull/610) Ensure that BlockHeight of scores is consistently set to be a topic epoch nonce. Also simplify the condition for idempotent payload submission.

## v0.4.0

Expand Down
24 changes: 9 additions & 15 deletions x/emissions/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,6 @@ func (k *Keeper) GetForecastsAtBlock(ctx context.Context, topicId TopicId, block
func (k *Keeper) AppendInference(
ctx sdk.Context,
topic types.Topic,
blockHeight BlockHeight,
nonceBlockHeight BlockHeight,
inference *types.Inference,
) error {
Expand Down Expand Up @@ -825,8 +824,7 @@ func (k *Keeper) AppendInference(
return errorsmod.Wrapf(err, "Error getting inferer score ema")
}
// Only calc and save if there's a new update
if previousEmaScore.BlockHeight != 0 &&
blockHeight-previousEmaScore.BlockHeight <= topic.WorkerSubmissionWindow {
if previousEmaScore.BlockHeight >= nonceBlockHeight {
return types.ErrCantUpdateEmaMoreThanOncePerWindow
}

Expand All @@ -848,7 +846,7 @@ func (k *Keeper) AppendInference(
err = k.CalcAndSaveInfererScoreEmaWithLastSavedTopicQuantile(
ctx,
topic,
blockHeight,
nonceBlockHeight,
inferences.Inferences[lowScoreIndex].Inferer,
)
if err != nil {
Expand All @@ -861,7 +859,7 @@ func (k *Keeper) AppendInference(
return k.InsertInferences(ctx, topic.Id, nonceBlockHeight, inferences)
} else {
// Update EMA score for the current inferer, who is the lowest score inferer
err = k.CalcAndSaveInfererScoreEmaWithLastSavedTopicQuantile(ctx, topic, blockHeight, inference.Inferer)
err = k.CalcAndSaveInfererScoreEmaWithLastSavedTopicQuantile(ctx, topic, nonceBlockHeight, inference.Inferer)
if err != nil {
return err
}
Expand Down Expand Up @@ -891,7 +889,6 @@ func (k *Keeper) InsertInferences(
func (k *Keeper) AppendForecast(
ctx sdk.Context,
topic types.Topic,
blockHeight BlockHeight,
nonceBlockHeight BlockHeight,
forecast *types.Forecast,
) error {
Expand Down Expand Up @@ -923,8 +920,7 @@ func (k *Keeper) AppendForecast(
return errorsmod.Wrapf(err, "Error getting forecaster score ema")
}
// Only calc and save if there's a new update
if previousEmaScore.BlockHeight != 0 &&
blockHeight-previousEmaScore.BlockHeight <= topic.WorkerSubmissionWindow {
if previousEmaScore.BlockHeight >= nonceBlockHeight {
return types.ErrCantUpdateEmaMoreThanOncePerWindow
}

Expand All @@ -946,7 +942,7 @@ func (k *Keeper) AppendForecast(
err = k.CalcAndSaveForecasterScoreEmaWithLastSavedTopicQuantile(
ctx,
topic,
blockHeight,
nonceBlockHeight,
forecasts.Forecasts[lowScoreIndex].Forecaster,
)
if err != nil {
Expand All @@ -959,7 +955,7 @@ func (k *Keeper) AppendForecast(
return k.InsertForecasts(ctx, topic.Id, nonceBlockHeight, forecasts)
} else {
// Update EMA score for the current forecaster, who is the lowest score forecaster
err = k.CalcAndSaveForecasterScoreEmaWithLastSavedTopicQuantile(ctx, topic, blockHeight, forecast.Forecaster)
err = k.CalcAndSaveForecasterScoreEmaWithLastSavedTopicQuantile(ctx, topic, nonceBlockHeight, forecast.Forecaster)
if err != nil {
return err
}
Expand Down Expand Up @@ -1019,7 +1015,6 @@ func (k *Keeper) DeleteTopicRewardNonce(ctx context.Context, topicId TopicId) er
func (k *Keeper) AppendReputerLoss(
ctx sdk.Context,
topic types.Topic,
blockHeight BlockHeight,
nonceBlockHeight BlockHeight,
reputerLoss *types.ReputerValueBundle,
) error {
Expand Down Expand Up @@ -1054,8 +1049,7 @@ func (k *Keeper) AppendReputerLoss(
return err
}
// Only calc and save if there's a new update
if previousEmaScore.BlockHeight != 0 &&
blockHeight-previousEmaScore.BlockHeight <= topic.EpochLength {
if previousEmaScore.BlockHeight >= nonceBlockHeight {
return types.ErrCantUpdateEmaMoreThanOncePerWindow
}

Expand All @@ -1077,7 +1071,7 @@ func (k *Keeper) AppendReputerLoss(
err = k.CalcAndSaveReputerScoreEmaWithLastSavedTopicQuantile(
ctx,
topic,
blockHeight,
nonceBlockHeight,
reputerLossBundles.ReputerValueBundles[lowScoreIndex].ValueBundle.Reputer,
)
if err != nil {
Expand All @@ -1091,7 +1085,7 @@ func (k *Keeper) AppendReputerLoss(
return k.InsertReputerLossBundlesAtBlock(ctx, topic.Id, nonceBlockHeight, reputerLossBundles)
} else {
// Update EMA score for the current reputer, who is the lowest score reputer
err = k.CalcAndSaveReputerScoreEmaWithLastSavedTopicQuantile(ctx, topic, blockHeight, reputerLoss.ValueBundle.Reputer)
err = k.CalcAndSaveReputerScoreEmaWithLastSavedTopicQuantile(ctx, topic, nonceBlockHeight, reputerLoss.ValueBundle.Reputer)
if err != nil {
return err
}
Expand Down
19 changes: 8 additions & 11 deletions x/emissions/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3497,7 +3497,7 @@ func (s *KeeperTestSuite) TestAppendInference() {
newInference := types.Inference{
TopicId: topicId, BlockHeight: blockHeightInferences, Inferer: worker4, Value: alloraMath.MustNewDecFromString("0.52"),
}
err = k.AppendInference(ctx, topic, blockHeightInferences, nonce.BlockHeight, &newInference)
err = k.AppendInference(ctx, topic, nonce.BlockHeight, &newInference)
s.Require().NoError(err)
newAllInferences, err := k.GetInferencesAtBlock(ctx, topicId, nonce.BlockHeight)
s.Require().NoError(err)
Expand All @@ -3516,7 +3516,7 @@ func (s *KeeperTestSuite) TestAppendInference() {
}
worker5OgScore, err := k.GetInfererScoreEma(ctx, topicId, worker5)
s.Require().NoError(err)
err = k.AppendInference(ctx, topic, blockHeightInferences, nonce.BlockHeight, &newInference2)
err = k.AppendInference(ctx, topic, nonce.BlockHeight, &newInference2)
s.Require().NoError(err)
newAllInferences, err = k.GetInferencesAtBlock(ctx, topicId, nonce.BlockHeight)
s.Require().NoError(err)
Expand Down Expand Up @@ -3549,14 +3549,14 @@ func (s *KeeperTestSuite) TestAppendInference() {
s.Require().Greater(updatedWorker2ScoreVal, ogWorker2ScoreVal, "worker2 score should go up given large ema value")
s.Require().Greater(updatedWorker2ScoreVal, worker5OgScoreVal, "worker2 could not overtake worker5, but not in this epoch")
// EMA score should be updated with the new time of update given that it was updated then
s.Require().Equal(blockHeightInferences, updatedWorker2Score.BlockHeight)
s.Require().Equal(nonce.BlockHeight, updatedWorker2Score.BlockHeight)

// Ensure passive set participant can't update their score within the same epoch
blockHeightInferences = blockHeightInferences + 1 // within the same epoch => no update
newInference2 = types.Inference{
TopicId: topicId, BlockHeight: blockHeightInferences, Inferer: worker2, Value: alloraMath.MustNewDecFromString("0.52"),
}
err = k.AppendInference(ctx, topic, blockHeightInferences, nonce.BlockHeight, &newInference2)
err = k.AppendInference(ctx, topic, nonce.BlockHeight, &newInference2)
s.Require().Error(err, types.ErrCantUpdateEmaMoreThanOncePerWindow.Error())
// Confirm no change in EMA score
newAllInferences, err = k.GetInferencesAtBlock(ctx, topicId, nonce.BlockHeight)
Expand Down Expand Up @@ -3668,7 +3668,7 @@ func (s *KeeperTestSuite) TestAppendForecast() {
topic, err := k.GetTopic(ctx, topicId)
s.Require().NoError(err)
blockHeightInferences = blockHeightInferences + topic.EpochLength
err = k.AppendForecast(ctx, topic, blockHeightInferences, nonce.BlockHeight, &newForecast)
err = k.AppendForecast(ctx, topic, nonce.BlockHeight, &newForecast)
s.Require().NoError(err)
newAllForecasts, err := k.GetForecastsAtBlock(ctx, topicId, nonce.BlockHeight)
s.Require().NoError(err)
Expand All @@ -3693,8 +3693,7 @@ func (s *KeeperTestSuite) TestAppendForecast() {
},
},
}
blockHeightInferences = blockHeightInferences + topic.EpochLength
err = k.AppendForecast(ctx, topic, blockHeightInferences, nonce.BlockHeight, &newInference2)
err = k.AppendForecast(ctx, topic, nonce.BlockHeight, &newInference2)
s.Require().NoError(err)
newAllForecasts, err = k.GetForecastsAtBlock(ctx, topicId, nonce.BlockHeight)
s.Require().NoError(err)
Expand Down Expand Up @@ -3775,8 +3774,7 @@ func (s *KeeperTestSuite) TestAppendReputerLoss() {
}
topic, err := k.GetTopic(ctx, topicId)
s.Require().NoError(err)
blockHeight = blockHeight + topic.EpochLength
err = k.AppendReputerLoss(ctx, topic, blockHeight, nonce.BlockHeight, &newReputerLoss)
err = k.AppendReputerLoss(ctx, topic, nonce.BlockHeight, &newReputerLoss)
s.Require().NoError(err)
newAllReputerLosses, err := k.GetReputerLossBundlesAtBlock(ctx, topicId, nonce.BlockHeight)
s.Require().NoError(err)
Expand All @@ -3795,8 +3793,7 @@ func (s *KeeperTestSuite) TestAppendReputerLoss() {
TopicId: topicId,
},
}
blockHeight = blockHeight + topic.EpochLength
err = k.AppendReputerLoss(ctx, topic, blockHeight, nonce.BlockHeight, &newReputerLoss2)
err = k.AppendReputerLoss(ctx, topic, nonce.BlockHeight, &newReputerLoss2)
s.Require().NoError(err)
newAllReputerLosses, err = k.GetReputerLossBundlesAtBlock(ctx, topicId, nonce.BlockHeight)
s.Require().NoError(err)
Expand Down
2 changes: 1 addition & 1 deletion x/emissions/keeper/msgserver/msg_server_reputer_payload.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (ms msgServer) InsertReputerPayload(ctx context.Context, msg *types.InsertR
}

sdkCtx := sdk.UnwrapSDKContext(ctx)
err = ms.k.AppendReputerLoss(sdkCtx, topic, blockHeight, nonce.ReputerNonce.BlockHeight, msg.ReputerValueBundle)
err = ms.k.AppendReputerLoss(sdkCtx, topic, nonce.ReputerNonce.BlockHeight, msg.ReputerValueBundle)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions x/emissions/keeper/msgserver/msg_server_worker_payload.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (ms msgServer) InsertWorkerPayload(ctx context.Context, msg *types.InsertWo
"inferer not using the same topic as bundle")
}

err = ms.k.AppendInference(sdkCtx, topic, blockHeight, nonce.BlockHeight, inference)
err = ms.k.AppendInference(sdkCtx, topic, nonce.BlockHeight, inference)
if err != nil {
return nil, errorsmod.Wrapf(err, "Error appending inference")
}
Expand Down Expand Up @@ -148,7 +148,7 @@ func (ms msgServer) InsertWorkerPayload(ctx context.Context, msg *types.InsertWo

if len(acceptedForecastElements) > 0 {
forecast.ForecastElements = acceptedForecastElements
err = ms.k.AppendForecast(sdkCtx, topic, blockHeight, nonce.BlockHeight, forecast)
err = ms.k.AppendForecast(sdkCtx, topic, nonce.BlockHeight, forecast)
if err != nil {
return nil, errorsmod.Wrapf(err,
"Error appending forecast")
Expand Down

0 comments on commit 7223eb4

Please sign in to comment.