Skip to content

Commit 842b5cd

Browse files
committed
nonce staking guards, iter close (#922)
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺ 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 Includes: nil guards in nonce paths, safer reputer nonce deref, prune-worker no-op consistency, missing iter.Close(). ## Are these changes tested and documented? - [x] If tested, please describe how. If not, why tests are not needed. -- unit tests modified accordingly - [ ] If documented, please describe where. If not, describe why docs are not needed. -- no need, no func change - [ ] Added to `Unreleased` section of `CHANGELOG.md`? -- no need , no func change <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Modularized the emissions keeper into focused sub-keepers and added robust nonce/staking guards to prevent nil derefs and window inconsistencies. No external message or query API changes; improves safety, clarity, and testability. - **Refactors** - Split monolithic Keeper into sub-keepers: Params, Topic, Staking, Scores, Nonce, Regrets, ReputerLoss, Weights, Banking, and ActorPenalties. - Rewired Msg/Query servers and tests to use injected sub-keepers and simple getters (e.g., GetParamsKeeper()). - Moved EMA score logic, weights, staking, topics, regrets, and nonce management into dedicated files with constructor funcs. - **Bug Fixes** - Added nil input checks and nil-safe iteration across nonce methods (Fulfill/Add/Is*/Prune) to prevent panics; PruneWorkerNonces now no-ops when empty. - Closed a missing iterator in staking (GetDelegateStakeRemovalForDelegatorReputerAndTopicId). <sup>Written for commit 903d5df. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. -->
1 parent aed8585 commit 842b5cd

3 files changed

Lines changed: 34 additions & 11 deletions

File tree

x/emissions/keeper/nonces.go

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ func (k *NonceKeeper) FulfillWorkerNonce(ctx context.Context, topicId TopicId, n
8686
if err := types.ValidateTopicId(topicId); err != nil {
8787
return false, errorsmod.Wrap(err, "error validating topic id")
8888
}
89+
if nonce == nil {
90+
return false, errors.New("nil worker nonce provided")
91+
}
8992
if err := nonce.Validate(); err != nil {
9093
return false, errorsmod.Wrap(err, "error validating nonce")
9194
}
@@ -121,6 +124,9 @@ func (k *NonceKeeper) FulfillReputerNonce(ctx context.Context, topicId TopicId,
121124
if err := types.ValidateTopicId(topicId); err != nil {
122125
return false, errorsmod.Wrap(err, "error validating topic id")
123126
}
127+
if nonce == nil {
128+
return false, errors.New("nil reputer nonce provided")
129+
}
124130
if err := nonce.Validate(); err != nil {
125131
return false, errorsmod.Wrap(err, "error validating nonce")
126132
}
@@ -131,6 +137,9 @@ func (k *NonceKeeper) FulfillReputerNonce(ctx context.Context, topicId TopicId,
131137

132138
// Check if the nonce is present in the unfulfilled nonces
133139
for i, n := range unfulfilledNonces.Nonces {
140+
if n == nil || n.ReputerNonce == nil {
141+
continue
142+
}
134143
if n.ReputerNonce.BlockHeight == nonce.BlockHeight {
135144
// Remove the nonce from the unfulfilled nonces
136145
unfulfilledNonces.Nonces = append(unfulfilledNonces.Nonces[:i], unfulfilledNonces.Nonces[i+1:]...)
@@ -172,6 +181,9 @@ func (k *NonceKeeper) IsWorkerNonceUnfulfilled(ctx context.Context, topicId Topi
172181

173182
// True if nonce is unfulfilled, false otherwise.
174183
func (k *NonceKeeper) IsReputerNonceUnfulfilled(ctx context.Context, topicId TopicId, nonce *types.Nonce) (isUnfulfilled bool, err error) {
184+
if nonce == nil {
185+
return false, errors.New("nil reputer nonce provided")
186+
}
175187
// Get the latest unfulfilled nonces
176188
unfulfilledNonces, err := k.GetUnfulfilledReputerNonces(ctx, topicId)
177189
if err != nil {
@@ -180,7 +192,7 @@ func (k *NonceKeeper) IsReputerNonceUnfulfilled(ctx context.Context, topicId Top
180192

181193
// Check if the nonce is present in the unfulfilled nonces
182194
for _, n := range unfulfilledNonces.Nonces {
183-
if n == nil {
195+
if n == nil || n.ReputerNonce == nil {
184196
continue
185197
}
186198
if n.ReputerNonce.BlockHeight == nonce.BlockHeight {
@@ -197,6 +209,9 @@ func (k *NonceKeeper) AddWorkerNonce(ctx context.Context, topicId TopicId, nonce
197209
if err := types.ValidateTopicId(topicId); err != nil {
198210
return errorsmod.Wrap(err, "error validating topic id")
199211
}
212+
if nonce == nil {
213+
return errors.New("nil worker nonce provided")
214+
}
200215
if err := nonce.Validate(); err != nil {
201216
return errorsmod.Wrap(err, "error validating nonce")
202217
}
@@ -236,20 +251,23 @@ func (k *NonceKeeper) AddReputerNonce(ctx context.Context, topicId TopicId, nonc
236251
if err := types.ValidateTopicId(topicId); err != nil {
237252
return errorsmod.Wrap(err, "error validating topic id")
238253
}
254+
if nonce == nil {
255+
return errors.New("nil reputer's nonce provided")
256+
}
239257
if err := nonce.Validate(); err != nil {
240258
return errorsmod.Wrap(err, "error validating nonce")
241259
}
242260
nonces, err := k.GetUnfulfilledReputerNonces(ctx, topicId)
243261
if err != nil {
244262
return errorsmod.Wrap(err, "error getting unfulfilled reputer nonces")
245263
}
246-
if nonce == nil {
247-
return errors.New("nil reputer's nonce provided")
248-
}
249264

250265
// Check that input nonce is not already contained in the nonces of this topic
251266
// nor that the `associatedWorkerNonce` is already associated with a worker request
252267
for _, n := range nonces.Nonces {
268+
if n == nil || n.ReputerNonce == nil {
269+
continue
270+
}
253271
// Do nothing if nonce is already in the list
254272
if n.ReputerNonce.BlockHeight == nonce.BlockHeight {
255273
return nil
@@ -381,10 +399,8 @@ func (k *NonceKeeper) PruneWorkerNonces(ctx context.Context, topicId uint64, blo
381399
if err := types.ValidateTopicId(topicId); err != nil {
382400
return errorsmod.Wrap(err, "error validating topic id")
383401
}
384-
nonces, err := k.unfulfilledWorkerNonces.Get(ctx, topicId)
385-
if errors.Is(err, collections.ErrNotFound) {
386-
return errorsmod.Wrapf(err, "no nonces found to prune for topic %d", topicId)
387-
} else if err != nil {
402+
nonces, err := k.GetUnfulfilledWorkerNonces(ctx, topicId)
403+
if err != nil {
388404
return errorsmod.Wrap(err, "error getting unfulfilled worker nonces")
389405
}
390406

@@ -420,6 +436,9 @@ func (k *NonceKeeper) PruneReputerNonces(ctx context.Context, topicId uint64, bl
420436
// Filter Nonces based on block_height
421437
filteredNonces := make([]*types.ReputerRequestNonce, 0)
422438
for _, nonce := range nonces.Nonces {
439+
if nonce == nil || nonce.ReputerNonce == nil {
440+
continue
441+
}
423442
if nonce.ReputerNonce.BlockHeight >= blockHeightThreshold {
424443
filteredNonces = append(filteredNonces, nonce)
425444
}

x/emissions/keeper/nonces_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package keeper_test
22

33
import (
4-
"cosmossdk.io/collections"
5-
64
"github.com/allora-network/allora-chain/x/emissions/testutil"
75
"github.com/allora-network/allora-chain/x/emissions/types"
86
)
@@ -663,14 +661,19 @@ func (s *KeeperTestSuite) TestPruneWorkerNoncesLogicNoNonces() {
663661

664662
// Call pruneWorkerNonces
665663
err = k.PruneWorkerNonces(s.Ctx(), topicId1, blockHeightThreshold)
666-
s.Require().ErrorIs(err, collections.ErrNotFound)
664+
s.Require().NoError(err)
667665

668666
// Check remaining nonces
669667
nonces, err := k.GetUnfulfilledWorkerNonces(s.Ctx(), topicId1)
670668
s.Require().NoError(err)
671669
s.Require().Empty(nonces.Nonces)
672670
}
673671

672+
func (s *KeeperTestSuite) TestAddReputerNonceNilInput() {
673+
err := s.NonceKeeper().AddReputerNonce(s.Ctx(), uint64(1), nil)
674+
s.Require().Error(err)
675+
}
676+
674677
func (s *KeeperTestSuite) TestPruneWorkerNoncesLogicCorrectness() {
675678
tests := []struct {
676679
name string

x/emissions/keeper/staking.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,7 @@ func (k *StakingKeeper) GetDelegateStakeRemovalForDelegatorReputerAndTopicId(
513513
if err != nil {
514514
return types.DelegateStakeRemovalInfo{}, false, errorsmod.Wrap(err, "error iterating over delegate stake removals by actor")
515515
}
516+
defer iter.Close()
516517
keys, err := iter.Keys()
517518
if err != nil {
518519
return types.DelegateStakeRemovalInfo{}, false, errorsmod.Wrap(err, "error getting keys")

0 commit comments

Comments
 (0)