x/incentives v2: Validator incentives #320
Conversation
WalkthroughThis pull request introduces extensive modifications primarily focused on upgrading the Gravity module from version 4 to version 5 and enhancing the incentives management system. Key updates include the addition of new parameters for validator incentives, updates to the Keeper struct, and improvements to the BeginBlocker method. Furthermore, new test cases and error handling mechanisms have been implemented, alongside the introduction of new modules and functions related to cellar fees and auctions. The changes reflect a comprehensive upgrade to the system, enhancing both functionality and maintainability across the affected files. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post
🛑 Comments failed to post (49)
app/upgrades/v8/upgrades.go (1)
13-18: 💡 Codebase verification
Issue Found: Incomplete v8 Upgrade Handler
The v8 upgrade handler contains a TODO comment indicating that the upgrade handle needs to be added:
app/app.go: // TODO: Add v8 upgrade handleThis suggests that the current implementation may be missing essential logic required for a complete v8 upgrade.
Please ensure that the v8 upgrade handler is fully implemented to handle all necessary upgrade steps.
🔗 Analysis chain
LGTM: Upgrade handler implementation is correct, but verify completeness.
The implementation follows the expected pattern for creating an upgrade handler in the Cosmos SDK:
- It returns an anonymous function with the correct signature.
- It logs the start of the upgrade process, which is helpful for debugging and monitoring.
- It calls
mm.RunMigrationsto execute necessary migrations for all modules.However, there's no custom logic or state modifications in this upgrade handler. While this might be intentional, it's worth verifying that no specific changes are required for the v8 upgrade.
To ensure the upgrade handler is complete, please run the following script to check for any TODO comments or upgrade-related changes in other files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for TODO comments or upgrade-related changes that might indicate missing logic in the v8 upgrade handler. # Test 1: Search for TODO comments related to v8 upgrade echo "Searching for TODO comments related to v8 upgrade:" rg --type go "TODO.*v8.*upgrade" # Test 2: Search for v8-related changes in other files echo "Searching for v8-related changes in other files:" rg --type go "v8.*upgrade" # Test 3: Check for any upgrade-related changes in the app module echo "Checking for upgrade-related changes in the app module:" rg --type go "upgrade" app/ # Note: Review the results manually to determine if any additional logic should be added to the v8 upgrade handler.Length of output: 6582
x/cellarfees/types/errors.go (1)
9-10: 💡 Codebase verification
Removed error variables are still in use.
The removed error variables
ErrInvalidFeeAccrualAuctionThresholdandErrInvalidFeeAccrualCountersare still referenced in the following files:
x/cellarfees/migrations/v1/types/params.gox/cellarfees/migrations/v1/types/errors.gox/cellarfees/migrations/v1/types/genesis.goPlease ensure that all usages of these error variables are addressed to prevent potential issues.
🔗 Analysis chain
LGTM. Verify unused errors in the codebase.
The added comment provides clear context for the missing error codes 2 and 6, which is helpful for future maintenance. The removal of
ErrInvalidFeeAccrualAuctionThresholdandErrInvalidFeeAccrualCountersaligns with the v2 module upgrade mentioned in the PR title.To ensure these changes don't introduce any issues, please run the following script to verify that the removed errors are no longer used in the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of the removed error variables # Test 1: Search for ErrInvalidFeeAccrualAuctionThreshold echo "Searching for ErrInvalidFeeAccrualAuctionThreshold:" rg --type go "ErrInvalidFeeAccrualAuctionThreshold" # Test 2: Search for ErrInvalidFeeAccrualCounters echo "Searching for ErrInvalidFeeAccrualCounters:" rg --type go "ErrInvalidFeeAccrualCounters" # If both searches return no results, it confirms that the removed errors are no longer used.Length of output: 1217
x/cellarfees/migrations/v1/types/genesis.go (2)
12-12: 🛠️ Refactor suggestion
Improve the function comment for clarity and correctness.
The comment for
DefaultGenesisStatecontains a grammatical error and can be rephrased for better clarity.Apply this change to enhance the comment:
-// DefaultGenesisState get raw genesis raw message for testing +// DefaultGenesisState returns the default genesis state for the cellarfees module.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// DefaultGenesisState returns the default genesis state for the cellarfees module.
36-36:
⚠️ Potential issueCorrect the format specifiers in the error message.
The error message uses
%T, which displays the type of the variable instead of its value. To show the actual contents of the slices, use%vor%+vfor more detailed output.Apply this change to improve the error message:
-return ErrInvalidFeeAccrualCounters.Wrapf("counters are unsorted! expected: %T, actual: %T", counters.Counters, gs.FeeAccrualCounters.Counters) +return ErrInvalidFeeAccrualCounters.Wrapf("counters are unsorted! expected: %v, actual: %v", counters.Counters, gs.FeeAccrualCounters.Counters)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.return ErrInvalidFeeAccrualCounters.Wrapf("counters are unsorted! expected: %v, actual: %v", counters.Counters, gs.FeeAccrualCounters.Counters)x/cellarfees/migrations/v1/keeper/auction.go (3)
24-24:
⚠️ Potential issueFix incorrect usage of logger's
ErrormethodThe
k.Logger(ctx).Errormethod expects a message and key-value pairs, not formatted strings. Modify the log statement to correctly log the error with the denomination.Apply this diff to fix the logger usage:
-k.Logger(ctx).Error("Attempted to begin auction for denom %s with a zero balance.", denom) +k.Logger(ctx).Error("Attempted to begin auction with zero balance", "denom", denom)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.k.Logger(ctx).Error("Attempted to begin auction with zero balance", "denom", denom)
43-43:
⚠️ Potential issueFix incorrect usage of logger's
ErrormethodSimilarly, adjust the logger usage to correctly log the error message without using a formatted string.
Apply this diff to fix the logger usage:
-k.Logger(ctx).Error(err.Error()) +k.Logger(ctx).Error("Error starting auction", "error", err.Error())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.k.Logger(ctx).Error("Error starting auction", "error", err.Error())
38-42: 🛠️ Refactor suggestion
Avoid using
panic; handle errors gracefullyUsing
panicfor error handling can lead to unintended application crashes. Instead, handle the errors by logging them and returning a value to indicate failure.Apply this diff to handle errors without panicking:
switch err { case auctiontypes.ErrUnauthorizedFundingModule: - panic("Attempted to start an auction with an unauthorized funding module") + k.Logger(ctx).Error("Unauthorized funding module", "error", err.Error()) return false case auctiontypes.ErrUnauthorizedProceedsModule: - panic("Attempted to start an auction with an unauthorized proceeds module") + k.Logger(ctx).Error("Unauthorized proceeds module", "error", err.Error()) return false default: k.Logger(ctx).Error("Error starting auction", "error", err.Error()) }Committable suggestion was skipped due to low confidence.
x/cellarfees/migrations/v1/keeper/abci.go (2)
19-20:
⚠️ Potential issuePrevent potential divide-by-zero error when
AuctionIntervalis zeroIn line 19, the modulus operation could result in a divide-by-zero panic if
cellarfeesParams.AuctionIntervalis zero. Ensure thatAuctionIntervalis validated to be greater than zero to prevent this issue.Apply this diff to add a validation check:
+ if cellarfeesParams.AuctionInterval == 0 { + k.Logger(ctx).Error("AuctionInterval is zero, cannot proceed with BeginBlocker") + return + } modulus := ctx.BlockHeader().Height % int64(cellarfeesParams.AuctionInterval)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if cellarfeesParams.AuctionInterval == 0 { k.Logger(ctx).Error("AuctionInterval is zero, cannot proceed with BeginBlocker") return } modulus := ctx.BlockHeader().Height % int64(cellarfeesParams.AuctionInterval)
34-47:
⚠️ Potential issueHandle errors without panicking; implement graceful error handling
Panicking within a
BeginBlockercan halt the entire blockchain, which may not be desirable. Instead of panicking when an error occurs during the emission transfer, consider logging the error and allowing the process to continue or handling it appropriately.Apply this diff to handle the error gracefully:
// Send to fee collector for distribution err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, moduleAccount.GetName(), authtypes.FeeCollectorName, emission) if err != nil { - panic(err) + k.Logger(ctx).Error("Failed to send emission to fee collector", "error", err) + // Consider handling the error appropriately, such as retrying or skipping this emission + return }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.moduleAccount := k.GetFeesAccount(ctx) remainingRewardsSupply := k.bankKeeper.GetBalance(ctx, moduleAccount.GetAddress(), params.BaseCoinUnit).Amount if remainingRewardsSupply.IsZero() { return } emission := k.GetEmission(ctx, remainingRewardsSupply) // Send to fee collector for distribution err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, moduleAccount.GetName(), authtypes.FeeCollectorName, emission) if err != nil { k.Logger(ctx).Error("Failed to send emission to fee collector", "error", err) // Consider handling the error appropriately, such as retrying or skipping this emission return }x/cellarfees/migrations/v1/keeper/cellarfees.go (1)
23-24:
⚠️ Potential issueCheck for division by zero in
GetEmissioncalculationsIn the calculation of
emissionAmount, the code divides bycellarfeesParams.RewardEmissionPeriodwithout verifying if it is zero. IfRewardEmissionPeriodis zero, this will cause a runtime panic due to division by zero.Consider adding a check to ensure
RewardEmissionPeriodis greater than zero before performing the division. IfRewardEmissionPeriodshould never be zero by design, you may still want to add validation to prevent potential misuse.x/cellarfees/migrations/v1/types/cellarfees.go (4)
37-50: 🛠️ Refactor suggestion
Optimize lookups by using binary search
Since
f.Countersis sorted byDenom, you can use binary search to find the index of a counter more efficiently than a linear search.Update
IncrementCounter:func (f *FeeAccrualCounters) IncrementCounter(denom string) uint64 { - found := false - var count uint64 - for i, k := range f.Counters { - if k.Denom == denom { - found = true - f.Counters[i].Count++ - count = f.Counters[i].Count - break - } - } - if !found { - f.insertCounter(denom, 1) - count = 1 - } + idx := sort.Search(len(f.Counters), func(i int) bool { + return f.Counters[i].Denom >= denom + }) + if idx < len(f.Counters) && f.Counters[idx].Denom == denom { + f.Counters[idx].Count++ + } else { + f.insertCounterAt(idx, denom, 1) + } + return f.Counters[idx].Count - return count }Similarly, update
ResetCounter:func (f *FeeAccrualCounters) ResetCounter(denom string) { - found := false - for i, k := range f.Counters { - if k.Denom == denom { - found = true - f.Counters[i].Count = 0 - break - } - } - if !found { - f.insertCounter(denom, 0) - } + idx := sort.Search(len(f.Counters), func(i int) bool { + return f.Counters[i].Denom >= denom + }) + if idx < len(f.Counters) && f.Counters[idx].Denom == denom { + f.Counters[idx].Count = 0 + } else { + f.insertCounterAt(idx, denom, 0) + } }And define the new
insertCounterAtmethod:func (f *FeeAccrualCounters) insertCounterAt(idx int, denom string, count uint64) { f.Counters = append(f.Counters, FeeAccrualCounter{}) copy(f.Counters[idx+1:], f.Counters[idx:]) f.Counters[idx] = FeeAccrualCounter{Denom: denom, Count: count} }Also applies to: 57-68
37-50: 🛠️ Refactor suggestion
Refactor to reduce code duplication
Both
IncrementCounterandResetCounterhave similar logic for finding a counter byDenom. Refactoring this into a helper function enhances maintainability.Create a helper method:
func (f *FeeAccrualCounters) findCounterIndex(denom string) (int, bool) { for i, k := range f.Counters { if k.Denom == denom { return i, true } } return -1, false }Update
IncrementCounter:func (f *FeeAccrualCounters) IncrementCounter(denom string) uint64 { - // Existing code + idx, found := f.findCounterIndex(denom) + if found { + f.Counters[idx].Count++ + } else { + f.insertCounter(denom, 1) + idx = len(f.Counters) - 1 + } + return f.Counters[idx].Count }And
ResetCounter:func (f *FeeAccrualCounters) ResetCounter(denom string) { - // Existing code + idx, found := f.findCounterIndex(denom) + if found { + f.Counters[idx].Count = 0 + } else { + f.insertCounter(denom, 0) + } }Also applies to: 57-68
19-21: 🛠️ Refactor suggestion
Simplify the comparison in the
LessmethodYou can simplify the
Lessmethod by using the<operator for string comparison, which is more idiomatic in Go.Apply this diff to streamline the method:
-func (f FeeAccrualCounters) Less(i, j int) bool { - return strings.Compare(f.Counters[i].Denom, f.Counters[j].Denom) == -1 +func (f FeeAccrualCounters) Less(i, j int) bool { + return f.Counters[i].Denom < f.Counters[j].Denom }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (f FeeAccrualCounters) Less(i, j int) bool { return f.Counters[i].Denom < f.Counters[j].Denom }
30-32: 🛠️ Refactor suggestion
Avoid unnecessary sorting after each insertion
Sorting the entire slice after every insertion in
insertCountercan be inefficient, especially with a large number of counters. Since you maintain the slice in sorted order byDenom, consider inserting the new counter directly at the correct position.Modify
insertCounterto insert at the correct index without sorting:-func (f *FeeAccrualCounters) insertCounter(denom string, count uint64) { - f.Counters = append(f.Counters, FeeAccrualCounter{Denom: denom, Count: count}) - sort.Sort(f) +func (f *FeeAccrualCounters) insertCounter(denom string, count uint64) { + idx := sort.Search(len(f.Counters), func(i int) bool { + return f.Counters[i].Denom >= denom + }) + f.Counters = append(f.Counters, FeeAccrualCounter{}) + copy(f.Counters[idx+1:], f.Counters[idx:]) + f.Counters[idx] = FeeAccrualCounter{Denom: denom, Count: count} }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (f *FeeAccrualCounters) insertCounter(denom string, count uint64) { idx := sort.Search(len(f.Counters), func(i int) bool { return f.Counters[i].Denom >= denom }) f.Counters = append(f.Counters, FeeAccrualCounter{}) copy(f.Counters[idx+1:], f.Counters[idx:]) f.Counters[idx] = FeeAccrualCounter{Denom: denom, Count: count} }x/cellarfees/migrations/v1/keeper/query_server.go (2)
51-55: 🛠️ Refactor suggestion
Add nil request check to
QueryAPYfor consistency.Unlike the other query methods,
QueryAPYdoes not check if the request parameter is nil. For consistency and to prevent potential issues in the future, consider adding a nil check similar to the other methods.You can add the following check:
if _ == nil { return nil, status.Error(codes.InvalidArgument, "empty request") }
7-8:
⚠️ Potential issueRedundant import of the same package.
The package
"github.com/peggyjv/sommelier/v7/x/cellarfees/migrations/v1/types"is imported twice: once without an alias (line 7) and once asv1types(line 8). This redundancy could lead to confusion and is unnecessary. Consider consolidating the imports to use a single alias.You can fix this by removing one of the imports or unifying them under a single alias. For example:
-import ( ... "github.com/peggyjv/sommelier/v7/x/cellarfees/migrations/v1/types" v1types "github.com/peggyjv/sommelier/v7/x/cellarfees/migrations/v1/types" ... ) +import ( ... v1types "github.com/peggyjv/sommelier/v7/x/cellarfees/migrations/v1/types" ... )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.v1types "github.com/peggyjv/sommelier/v7/x/cellarfees/migrations/v1/types"x/cellarfees/migrations/v1/store.go (1)
48-48: 🛠️ Refactor suggestion
Clarify or remove outdated comment on
FeeAccrualCountersdeletionThe comment on line 48 raises a concern about whether setting the new parameter set deletes the old
FeeAccrualCounterskey. Since themigrateCellarfeesFeeAccrualCountersfunction explicitly handles the deletion of the oldFeeAccrualCountersfrom the store (lines 56-68), this concern has been addressed. Consider removing or updating the comment to reflect the current implementation.x/incentives/types/expected_keepers.go (1)
46-48:
⚠️ Potential issueConsider handling cases where the validator is not found
The method
ValidatorByConsAddr(ctx sdk.Context, consAddr sdk.ConsAddress) stakingtypes.ValidatorIdoes not indicate whether the validator exists. Consider modifying the method signature to return an additional boolean or error to handle cases where the validator is not found.x/cellarfees/migrations/v1/keeper/hooks.go (3)
37-76:
⚠️ Potential issueImplement the auction initiation logic when threshold is reached
The comments in lines 34-36 mention that when the fee accrual counter for a denomination reaches a threshold defined in the
cellarfeesparameters, an auction should be initiated, and the counter reset. However, the current implementation of theAfterSendToCosmosEventfunction does not include logic to check the counter against the threshold or to start an auction when the threshold is reached.Would you like assistance in implementing the logic to initiate the auction when the threshold is reached? This could involve retrieving the threshold from the parameters, comparing it with the
count, and invoking the auction initiation process when appropriate.
13-15: 🛠️ Refactor suggestion
Use pointer receivers for the
Hooksmethods to avoid unnecessary copyingCurrently, the
Hooksstruct methods use value receivers. SinceHookscontains theKeeper, which may be a sizable struct, using value receivers can lead to unnecessary copying of theKeeperfield with each method call. Consider changing the receivers to pointers to improve performance.Apply this diff to change the receivers to pointers:
type Hooks struct { - k Keeper + k Keeper } -var _ gravitytypes.GravityHooks = Hooks{} +var _ gravitytypes.GravityHooks = &Hooks{} -func (k Keeper) Hooks() Hooks { - return Hooks{k} +func (k Keeper) Hooks() *Hooks { + return &Hooks{k} } -func (h Hooks) AfterContractCallExecutedEvent(ctx sdk.Context, event gravitytypes.ContractCallExecutedEvent) { +func (h *Hooks) AfterContractCallExecutedEvent(ctx sdk.Context, event gravitytypes.ContractCallExecutedEvent) {Also applies to: 17-17, 20-22
55-58:
⚠️ Potential issueHandle potential errors from
ERC20ToDenomLookupThe function
ERC20ToDenomLookupmay not find a matching denomination for the given token contract, which could lead to an empty or incorrectdenom. Consider adding error handling to manage this scenario gracefully.Apply this diff to handle potential errors:
_, denom := h.k.gravityKeeper.ERC20ToDenomLookup(ctx, common.HexToAddress(event.TokenContract)) +if denom == "" { + return +} if denom == params.BaseCoinUnit { return }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements._, denom := h.k.gravityKeeper.ERC20ToDenomLookup(ctx, common.HexToAddress(event.TokenContract)) if denom == "" { return } if denom == params.BaseCoinUnit { return }x/incentives/keeper/abci.go (1)
33-33: 🛠️ Refactor suggestion
Evaluate the necessity of the
reqparameter ingetValidatorInfosThe function call
valInfos := k.getValidatorInfos(ctx, req)introduces a dependency on thereq abci.RequestBeginBlockparameter. Assess whether the required information can be obtained fromctxinstead ofreq. Simplifying the parameter list can improve code maintainability and reduce coupling between functions.x/incentives/keeper/incentives.go (3)
65-84:
⚠️ Potential issueEnsure
totalPreviousPoweris not zero to prevent division by zeroIn the
AllocateTokensfunction,totalPreviousPoweris used as a divisor when calculatingpowerFraction. IftotalPreviousPoweris zero, this will result in a division by zero panic.Add a check to ensure
totalPreviousPoweris greater than zero before proceeding:if totalPreviousPower == 0 { // Handle the zero case appropriately // For example, return the remaining distribution without allocating: return totalDistribution }This prevents a potential runtime error and allows the function to handle the edge case gracefully.
28-36: 🛠️ Refactor suggestion
Avoid passing slices by pointer; pass slices directly
In the
getTotalPowerfunction, you are passingvalInfosas a pointer to a slice (*[]ValidatorInfo). Since slices in Go are reference types, passing them directly is sufficient and more idiomatic. Passing a pointer to a slice is rarely necessary and may lead to confusion.Refactor the function to pass the slice directly:
-func getTotalPower(valInfos *[]ValidatorInfo) int64 { +func getTotalPower(valInfos []ValidatorInfo) int64 { totalPower := int64(0) - for _, valInfo := range *valInfos { + for _, valInfo := range valInfos { totalPower += valInfo.Power } return totalPower }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// GetTotalPower returns the total power of the passed in validatorInfos func getTotalPower(valInfos []ValidatorInfo) int64 { totalPower := int64(0) for _, valInfo := range valInfos { totalPower += valInfo.Power } return totalPower }
39-53:
⚠️ Potential issueCheck for
nilvalidator to prevent nil pointer dereferenceIn the
getValidatorInfosfunction, theValidatorByConsAddrmethod may returnnilif the validator is not found. Attempting to use anilvalidator can lead to a runtime panic when accessing its methods.Add a
nilcheck before appending tovalidatorInfos:validator := k.StakingKeeper.ValidatorByConsAddr(ctx, vote.Validator.Address) +if validator == nil { + continue +} validatorInfos = append(validatorInfos, ValidatorInfo{ Validator: validator, Power: vote.Validator.Power, })This ensures that only valid validators are processed, preventing potential errors.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (k Keeper) getValidatorInfos(ctx sdk.Context, req abci.RequestBeginBlock) []ValidatorInfo { validatorInfos := []ValidatorInfo{} for _, vote := range req.LastCommitInfo.GetVotes() { if !vote.SignedLastBlock { continue } validator := k.StakingKeeper.ValidatorByConsAddr(ctx, vote.Validator.Address) if validator == nil { continue } validatorInfos = append(validatorInfos, ValidatorInfo{ Validator: validator, Power: vote.Validator.Power, }) } return validatorInfos }integration_tests/genesis.go (1)
114-126:
⚠️ Potential issueHandle potential marshaling errors instead of using
MustMarshalJSONIn line 124,
cdc.MustMarshalJSONis used to marshalincentivesGenState.MustMarshalJSONwill panic if marshaling fails, which could terminate the test suite unexpectedly. It's better practice to handle the error explicitly to make the code more robust.You can modify the code to handle the error:
func (s *IntegrationTestSuite) setIncentivesGenState(appGenState map[string]json.RawMessage) error { incentivesGenState := incentivestypes.DefaultGenesisState() err := cdc.UnmarshalJSON(appGenState[incentivestypes.ModuleName], &incentivesGenState) if err != nil { return fmt.Errorf("failed to unmarshal incentives genesis state: %w", err) } incentivesGenState.Params.ValidatorIncentivesCutoffHeight = 0 incentivesGenState.Params.ValidatorDistributionPerBlock = sdk.NewCoin(params.BaseCoinUnit, sdk.NewInt(0)) - appGenState[incentivestypes.ModuleName] = cdc.MustMarshalJSON(&incentivesGenState) + incentivesGenStateBz, err := cdc.MarshalJSON(&incentivesGenState) + if err != nil { + return fmt.Errorf("failed to marshal incentives genesis state: %w", err) + } + appGenState[incentivestypes.ModuleName] = incentivesGenStateBz return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (s *IntegrationTestSuite) setIncentivesGenState(appGenState map[string]json.RawMessage) error { incentivesGenState := incentivestypes.DefaultGenesisState() err := cdc.UnmarshalJSON(appGenState[incentivestypes.ModuleName], &incentivesGenState) if err != nil { return fmt.Errorf("failed to unmarshal incentives genesis state: %w", err) } incentivesGenState.Params.ValidatorIncentivesCutoffHeight = 0 incentivesGenState.Params.ValidatorDistributionPerBlock = sdk.NewCoin(params.BaseCoinUnit, sdk.NewInt(0)) incentivesGenStateBz, err := cdc.MarshalJSON(&incentivesGenState) if err != nil { return fmt.Errorf("failed to marshal incentives genesis state: %w", err) } appGenState[incentivestypes.ModuleName] = incentivesGenStateBz return nil }x/cellarfees/migrations/v1/keeper/keeper.go (3)
85-87:
⚠️ Potential issueAvoid using
panicin production codeUsing
paniccan cause the application to crash and is generally discouraged in production code. Instead of panicking whenbisnil, consider handling the error gracefully by returning a default value or an error.Apply this diff to handle the
nilcase without panicking:func (k Keeper) GetLastRewardSupplyPeak(ctx sdk.Context) math.Int { store := ctx.KVStore(k.storeKey) b := store.Get(v1types.GetLastRewardSupplyPeakKey()) if b == nil { - panic("Last highest reward supply should not have been nil") + // Return zero amount if the value is nil + return sdk.ZeroInt() } var amount big.Int return sdk.NewIntFromBigInt((&amount).SetBytes(b)) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if b == nil { // Return zero amount if the value is nil return sdk.ZeroInt() }
144-145:
⚠️ Potential issuePrevent division by zero in APY calculation
There is a potential risk of division by zero if
totalCoinsorbondedRatioare zero, which would cause a runtime error. Ensure to check these values before performing the division.Apply this diff to add checks for zero values:
annualRewards := emission.AmountOf(params.BaseCoinUnit).Mul(sdk.NewInt(int64(mintParams.BlocksPerYear))) + if totalCoins.IsZero() || bondedRatio.IsZero() { + return sdk.ZeroDec() + } return sdk.NewDecFromInt(annualRewards).Quo(sdk.NewDecFromInt(totalCoins)).Quo(bondedRatio)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.annualRewards := emission.AmountOf(params.BaseCoinUnit).Mul(sdk.NewInt(int64(mintParams.BlocksPerYear))) if totalCoins.IsZero() || bondedRatio.IsZero() { return sdk.ZeroDec() } return sdk.NewDecFromInt(annualRewards).Quo(sdk.NewDecFromInt(totalCoins)).Quo(bondedRatio) }
105-107:
⚠️ Potential issueHandle
nilvalues without panickingEncountering a
nilvalue for fee accrual counters should not cause a panic. Instead, return a default value or handle the error appropriately to maintain application stability.Apply this diff to handle the
nilcase:func (k Keeper) GetFeeAccrualCounters(ctx sdk.Context) (counters v1types.FeeAccrualCounters) { store := ctx.KVStore(k.storeKey) b := store.Get(v1types.GetFeeAccrualCountersKey()) if b == nil { - panic("Fee accrual counters is nil, it should have been set by InitGenesis") + // Return default fee accrual counters if the value is nil + return v1types.DefaultFeeAccrualCounters() } if len(b) == 0 { return v1types.DefaultFeeAccrualCounters() } k.cdc.MustUnmarshal(b, &counters) return }Committable suggestion was skipped due to low confidence.
x/incentives/types/params.go (2)
93-95:
⚠️ Potential issueAdd type validation in
validateValidatorIncentivesCutoffHeightfunction.The
validateValidatorIncentivesCutoffHeightfunction currently returnsnilwithout performing any validation. Even if there's no invalid height value, it's important to ensure that the parameter is of the expected type (uint64). Adding a type check will prevent unexpected parameter types from being accepted.Apply this diff to add type validation:
func validateValidatorIncentivesCutoffHeight(i interface{}) error { + _, ok := i.(uint64) + if !ok { + return errorsmod.Wrapf(ErrInvalidValidatorIncentivesCutoffHeight, "invalid parameter type: %T", i) + } return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func validateValidatorIncentivesCutoffHeight(i interface{}) error { _, ok := i.(uint64) if !ok { return errorsmod.Wrapf(ErrInvalidValidatorIncentivesCutoffHeight, "invalid parameter type: %T", i) } return nil }
86-88:
⚠️ Potential issueWrap the original error in
validateValidatorDistributionPerBlockfunction.Currently,
validateValidatorDistributionPerBlockwraps the error fromvalidateDistributionPerBlockwith a generic message that may not accurately reflect the underlying issue. It's better to wrap the original error to preserve detailed error information and provide better context.Apply this diff to wrap the original error:
func validateValidatorDistributionPerBlock(i interface{}) error { if err := validateDistributionPerBlock(i); err != nil { - return errorsmod.Wrapf(ErrInvalidValidatorDistributionPerBlock, "invalid parameter type: %T", i) + return errorsmod.Wrap(ErrInvalidValidatorDistributionPerBlock, err.Error()) } return nil }Committable suggestion was skipped due to low confidence.
x/cellarfees/migrations/v1/types/params.go (3)
81-92:
⚠️ Potential issueEnsure consistent error references in
validateFeeAccrualAuctionThresholdIn the
validateFeeAccrualAuctionThresholdfunction, the errorErrInvalidFeeAccrualAuctionThresholdis referenced without thetypes.prefix. In other validation functions, errors are referenced with thetypes.prefix (e.g.,types.ErrInvalidRewardEmissionPeriod). This inconsistency might lead to undefined errors or confusion. To maintain consistency and ensure correct error handling, prefix the error withtypes..Apply the following changes:
func validateFeeAccrualAuctionThreshold(i interface{}) error { threshold, ok := i.(uint64) if !ok { - return errorsmod.Wrapf(ErrInvalidFeeAccrualAuctionThreshold, "fee accrual auction threshold: %T", i) + return errorsmod.Wrapf(types.ErrInvalidFeeAccrualAuctionThreshold, "fee accrual auction threshold: %T", i) } if threshold == 0 { - return errorsmod.Wrapf(ErrInvalidFeeAccrualAuctionThreshold, "fee accrual auction threshold cannot be zero") + return errorsmod.Wrapf(types.ErrInvalidFeeAccrualAuctionThreshold, "fee accrual auction threshold cannot be zero") } return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func validateFeeAccrualAuctionThreshold(i interface{}) error { threshold, ok := i.(uint64) if !ok { return errorsmod.Wrapf(types.ErrInvalidFeeAccrualAuctionThreshold, "fee accrual auction threshold: %T", i) } if threshold == 0 { return errorsmod.Wrapf(types.ErrInvalidFeeAccrualAuctionThreshold, "fee accrual auction threshold cannot be zero") } return nil }
65-79:
⚠️ Potential issueInclude
AuctionIntervalvalidation inValidateBasicThe
ValidateBasicmethod currently does not validateAuctionInterval. To ensure all parameters are properly validated, add a call tovalidateAuctionInterval.Apply the following change:
func (p *Params) ValidateBasic() error { if err := validateFeeAccrualAuctionThreshold(p.FeeAccrualAuctionThreshold); err != nil { return err } if err := validateRewardEmissionPeriod(p.RewardEmissionPeriod); err != nil { return err } if err := validateInitialPriceDecreaseRate(p.InitialPriceDecreaseRate); err != nil { return err } if err := validatePriceDecreaseBlockInterval(p.PriceDecreaseBlockInterval); err != nil { return err } + if err := validateAuctionInterval(p.AuctionInterval); err != nil { + return err + } return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (p *Params) ValidateBasic() error { if err := validateFeeAccrualAuctionThreshold(p.FeeAccrualAuctionThreshold); err != nil { return err } if err := validateRewardEmissionPeriod(p.RewardEmissionPeriod); err != nil { return err } if err := validateInitialPriceDecreaseRate(p.InitialPriceDecreaseRate); err != nil { return err } if err := validatePriceDecreaseBlockInterval(p.PriceDecreaseBlockInterval); err != nil { return err } if err := validateAuctionInterval(p.AuctionInterval); err != nil { return err } return nil }
108-122:
⚠️ Potential issueImprove validation logic for
InitialPriceDecreaseRateThe current validation for
InitialPriceDecreaseRateonly checks if the rate is equal to zero or one. According to the error messages, the valid range should be0 < x < 1. To ensure the rate falls within this range, the validation should check that the rate is greater than zero and less than one.Apply the following changes to correct the validation logic:
func validateInitialPriceDecreaseRate(i interface{}) error { rate, ok := i.(sdk.Dec) if !ok { return errorsmod.Wrapf(types.ErrInvalidInitialPriceDecreaseRate, "initial price decrease rate: %T", i) } - if rate == sdk.ZeroDec() { + if rate.LTE(sdk.ZeroDec()) { return errorsmod.Wrapf(types.ErrInvalidInitialPriceDecreaseRate, "initial price decrease rate cannot be zero, must be 0 < x < 1") } - if rate == sdk.OneDec() { + if rate.GTE(sdk.OneDec()) { return errorsmod.Wrapf(types.ErrInvalidInitialPriceDecreaseRate, "initial price decrease rate cannot be one, must be 0 < x < 1") } return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.rate, ok := i.(sdk.Dec) if !ok { return errorsmod.Wrapf(types.ErrInvalidInitialPriceDecreaseRate, "initial price decrease rate: %T", i) } if rate.LTE(sdk.ZeroDec()) { return errorsmod.Wrapf(types.ErrInvalidInitialPriceDecreaseRate, "initial price decrease rate cannot be zero, must be 0 < x < 1") } if rate.GTE(sdk.OneDec()) { return errorsmod.Wrapf(types.ErrInvalidInitialPriceDecreaseRate, "initial price decrease rate cannot be one, must be 0 < x < 1") } return nil }x/auction/migrations/v1/types/params.go (3)
111-118:
⚠️ Potential issueAdd value validation for
AuctionMaxBlockAgeparameterThe
validateAuctionMaxBlockAgefunction checks the type but doesn't validate the value. Consider adding a check to ensure thatAuctionMaxBlockAgeis greater than zero to prevent invalid configurations.Apply this diff to add the value validation:
func validateAuctionMaxBlockAge(i interface{}) error { auctionMaxBlockAge, ok := i.(uint64) if !ok { return errorsmod.Wrapf(ErrInvalidAuctionMaxBlockAgeParam, "invalid auction max block age parameter type: %T", i) } + if auctionMaxBlockAge == 0 { + return errorsmod.Wrapf(ErrInvalidAuctionMaxBlockAgeParam, "auction max block age must be greater than zero") + } return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func validateAuctionMaxBlockAge(i interface{}) error { auctionMaxBlockAge, ok := i.(uint64) if !ok { return errorsmod.Wrapf(ErrInvalidAuctionMaxBlockAgeParam, "invalid auction max block age parameter type: %T", i) } if auctionMaxBlockAge == 0 { return errorsmod.Wrapf(ErrInvalidAuctionMaxBlockAgeParam, "auction max block age must be greater than zero") } return nil }
88-95:
⚠️ Potential issueAdd value validation for
MinimumBidInUsommparameterThe
validateMinimumBidInUsommfunction checks the type but doesn't validate the value. Consider adding a check to ensure thatMinimumBidInUsommis greater than zero to prevent invalid configurations.Apply this diff to add the value validation:
func validateMinimumBidInUsomm(i interface{}) error { minimumBidInUsomm, ok := i.(uint64) if !ok { return errorsmod.Wrapf(ErrMinimumBidParam, "invalid minimum bid in usomm parameter type: %T", i) } + if minimumBidInUsomm == 0 { + return errorsmod.Wrapf(ErrMinimumBidParam, "minimum bid in usomm must be greater than zero") + } return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func validateMinimumBidInUsomm(i interface{}) error { minimumBidInUsomm, ok := i.(uint64) if !ok { return errorsmod.Wrapf(ErrMinimumBidParam, "invalid minimum bid in usomm parameter type: %T", i) } if minimumBidInUsomm == 0 { return errorsmod.Wrapf(ErrMinimumBidParam, "minimum bid in usomm must be greater than zero") } return nil }
51-73:
⚠️ Potential issueInclude
validateMinimumAuctionHeightinValidateBasicmethodThe
ValidateBasicmethod does not callvalidateMinimumAuctionHeightto validate theMinimumAuctionHeightparameter. To ensure all parameters are properly validated, please include this validation.Apply this diff to include the missing validation:
func (p *Params) ValidateBasic() error { if err := validatePriceMaxBlockAge(p.PriceMaxBlockAge); err != nil { return err } if err := validateMinimumBidInUsomm(p.MinimumBidInUsomm); err != nil { return err } if err := validateMinimumSaleTokensUSDValue(p.MinimumSaleTokensUsdValue); err != nil { return err } if err := validateAuctionMaxBlockAge(p.AuctionMaxBlockAge); err != nil { return err } if err := validateAuctionPriceDecreaseAccelerationRate(p.AuctionPriceDecreaseAccelerationRate); err != nil { return err } + if err := validateMinimumAuctionHeight(p.MinimumAuctionHeight); err != nil { + return err + } return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (p *Params) ValidateBasic() error { if err := validatePriceMaxBlockAge(p.PriceMaxBlockAge); err != nil { return err } if err := validateMinimumBidInUsomm(p.MinimumBidInUsomm); err != nil { return err } if err := validateMinimumSaleTokensUSDValue(p.MinimumSaleTokensUsdValue); err != nil { return err } if err := validateAuctionMaxBlockAge(p.AuctionMaxBlockAge); err != nil { return err } if err := validateAuctionPriceDecreaseAccelerationRate(p.AuctionPriceDecreaseAccelerationRate); err != nil { return err } if err := validateMinimumAuctionHeight(p.MinimumAuctionHeight); err != nil { return err } return nil }x/incentives/keeper/abci_test.go (1)
108-167:
⚠️ Potential issueInclude Specific Assertions in
TestBeginBlockerSuccessThe test sets up mocks and ensures
BeginBlockerdoes not panic, but it lacks assertions on the expected outcomes. To enhance the test:
- Assert that the validators receive the correct incentive amounts.
- Verify that the community pool balance is updated appropriately after distribution.
- Check that the emitted events match the expected events for successful incentive distribution.
This will confirm that
BeginBlockernot only runs without errors but also produces the intended effects.x/incentives/keeper/incentives_test.go (3)
339-341:
⚠️ Potential issueRemove unnecessary
string()conversions in assertions.As with previous instances, the
event.Attributeskeys and values are already of typestring. Removing the redundantstring()conversions will simplify the assertions.Apply this diff:
-suite.Require().Equal(sdk.AttributeKeyAmount, string(event.Attributes[0].Key)) -suite.Require().Equal(types.AttributeKeyValidator, string(event.Attributes[1].Key)) -suite.Require().Equal(qualifyingVoters[i].Validator.GetOperator().String(), string(event.Attributes[1].Value)) +suite.Require().Equal(sdk.AttributeKeyAmount, event.Attributes[0].Key) +suite.Require().Equal(types.AttributeKeyValidator, event.Attributes[1].Key) +suite.Require().Equal(qualifyingVoters[i].Validator.GetOperator().String(), event.Attributes[1].Value)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.suite.Require().Equal(sdk.AttributeKeyAmount, event.Attributes[0].Key) suite.Require().Equal(types.AttributeKeyValidator, event.Attributes[1].Key) suite.Require().Equal(qualifyingVoters[i].Validator.GetOperator().String(), event.Attributes[1].Value)🧰 Tools
🪛 GitHub Check: golangci-lint
[failure] 339-339:
unnecessary conversion (unconvert)
[failure] 340-340:
unnecessary conversion (unconvert)
[failure] 341-341:
unnecessary conversion (unconvert)
279-281:
⚠️ Potential issueRemove unnecessary
string()conversions in assertions.Similarly, in this section, the
event.Attributeskeys and values are alreadystringtypes. Thestring()conversions can be removed to clean up the code.Apply this diff:
-suite.Require().Equal(newTokens.String(), string(event.Attributes[0].Value)) -suite.Require().Equal(types.AttributeKeyValidator, string(event.Attributes[1].Key)) -suite.Require().Equal(valAddr.String(), string(event.Attributes[1].Value)) +suite.Require().Equal(newTokens.String(), event.Attributes[0].Value) +suite.Require().Equal(types.AttributeKeyValidator, event.Attributes[1].Key) +suite.Require().Equal(valAddr.String(), event.Attributes[1].Value)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.suite.Require().Equal(newTokens.String(), event.Attributes[0].Value) suite.Require().Equal(types.AttributeKeyValidator, event.Attributes[1].Key) suite.Require().Equal(valAddr.String(), event.Attributes[1].Value)🧰 Tools
🪛 GitHub Check: golangci-lint
[failure] 281-281:
unnecessary conversion (unconvert)
[failure] 279-279:
unnecessary conversion (unconvert)
[failure] 280-280:
unnecessary conversion (unconvert)
235-238:
⚠️ Potential issueRemove unnecessary
string()conversions in assertions.The
event.Attributeskeys and values are already of typestring, so converting them usingstring()is redundant. Simplify the assertions by removing the unnecessary conversions.Apply this diff to fix the issue:
-suite.Require().Equal(sdk.AttributeKeyAmount, string(event.Attributes[0].Key)) -suite.Require().Equal(tokens.String(), string(event.Attributes[0].Value)) -suite.Require().Equal(types.AttributeKeyValidator, string(event.Attributes[1].Key)) -suite.Require().Equal(valAddr.String(), string(event.Attributes[1].Value)) +suite.Require().Equal(sdk.AttributeKeyAmount, event.Attributes[0].Key) +suite.Require().Equal(tokens.String(), event.Attributes[0].Value) +suite.Require().Equal(types.AttributeKeyValidator, event.Attributes[1].Key) +suite.Require().Equal(valAddr.String(), event.Attributes[1].Value)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.suite.Require().Equal(sdk.AttributeKeyAmount, event.Attributes[0].Key) suite.Require().Equal(tokens.String(), event.Attributes[0].Value) suite.Require().Equal(types.AttributeKeyValidator, event.Attributes[1].Key) suite.Require().Equal(valAddr.String(), event.Attributes[1].Value)🧰 Tools
🪛 GitHub Check: golangci-lint
[failure] 235-235:
unnecessary conversion (unconvert)
[failure] 238-238:
unnecessary conversion (unconvert)
[failure] 236-236:
unnecessary conversion (unconvert)
[failure] 237-237:
unnecessary conversion (unconvert)integration_tests/incentives_test.go (8)
262-271: 🛠️ Refactor suggestion
Use constants instead of magic numbers for initial rewards
In lines 262-271, the value
4000000is hard-coded when defininginitialRewards. Consider defining this value as a constant or retrieving it from a configuration to enhance readability and maintainability.
281-281: 🛠️ Refactor suggestion
Define 'cutoffHeight' as a constant
At line 281, the
cutoffHeightis set directly to100. Defining this value as a constant (e.g.,const cutoffHeight = 100) improves code clarity and makes it easier to update in the future.
294-294: 🛠️ Refactor suggestion
Replace magic number with constant for 'ValidatorDistributionPerBlock' amount
In line 294, the amount
1000000is hard-coded. Consider defining a constant for this value to improve code readability and ease future adjustments.
403-408: 🛠️ Refactor suggestion
Avoid using hard-coded transaction fee amounts
In lines 403-408, the transaction fee amount
246913560is hard-coded. To enhance maintainability and reduce potential errors if fees change, consider retrieving the actual transaction fees dynamically or defining them as constants.
435-460:
⚠️ Potential issueAdd a timeout to 'waitForBlocks' to prevent indefinite execution
The
waitForBlocksfunction (lines 435-460) lacks a timeout mechanism. If the target block height is not reached due to an error, the test could hang indefinitely. Consider adding a timeout to ensure the test fails gracefully after a reasonable period.
435-460: 🛠️ Refactor suggestion
Consider consolidating 'waitForBlocks' and 'waitUntilHeight' functions
The
waitForBlocks(lines 435-460) andwaitUntilHeight(lines 462-489) functions have similar logic for waiting until a certain block height. Refactoring them into a single function or extracting shared code could reduce duplication and improve maintainability.Also applies to: 462-489
319-319:
⚠️ Potential issueIncorrect format specifier in log message
At line 319, the format specifier
%eis used for an error message. The%everb is intended for floating-point numbers in scientific notation. Use%vor%sto correctly format and display error messages.Apply this diff to fix the format specifier:
-s.T().Logf("error querying proposals: %e", err) +s.T().Logf("error querying proposals: %v", err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.s.T().Logf("error querying proposals: %v", err)
469-479:
⚠️ Potential issueEnhance error handling in 'waitUntilHeight' function
In lines 469-479, the
errorsTotalcounter is used to limit error retries, but specific errors are not logged. Logging the errors will aid in troubleshooting if the function fails to reach the desired block height.Consider modifying the error handling as follows:
... if err != nil { errorsTotal++ + s.T().Logf("Error getting current height: %v", err) continue } ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.errorsTotal := 0 for { if errorsTotal > 5 { return fmt.Errorf("failed to get to height %d: too many errors", height) } currentHeight, err := s.getCurrentHeight(clientCtx) if err != nil { errorsTotal++ s.T().Logf("Error getting current height: %v", err) continue }
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- x/cellarfees/migrations/v1/keeper/query_server.go (1 hunks)
- x/incentives/keeper/abci_test.go (3 hunks)
- x/incentives/keeper/incentives_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- x/cellarfees/migrations/v1/keeper/query_server.go
- x/incentives/keeper/abci_test.go
🧰 Additional context used
🔇 Additional comments (4)
x/incentives/keeper/incentives_test.go (4)
1-11: LGTM: Package declaration and imports are appropriate.The package declaration and imports are well-organized and include all necessary dependencies for testing the incentives keeper.
33-41: LGTM: Well-structured helper function for creating mock validators.The
getMockValidatorsfunction is a well-implemented helper that creates diverse mock validators for testing purposes. It's a good practice to have such utility functions in test files.
92-116: LGTM: Good edge case test forgetValidatorInfos.The
TestGetValidatorInfosNoSignersfunction is a well-implemented test case that verifies the behavior ofgetValidatorInfoswhen no validators have signed the last block. It's important to test such edge cases.
1-359: Overall, well-implemented test suite with room for enhancement.The
incentives_test.gofile provides a comprehensive set of test cases for the incentives keeper. The tests are well-structured, cover various scenarios, and effectively use mock objects. Key strengths include:
- Good coverage of main functionality
- Effective use of the
KeeperTestSuitestructure- Well-implemented helper functions
Areas for improvement:
- Enhance edge case testing, particularly for extreme values and boundary conditions
- Improve error handling tests
- Consider refactoring some utility functions (e.g., consensus keys) into a separate test utilities file
Overall, this is a solid test suite that, with the suggested enhancements, will provide even more robust coverage of the incentives keeper functionality.
| var ( | ||
| // ConsPrivKeys generate ed25519 ConsPrivKeys to be used for validator operator keys | ||
| ConsPrivKeys = []ccrypto.PrivKey{ | ||
| ed25519.GenPrivKey(), | ||
| ed25519.GenPrivKey(), | ||
| ed25519.GenPrivKey(), | ||
| ed25519.GenPrivKey(), | ||
| ed25519.GenPrivKey(), | ||
| } | ||
|
|
||
| // ConsPubKeys holds the consensus public keys to be used for validator operator keys | ||
| ConsPubKeys = []ccrypto.PubKey{ | ||
| ConsPrivKeys[0].PubKey(), | ||
| ConsPrivKeys[1].PubKey(), | ||
| ConsPrivKeys[2].PubKey(), | ||
| ConsPrivKeys[3].PubKey(), | ||
| ConsPrivKeys[4].PubKey(), | ||
| } | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider moving consensus keys to a separate test utilities file.
The global variables ConsPrivKeys and ConsPubKeys are well-defined and useful for testing. However, to improve modularity and potential reuse across different test files, consider moving these to a separate test utilities file.
| func (suite *KeeperTestSuite) TestGetValidatorInfos() { | ||
| ctx, incentivesKeeper := suite.ctx, suite.incentivesKeeper | ||
|
|
||
| // Create mock validators | ||
| validators := suite.getMockValidators() | ||
| validator1, validator2, validator3 := validators[0], validators[1], validators[2] | ||
|
|
||
| consAddr1, err := validator1.GetConsAddr() | ||
| suite.Require().NoError(err) | ||
| consAddr2, err := validator2.GetConsAddr() | ||
| suite.Require().NoError(err) | ||
| consAddr3, err := validator3.GetConsAddr() | ||
| suite.Require().NoError(err) | ||
|
|
||
| // Create mock RequestBeginBlock | ||
| req := abci.RequestBeginBlock{ | ||
| LastCommitInfo: abci.CommitInfo{ | ||
| Votes: []abci.VoteInfo{ | ||
| { | ||
| Validator: abci.Validator{Address: consAddr1, Power: 10}, | ||
| SignedLastBlock: true, | ||
| }, | ||
| { | ||
| Validator: abci.Validator{Address: consAddr2, Power: 20}, | ||
| SignedLastBlock: true, | ||
| }, | ||
| { | ||
| Validator: abci.Validator{Address: consAddr3, Power: 30}, | ||
| SignedLastBlock: false, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| // Set up expectations for the mock StakingKeeper | ||
| suite.stakingKeeper.EXPECT().ValidatorByConsAddr(ctx, consAddr1).Return(validator1) | ||
| suite.stakingKeeper.EXPECT().ValidatorByConsAddr(ctx, consAddr2).Return(validator2) | ||
|
|
||
| // Call the function being tested | ||
| validatorInfos := incentivesKeeper.getValidatorInfos(ctx, req) | ||
|
|
||
| // Assert the results | ||
| suite.Require().Len(validatorInfos, 2) | ||
| suite.Require().Equal(validator1, validatorInfos[0].Validator) | ||
| suite.Require().Equal(int64(10), validatorInfos[0].Power) | ||
| suite.Require().Equal(validator2, validatorInfos[1].Validator) | ||
| suite.Require().Equal(int64(20), validatorInfos[1].Power) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage for TestGetValidatorInfos.
The test case is well-structured and covers the basic scenarios. To improve coverage:
- Add an assertion to verify that the unsigned validator (validator3) is not included in the result.
- Consider adding a test case where all validators have signed the last block.
You can add these assertions after line 89:
suite.Require().NotContains(validatorInfos, validator3)| func (suite *KeeperTestSuite) TestSortValidatorInfosByPower() { | ||
| // Create a slice of ValidatorInfo with unsorted power | ||
| valInfos := []ValidatorInfo{ | ||
| {Power: 30}, | ||
| {Power: 10}, | ||
| {Power: 50}, | ||
| {Power: 20}, | ||
| {Power: 40}, | ||
| } | ||
|
|
||
| // Sort the validator infos | ||
| sortedValInfos := sortValidatorInfosByPower(valInfos) | ||
|
|
||
| // Assert the results | ||
| suite.Require().Len(sortedValInfos, 5) | ||
| suite.Require().Equal(int64(50), sortedValInfos[0].Power) | ||
| suite.Require().Equal(int64(40), sortedValInfos[1].Power) | ||
| suite.Require().Equal(int64(30), sortedValInfos[2].Power) | ||
| suite.Require().Equal(int64(20), sortedValInfos[3].Power) | ||
| suite.Require().Equal(int64(10), sortedValInfos[4].Power) | ||
| } | ||
|
|
||
| func (suite *KeeperTestSuite) TestTruncateVoters() { | ||
| // Create a slice of ValidatorInfo | ||
| valInfos := []ValidatorInfo{ | ||
| {Power: 30}, | ||
| {Power: 10}, | ||
| {Power: 50}, | ||
| {Power: 20}, | ||
| {Power: 40}, | ||
| } | ||
|
|
||
| // Get the truncated voters | ||
| truncatedVoters := truncateVoters(valInfos, 3) | ||
|
|
||
| // Assert the results | ||
| suite.Require().Len(truncatedVoters, 3) | ||
| suite.Require().Equal(int64(30), truncatedVoters[0].Power) | ||
| suite.Require().Equal(int64(10), truncatedVoters[1].Power) | ||
| suite.Require().Equal(int64(50), truncatedVoters[2].Power) | ||
| } | ||
|
|
||
| func (suite *KeeperTestSuite) TestSortValidatorInfosByPowerEmptySlice() { | ||
| // Create an empty slice of ValidatorInfo | ||
| var valInfos []ValidatorInfo | ||
|
|
||
| // Sort the validator infos | ||
| sortedValInfos := sortValidatorInfosByPower(valInfos) | ||
|
|
||
| // Assert the results | ||
| suite.Require().Len(sortedValInfos, 0) | ||
| } | ||
|
|
||
| func (suite *KeeperTestSuite) TestGetTotalPower() { | ||
| // Create a slice of ValidatorInfo | ||
| valInfos := []ValidatorInfo{ | ||
| {Power: 30}, | ||
| {Power: 10}, | ||
| {Power: 50}, | ||
| {Power: 20}, | ||
| {Power: 40}, | ||
| } | ||
|
|
||
| // Get the total power | ||
| totalPower := getTotalPower(&valInfos) | ||
|
|
||
| // Assert the result | ||
| suite.Require().Equal(int64(150), totalPower) | ||
| } | ||
|
|
||
| func (suite *KeeperTestSuite) TestGetTotalPowerEmptySlice() { | ||
| // Create an empty slice of ValidatorInfo | ||
| var valInfos []ValidatorInfo | ||
|
|
||
| // Get the total power | ||
| totalPower := getTotalPower(&valInfos) | ||
|
|
||
| // Assert the result | ||
| suite.Require().Equal(int64(0), totalPower) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance TestTruncateVoters with more test cases.
The utility function tests are well-implemented and cover important scenarios. To improve the TestTruncateVoters function:
- Add test cases with different truncation sizes, including:
- Truncating to a size larger than the input slice
- Truncating to the same size as the input slice
- Truncating to size 1
- Truncating to size 0
Here's an example of how you can add these cases:
func (suite *KeeperTestSuite) TestTruncateVoters() {
valInfos := []ValidatorInfo{
{Power: 30},
{Power: 10},
{Power: 50},
{Power: 20},
{Power: 40},
}
// Existing test case
truncatedVoters := truncateVoters(valInfos, 3)
suite.Require().Len(truncatedVoters, 3)
// ... (existing assertions)
// New test cases
suite.Require().Len(truncateVoters(valInfos, 6), 5) // Larger than input
suite.Require().Len(truncateVoters(valInfos, 5), 5) // Same as input
suite.Require().Len(truncateVoters(valInfos, 1), 1) // Size 1
suite.Require().Len(truncateVoters(valInfos, 0), 0) // Size 0
}| func (suite *KeeperTestSuite) TestAllocateTokensToValidator() { | ||
| ctx, incentivesKeeper := suite.ctx, suite.incentivesKeeper | ||
|
|
||
| // Create a mock validator | ||
| valAddr := sdk.ValAddress([]byte("validatorvalidatorva")) | ||
| validator, err := stakingtypes.NewValidator(valAddr, ConsPubKeys[0], stakingtypes.Description{}) | ||
| suite.Require().NoError(err) | ||
|
|
||
| // Create mock tokens to allocate | ||
| tokens := sdk.NewDecCoins(sdk.NewDecCoin("usom", sdk.NewInt(100))) | ||
|
|
||
| // Set up expectations for the mock DistributionKeeper | ||
| currentRewards := distributiontypes.ValidatorCurrentRewards{Rewards: sdk.DecCoins{}} | ||
| outstandingRewards := distributiontypes.ValidatorOutstandingRewards{Rewards: sdk.DecCoins{}} | ||
|
|
||
| suite.distributionKeeper.EXPECT(). | ||
| GetValidatorCurrentRewards(ctx, valAddr). | ||
| Return(currentRewards) | ||
| suite.distributionKeeper.EXPECT(). | ||
| SetValidatorCurrentRewards(ctx, valAddr, distributiontypes.ValidatorCurrentRewards{Rewards: tokens}) | ||
| suite.distributionKeeper.EXPECT(). | ||
| GetValidatorOutstandingRewards(ctx, valAddr). | ||
| Return(outstandingRewards) | ||
| suite.distributionKeeper.EXPECT(). | ||
| SetValidatorOutstandingRewards(ctx, valAddr, distributiontypes.ValidatorOutstandingRewards{Rewards: tokens}) | ||
|
|
||
| // Call the function being tested | ||
| incentivesKeeper.AllocateTokensToValidator(ctx, validator, tokens) | ||
|
|
||
| // Verify that the event was emitted | ||
| events := ctx.EventManager().Events() | ||
| suite.Require().Len(events, 1) | ||
| event := events[0] | ||
| suite.Require().Equal(types.EventTypeIncentivesRewards, event.Type) | ||
| suite.Require().Len(event.Attributes, 2) | ||
| suite.Require().Equal(sdk.AttributeKeyAmount, event.Attributes[0].Key) | ||
| suite.Require().Equal(tokens.String(), event.Attributes[0].Value) | ||
| suite.Require().Equal(types.AttributeKeyValidator, event.Attributes[1].Key) | ||
| suite.Require().Equal(valAddr.String(), event.Attributes[1].Value) | ||
| } | ||
|
|
||
| func (suite *KeeperTestSuite) TestAllocateTokensToValidatorWithExistingRewards() { | ||
| ctx, incentivesKeeper := suite.ctx, suite.incentivesKeeper | ||
|
|
||
| // Create a mock validator | ||
| valAddr := sdk.ValAddress([]byte("validatorvalidatorva")) | ||
| validator, err := stakingtypes.NewValidator(valAddr, ConsPubKeys[0], stakingtypes.Description{}) | ||
| suite.Require().NoError(err) | ||
|
|
||
| // Create mock tokens to allocate | ||
| existingRewards := sdk.NewDecCoins(sdk.NewDecCoin("usom", sdk.NewInt(50))) | ||
| newTokens := sdk.NewDecCoins(sdk.NewDecCoin("usom", sdk.NewInt(100))) | ||
| expectedTotalRewards := existingRewards.Add(newTokens...) | ||
|
|
||
| // Set up expectations for the mock DistributionKeeper | ||
| currentRewards := distributiontypes.ValidatorCurrentRewards{Rewards: existingRewards} | ||
| outstandingRewards := distributiontypes.ValidatorOutstandingRewards{Rewards: existingRewards} | ||
|
|
||
| suite.distributionKeeper.EXPECT(). | ||
| GetValidatorCurrentRewards(ctx, valAddr). | ||
| Return(currentRewards) | ||
| suite.distributionKeeper.EXPECT(). | ||
| SetValidatorCurrentRewards(ctx, valAddr, distributiontypes.ValidatorCurrentRewards{Rewards: expectedTotalRewards}) | ||
| suite.distributionKeeper.EXPECT(). | ||
| GetValidatorOutstandingRewards(ctx, valAddr). | ||
| Return(outstandingRewards) | ||
| suite.distributionKeeper.EXPECT(). | ||
| SetValidatorOutstandingRewards(ctx, valAddr, distributiontypes.ValidatorOutstandingRewards{Rewards: expectedTotalRewards}) | ||
|
|
||
| // Call the function being tested | ||
| incentivesKeeper.AllocateTokensToValidator(ctx, validator, newTokens) | ||
|
|
||
| // Verify that the event was emitted | ||
| events := ctx.EventManager().Events() | ||
| suite.Require().Len(events, 1) | ||
| event := events[0] | ||
| suite.Require().Equal(types.EventTypeIncentivesRewards, event.Type) | ||
| suite.Require().Len(event.Attributes, 2) | ||
| suite.Require().Equal(sdk.AttributeKeyAmount, event.Attributes[0].Key) | ||
| suite.Require().Equal(newTokens.String(), event.Attributes[0].Value) | ||
| suite.Require().Equal(types.AttributeKeyValidator, event.Attributes[1].Key) | ||
| suite.Require().Equal(valAddr.String(), event.Attributes[1].Value) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and edge cases in allocation tests.
The TestAllocateTokensToValidator and TestAllocateTokensToValidatorWithExistingRewards functions are well-implemented and cover the main scenarios. To improve these tests:
- Add error handling tests:
- Test with invalid validator addresses
- Test with zero or negative token amounts
- Add edge case tests:
- Test with maximum possible token amounts
- Test with very small (but non-zero) token amounts
Here's an example of how you can add an error handling test:
func (suite *KeeperTestSuite) TestAllocateTokensToValidatorInvalidAddress() {
ctx, incentivesKeeper := suite.ctx, suite.incentivesKeeper
invalidValAddr := sdk.ValAddress([]byte("invalidinvalidinva"))
validator, err := stakingtypes.NewValidator(invalidValAddr, ConsPubKeys[0], stakingtypes.Description{})
suite.Require().NoError(err)
tokens := sdk.NewDecCoins(sdk.NewDecCoin("usom", sdk.NewInt(100)))
suite.distributionKeeper.EXPECT().
GetValidatorCurrentRewards(ctx, invalidValAddr).
Return(distributiontypes.ValidatorCurrentRewards{}, sdkerrors.ErrInvalidAddress)
// Expect the function to panic or handle the error appropriately
suite.Require().Panics(func() {
incentivesKeeper.AllocateTokensToValidator(ctx, validator, tokens)
})
}| func (suite *KeeperTestSuite) TestAllocateTokens() { | ||
| ctx, incentivesKeeper := suite.ctx, suite.incentivesKeeper | ||
|
|
||
| // Create mock validators | ||
| validators := suite.getMockValidators() | ||
| validator1, validator2, validator3 := validators[0], validators[1], validators[2] | ||
|
|
||
| // Set up qualifying voters | ||
| qualifyingVoters := []ValidatorInfo{ | ||
| {Validator: validator1, Power: 30}, | ||
| {Validator: validator2, Power: 20}, | ||
| {Validator: validator3, Power: 10}, | ||
| } | ||
|
|
||
| totalPreviousPower := int64(60) | ||
| totalDistribution := sdk.NewDecCoins(sdk.NewDecCoin("usom", sdk.NewInt(100))) | ||
| maxFraction := sdk.NewDecWithPrec(5, 1) // 0.5 | ||
|
|
||
| // Set up expectations for the mock DistributionKeeper | ||
| totalExpectedRewards := sdk.NewDecCoins() | ||
| for _, voter := range qualifyingVoters { | ||
| powerFraction := sdk.NewDecFromInt(sdk.NewInt(voter.Power)).QuoInt64(totalPreviousPower) | ||
| expectedReward := totalDistribution.MulDecTruncate(powerFraction) | ||
| if powerFraction.GT(maxFraction) { | ||
| expectedReward = totalDistribution.MulDecTruncate(maxFraction) | ||
| } | ||
|
|
||
| totalExpectedRewards = totalExpectedRewards.Add(expectedReward...) | ||
|
|
||
| suite.distributionKeeper.EXPECT(). | ||
| GetValidatorCurrentRewards(ctx, voter.Validator.GetOperator()). | ||
| Return(distributiontypes.ValidatorCurrentRewards{Rewards: sdk.DecCoins{}}) | ||
| suite.distributionKeeper.EXPECT(). | ||
| SetValidatorCurrentRewards(ctx, voter.Validator.GetOperator(), distributiontypes.ValidatorCurrentRewards{Rewards: expectedReward}) | ||
| suite.distributionKeeper.EXPECT(). | ||
| GetValidatorOutstandingRewards(ctx, voter.Validator.GetOperator()). | ||
| Return(distributiontypes.ValidatorOutstandingRewards{Rewards: sdk.DecCoins{}}) | ||
| suite.distributionKeeper.EXPECT(). | ||
| SetValidatorOutstandingRewards(ctx, voter.Validator.GetOperator(), distributiontypes.ValidatorOutstandingRewards{Rewards: expectedReward}) | ||
| } | ||
|
|
||
| // Call the function being tested | ||
| remaining := incentivesKeeper.AllocateTokens(ctx, totalPreviousPower, totalDistribution, qualifyingVoters, maxFraction) | ||
|
|
||
| // Verify that the sum of remaining and distributed rewards equals totalDistribution | ||
| totalAllocated := remaining.Add(totalExpectedRewards...) | ||
| suite.Require().Equal(totalDistribution, totalAllocated, "Sum of remaining and distributed rewards should equal total distribution") | ||
|
|
||
| // Verify that events were emitted | ||
| events := ctx.EventManager().Events() | ||
| suite.Require().Len(events, 3) // One event for each validator | ||
| for i, event := range events { | ||
| suite.Require().Equal(types.EventTypeIncentivesRewards, event.Type) | ||
| suite.Require().Len(event.Attributes, 2) | ||
| suite.Require().Equal(sdk.AttributeKeyAmount, event.Attributes[0].Key) | ||
| suite.Require().Equal(types.AttributeKeyValidator, event.Attributes[1].Key) | ||
| suite.Require().Equal(qualifyingVoters[i].Validator.GetOperator().String(), event.Attributes[1].Value) | ||
| } | ||
| } | ||
|
|
||
| func (suite *KeeperTestSuite) TestAllocateTokensNoQualifyingVoters() { | ||
| ctx, incentivesKeeper := suite.ctx, suite.incentivesKeeper | ||
|
|
||
| totalPreviousPower := int64(100) | ||
| totalDistribution := sdk.NewDecCoins(sdk.NewDecCoin("usom", sdk.NewInt(100))) | ||
| maxFraction := sdk.NewDecWithPrec(5, 1) // 0.5 | ||
|
|
||
| // Call the function being tested with empty qualifyingVoters | ||
| remaining := incentivesKeeper.AllocateTokens(ctx, totalPreviousPower, totalDistribution, []ValidatorInfo{}, maxFraction) | ||
|
|
||
| // Verify that all tokens remain unallocated | ||
| suite.Require().Equal(totalDistribution, remaining, "All tokens should remain unallocated when there are no qualifying voters") | ||
|
|
||
| // Verify that no events were emitted | ||
| events := ctx.EventManager().Events() | ||
| suite.Require().Len(events, 0, "No events should be emitted when there are no qualifying voters") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance edge case testing in TestAllocateTokens.
The TestAllocateTokens and TestAllocateTokensNoQualifyingVoters functions are well-implemented and cover the main scenarios. To improve these tests:
-
Add edge case tests:
- Test with a single qualifying voter
- Test with a very large number of qualifying voters
- Test with very small power differences between voters
- Test with extreme
maxFractionvalues (very close to 0 and 1)
-
Consider adding a test case where the total power of qualifying voters exceeds
totalPreviousPower
Here's an example of how you can add an edge case test:
func (suite *KeeperTestSuite) TestAllocateTokensSingleQualifyingVoter() {
ctx, incentivesKeeper := suite.ctx, suite.incentivesKeeper
validator, err := stakingtypes.NewValidator(sdk.ValAddress([]byte("singlevalidatorva")), ConsPubKeys[0], stakingtypes.Description{})
suite.Require().NoError(err)
qualifyingVoters := []ValidatorInfo{
{Validator: validator, Power: 100},
}
totalPreviousPower := int64(100)
totalDistribution := sdk.NewDecCoins(sdk.NewDecCoin("usom", sdk.NewInt(100)))
maxFraction := sdk.OneDec()
// Set up expectations for the mock DistributionKeeper
suite.distributionKeeper.EXPECT().
GetValidatorCurrentRewards(ctx, validator.GetOperator()).
Return(distributiontypes.ValidatorCurrentRewards{Rewards: sdk.DecCoins{}})
suite.distributionKeeper.EXPECT().
SetValidatorCurrentRewards(ctx, validator.GetOperator(), distributiontypes.ValidatorCurrentRewards{Rewards: totalDistribution})
suite.distributionKeeper.EXPECT().
GetValidatorOutstandingRewards(ctx, validator.GetOperator()).
Return(distributiontypes.ValidatorOutstandingRewards{Rewards: sdk.DecCoins{}})
suite.distributionKeeper.EXPECT().
SetValidatorOutstandingRewards(ctx, validator.GetOperator(), distributiontypes.ValidatorOutstandingRewards{Rewards: totalDistribution})
remaining := incentivesKeeper.AllocateTokens(ctx, totalPreviousPower, totalDistribution, qualifyingVoters, maxFraction)
suite.Require().True(remaining.IsZero(), "All tokens should be allocated to the single qualifying voter")
events := ctx.EventManager().Events()
suite.Require().Len(events, 1, "One event should be emitted for the single qualifying voter")
}
* Initial cellarfees v2 migration code * Auction v2 Param migration * Bump gravity dep to v5.0.0 * Minor tweak to upgrade handler * x/incentives v2: Validator incentives (#320) * Add validator distribution fields to incentives Params * Fix help for a cork-result commands * WIP - First attempt at validator incentives allocation * Implement validator rewards in BeginBlocker * Fix validator incentives bug revealed by tests * Unit and integration tests * Fix linter issues * Param name change for clarity. Add event for total val incentive reward per block * Incentives v2 migration * Bump sommelier package to v8 * Additional cork and axelarcork events for easier tracking (#322) * More cork events * Axelarcork events * Fix linting errors * Bump version in go.mod to v8, and include latest gravity-bridge module * Event for relay cork call * Emit event on RelayCorkRequest * Additional unit test for cellarfees * Upgrade README additions * Tweak * Bump Cosmos SDK to 0.47.14
Summary by CodeRabbit
New Features
Bug Fixes
Testing Improvements