feat(x/mint): Add max supply param #24979
Conversation
📝 WalkthroughWalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MintModule
participant BankKeeper
User->>MintModule: BeginBlock (minting event)
MintModule->>BankKeeper: GetSupply(denom)
BankKeeper-->>MintModule: currentSupply
MintModule->>MintModule: Calculate mintAmount
alt maxSupply == 0 or currentSupply + mintAmount <= maxSupply
MintModule->>BankKeeper: MintCoins(mintAmount)
else maxSupply > 0 and currentSupply + mintAmount > maxSupply
MintModule->>BankKeeper: MintCoins(maxSupply - currentSupply)
end
MintModule->>MintModule: Update minter state, emit events
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Note ⚡️ Unit Test Generation - BetaCodeRabbit's unit test generation is now available in Beta! Automatically generate comprehensive unit tests for your code changes, ensuring better test coverage and catching edge cases you might miss. Our AI analyzes your code structure and creates tests that follow best practices and your project's testing patterns. Learn more here, or just try it under ✨ Finishing Touches. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tools/benchmark/CHANGELOG.md (1)
31-31: Fix the formatting inconsistency in the changelog entry.The PR reference is missing the
#symbol before the number, which is inconsistent with the format used in line 30.-* [19896](https://github.com/cosmos/cosmos-sdk/pull/19896) Added a new max supply genesis param to existing params. +* [#19896](https://github.com/cosmos/cosmos-sdk/pull/19896) Added a new max supply genesis param to existing params.x/mint/migrations/v3/migrator_test.go (1)
28-50: Consider explicitly verifying MaxSupply initialization.While the test verifies that parameters are preserved during migration, it would be more robust to explicitly check that the
MaxSupplyfield is properly initialized to zero.Add explicit verification:
require.NoError(t, err) require.NoError(t, cdc.Unmarshal(bz, &res)) require.Equal(t, dp.ps, res) + // Explicitly verify MaxSupply is initialized to zero + require.True(t, res.MaxSupply.IsZero(), "MaxSupply should be zero after migration")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
⛔ Files ignored due to path filters (6)
api/cosmos/mint/v1beta1/mint.pulsar.gois excluded by!**/*.pulsar.gogo.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sumtests/go.modis excluded by!**/*.modx/mint/testutil/expected_keepers_mocks.gois excluded by!**/*_mocks.gox/mint/types/mint.pb.gois excluded by!**/*.pb.go,!**/*.pb.go
📒 Files selected for processing (16)
proto/cosmos/mint/v1beta1/mint.proto(1 hunks)tests/e2e/mint/grpc.go(1 hunks)tools/benchmark/CHANGELOG.md(1 hunks)x/gov/types/config.go(1 hunks)x/mint/README.md(5 hunks)x/mint/keeper/genesis_test.go(1 hunks)x/mint/keeper/migrator.go(2 hunks)x/mint/keeper/mint.go(1 hunks)x/mint/keeper/mint_test.go(1 hunks)x/mint/keeper/msg_server_test.go(1 hunks)x/mint/migrations/v3/migrate.go(1 hunks)x/mint/migrations/v3/migrator_test.go(1 hunks)x/mint/module.go(2 hunks)x/mint/simulation/genesis.go(1 hunks)x/mint/types/expected_keepers.go(1 hunks)x/mint/types/params.go(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
tests/e2e/mint/grpc.go (2)
math/legacy_dec.go (2)
LegacyNewDec(101-103)LegacyNewDecWithPrec(107-112)math/int.go (1)
ZeroInt(192-192)
x/mint/keeper/msg_server_test.go (1)
math/int.go (1)
ZeroInt(192-192)
x/mint/simulation/genesis.go (2)
x/mint/types/params.go (1)
NewParams(14-24)math/int.go (1)
ZeroInt(192-192)
x/mint/keeper/mint.go (1)
types/coin.go (2)
NewCoins(194-201)NewCoin(19-30)
x/mint/keeper/mint_test.go (2)
math/int.go (1)
NewInt(119-121)types/coin.go (1)
NewCoin(19-30)
x/mint/migrations/v3/migrate.go (1)
math/int.go (1)
NewInt(119-121)
x/mint/types/params.go (1)
math/int.go (1)
ZeroInt(192-192)
x/mint/keeper/genesis_test.go (1)
math/int.go (1)
ZeroInt(192-192)
x/mint/keeper/migrator.go (1)
x/mint/migrations/v3/migrate.go (1)
Migrate(21-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (17)
x/gov/types/config.go (1)
3-3: Good catch on spelling correctionDocumentation now consistently uses American English across the module.
x/mint/keeper/genesis_test.go (1)
72-72: LGTM! Correct addition of MaxSupply parameter to genesis test.The addition of
math.ZeroInt()as the MaxSupply parameter aligns with the updatedNewParamsconstructor signature and correctly represents infinite supply for the test scenario.x/mint/types/expected_keepers.go (1)
32-32: LGTM! Well-designed interface extension for supply querying.The new
GetSupplymethod provides a clean interface for the mint module to query current token supply, which is essential for enforcing the maximum supply constraint. The method signature follows Go conventions and returns appropriate types.x/mint/keeper/msg_server_test.go (1)
56-56: LGTM! Clear test update with helpful documentation.The addition of
MaxSupply: sdkmath.ZeroInt()correctly extends the valid parameters test case. The inline comment clarifying that zero represents infinite supply is particularly helpful for code readability.proto/cosmos/mint/v1beta1/mint.proto (1)
62-67: LGTM! Well-defined protobuf field for maximum supply parameter.The new
max_supplyfield is properly defined with:
- Sequential field number (7)
- Appropriate string type with
cosmos.Intscalar annotation- Correct Go custom type mapping to
cosmossdk.io/math.Int- Non-nullable constraint
- Clear documentation
The definition follows established patterns in the codebase and integrates well with the existing parameter structure.
tests/e2e/mint/grpc.go (1)
32-32: LGTM! Test correctly updated for new MaxSupply parameter.The addition of
math.ZeroInt()as the seventh parameter properly reflects the newMaxSupplyfield in the mint module parameters, with zero indicating unlimited supply.x/mint/keeper/mint_test.go (1)
90-92: LGTM! Proper test mocking for new supply checking logic.The addition of
totalSupplyvariable and theGetSupplyexpectation correctly mocks the new functionality where the mint process queries current token supply to enforce maximum supply limits.x/mint/keeper/migrator.go (2)
7-7: LGTM! Proper import for v3 migration package.
32-36: LGTM! Migration method follows established pattern.The
Migrate2to3method correctly follows the same pattern as existing migrations and properly passes all required parameters to the v3 migration function.x/mint/migrations/v3/migrate.go (2)
13-17: LGTM! Proper migration constants defined.The
ModuleNameandParamsKeyconstants are correctly defined for the migration context.
19-39: LGTM! Safe and well-structured migration implementation.The migration correctly:
- Retrieves existing parameters using the collections API
- Sets the new
MaxSupplyfield to zero (unlimited supply)- Validates the updated parameters before storing
- Handles errors appropriately at each step
- Uses the proper codec for marshaling
This follows Cosmos SDK migration best practices.
x/mint/module.go (2)
31-31: LGTM! Consensus version correctly incremented.The consensus version bump from 2 to 3 properly reflects the state-breaking change with the addition of the MaxSupply parameter.
137-139: LGTM! Migration registration follows established pattern.The migration from version 2 to 3 is properly registered with appropriate error handling, following the same pattern as the existing 1to2 migration.
x/mint/simulation/genesis.go (1)
67-67: LGTM!The addition of
math.ZeroInt()correctly updates theNewParamscall to include the newmaxSupplyparameter, maintaining consistency with the updated function signature.x/mint/README.md (1)
143-230: Documentation updates are comprehensive and accurate.The documentation properly describes the new
MaxSupplyparameter, including:
- Clear explanation that
0indicates unlimited supply- Important note about MaxSupply being independent of external burning operations
- Consistent updates across all example outputs
x/mint/keeper/mint.go (1)
54-80: Critical: Duplicate minting logic causes coins to be minted twice.The current implementation has a serious bug where coins are minted twice when the max supply would be exceeded:
- First minting occurs at lines 67-71 when adjusting for max supply
- Second minting occurs at lines 77-79
Additionally:
- The condition on line 75 is redundant since we've already handled the max supply exceeded case
- The
mintedCoinsreassignment (line 70) doesn't updatemintedCoinused for telemetry/eventsApply this fix to correct the minting logic:
- maxSupply := params.MaxSupply - totalSupply := k.bankKeeper.GetSupply(ctx, params.MintDenom).Amount // fetch total supply from the bank module - - // if maxSupply is not infinite, check against max_supply parameter - if !maxSupply.IsZero() { - if totalSupply.Add(mintedCoins.AmountOf(params.MintDenom)).GT(maxSupply) { - // calculate the difference between maxSupply and totalSupply - diff := maxSupply.Sub(totalSupply) - // mint the difference - diffCoin := sdk.NewCoin(params.MintDenom, diff) - diffCoins := sdk.NewCoins(diffCoin) - - // mint coins - if err := k.MintCoins(ctx, diffCoins); err != nil { - return err - } - mintedCoins = diffCoins - } - } - - // mint coins if maxSupply is infinite or total staking supply is less than maxSupply - if maxSupply.IsZero() || totalSupply.Add(mintedCoins.AmountOf(params.MintDenom)).LT(maxSupply) { - // mint coins - if err := k.MintCoins(ctx, mintedCoins); err != nil { - return err - } + maxSupply := params.MaxSupply + totalSupply := k.bankKeeper.GetSupply(ctx, params.MintDenom).Amount + + // if maxSupply is not infinite, check against max_supply parameter + if !maxSupply.IsZero() && totalSupply.Add(mintedCoin.Amount).GT(maxSupply) { + // calculate the difference between maxSupply and totalSupply + diff := maxSupply.Sub(totalSupply) + if diff.IsPositive() { + // update minted amounts to not exceed max supply + mintedCoin = sdk.NewCoin(params.MintDenom, diff) + mintedCoins = sdk.NewCoins(mintedCoin) + } else { + // max supply already reached, don't mint anything + return nil + } + } + + // mint coins + if err := k.MintCoins(ctx, mintedCoins); err != nil { + return err }Likely an incorrect or invalid review comment.
x/mint/types/params.go (1)
14-24: Constructor properly updated for new MaxSupply parameter.The
NewParamsfunction correctly includes the newmaxSupplyparameter and assigns it to the struct field.
| func validateMaxSupply(i interface{}) error { | ||
| v, ok := i.(math.Int) | ||
| if !ok { | ||
| return fmt.Errorf("invalid parameter type: %T", i) | ||
| } | ||
|
|
||
| if v.IsNegative() { | ||
| return fmt.Errorf("max supply must be positive: %d", v) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Fix inconsistent error message in validateMaxSupply.
The error message states "max supply must be positive" but the validation allows zero values (which indicates unlimited supply). This is misleading.
Update the error message to be accurate:
if v.IsNegative() {
- return fmt.Errorf("max supply must be positive: %d", v)
+ return fmt.Errorf("max supply must not be negative: %s", v)
}Also note: Using %s instead of %d for math.Int ensures proper string representation.
📝 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 validateMaxSupply(i interface{}) error { | |
| v, ok := i.(math.Int) | |
| if !ok { | |
| return fmt.Errorf("invalid parameter type: %T", i) | |
| } | |
| if v.IsNegative() { | |
| return fmt.Errorf("max supply must be positive: %d", v) | |
| } | |
| return nil | |
| } | |
| func validateMaxSupply(i interface{}) error { | |
| v, ok := i.(math.Int) | |
| if !ok { | |
| return fmt.Errorf("invalid parameter type: %T", i) | |
| } | |
| if v.IsNegative() { | |
| return fmt.Errorf("max supply must not be negative: %s", v) | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In x/mint/types/params.go lines 177 to 188, the error message for negative max
supply incorrectly states "max supply must be positive" while zero is allowed.
Update the error message to say "max supply must be non-negative" to reflect
this. Also, change the format specifier from %d to %s when printing the math.Int
value to ensure correct string representation.
JayT106
force-pushed
the
jt/mint-max-supply
branch
from
2daf046 to
1826ed7
Compare
Description
backport #19896
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests
Chores