feat(x/mint): Mint issuance using epochs #20044
Conversation
WalkthroughWalkthroughThe changes migrate the Changes
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![github-advanced-security[bot]](https://github.com/avatars/in/57789?s=60&v=4){kind=small}
…ita/mint-issuance
…ita/mint-issuance
…ita/mint-issuance
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (2)
api/cosmos/mint/v1beta1/genesis.pulsar.go (2)
21-21: Add a comment describing thereduction_started_epochfield.Adding a descriptive comment for the new field
reduction_started_epochwill improve code readability and maintainability, especially for new developers or external contributors who might work with this codebase in the future.
602-606: Document thereduction_started_epochfield in theGenesisStatestruct.Add comprehensive documentation for the
reduction_started_epochfield in theGenesisStatestruct to clarify its purpose and usage. This documentation is crucial for developers and users of the SDK to understand the implications of this field.
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
x/mint/README.md (1)
Line range hint
10-26: Correct the indentation of the unordered list items for better readability.- * [State](#state) - * [Minter](#minter) - * [Params](#params) - * [LastReductionEpoch](#lastreductionepoch) - * [Begin Epoch](#begin-epoch) - * [NextEpochProvisions](#nextepochprovisions) - * [EpochProvision](#epochprovision) + * [State](#state) + * [Minter](#minter) + * [Params](#params) + * [LastReductionEpoch](#lastreductionepoch) + * [Begin Epoch](#begin-epoch) + * [NextEpochProvisions](#nextepochprovisions) + * [EpochProvision](#epochprovision)
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- x/mint/README.md (10 hunks)
- x/mint/keeper/migrator.go (2 hunks)
Additional Context Used
LanguageTool (2)
x/mint/README.md (2)
Near line 128: Possible missing preposition found.
Context: ...e desired ratio (67%). The maximum rate change possible is defined to be 13% per year,...
Near line 129: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...e possible is defined to be 13% per year, however, the annual inflation is capped as betwe...
Path-based Instructions (2)
x/mint/keeper/migrator.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/mint/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (3)
x/mint/keeper/migrator.go (2)
Line range hint
39-48: LGTM! The migration logic forMigrate2to3correctly initializes and sets the newMaxSupplyparameter.
51-66: LGTM! The migration logic forMigrate3to4correctly handles theLastReductionEpochparameter, ensuring it is initialized and stored properly.x/mint/README.md (1)
12-15: LGTM! The documentation updates correctly reflect the new epoch-based minting system.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- x/mint/keeper/migrator.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/mint/keeper/migrator.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- x/mint/keeper/hooks.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/mint/keeper/hooks.go
…ita/mint-issuance
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
x/mint/go.modis excluded by!**/*.modx/mint/go.sumis excluded by!**/*.sum
Files selected for processing (1)
- simapp/app.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- simapp/app.go
This reverts commit 3a7479b.
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (5)
x/mint/types/expected_keepers.go (1)
37-40: Add documentation for theEpochsKeeperinterface and its method.To maintain consistency and clarity, consider adding comments to describe the
EpochsKeeperinterface and itsGetEpochInfomethod.x/mint/depinject.go (1)
37-37: Add documentation for theEpochsKeeperfield inModuleInputs.To maintain consistency and clarity, consider adding comments to describe the
EpochsKeeperfield in theModuleInputsstruct.x/mint/keeper/keeper_test.go (1)
53-53: Add documentation for theepochsKeeperfield inIntegrationTestSuite.To maintain consistency and clarity, consider adding comments to describe the
epochsKeeperfield in theIntegrationTestSuitestruct.x/mint/keeper/genesis_test.go (1)
57-57: Add documentation for theepochsKeeperfield inGenesisTestSuite.To maintain consistency and clarity, consider adding comments to describe the
epochsKeeperfield in theGenesisTestSuitestruct.x/mint/testutil/expected_keepers_mocks.go (1)
227-263: Add documentation for theMockEpochsKeeperstruct and its methods.To maintain consistency and clarity, consider adding comments to describe the
MockEpochsKeeperstruct and itsGetEpochInfomethod.
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (9)
- x/epochs/keeper/epoch.go (1 hunks)
- x/mint/depinject.go (2 hunks)
- x/mint/keeper/abci.go (1 hunks)
- x/mint/keeper/genesis_test.go (2 hunks)
- x/mint/keeper/grpc_query_test.go (3 hunks)
- x/mint/keeper/keeper.go (4 hunks)
- x/mint/keeper/keeper_test.go (2 hunks)
- x/mint/testutil/expected_keepers_mocks.go (9 hunks)
- x/mint/types/expected_keepers.go (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- x/mint/keeper/grpc_query_test.go
- x/mint/keeper/keeper.go
Additional Context Used
Path-based Instructions (7)
x/mint/types/expected_keepers.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/mint/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/epochs/keeper/epoch.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/mint/keeper/keeper_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/mint/keeper/genesis_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/mint/keeper/abci.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/mint/testutil/expected_keepers_mocks.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (7)
x/mint/depinject.go (1)
71-71: Ensure proper error handling for theEpochsKeeperinitialization.Verify that the
EpochsKeeperis correctly initialized and handled in all relevant parts of the codebase.x/epochs/keeper/epoch.go (1)
10-18: Ensure proper error handling and logging for theGetEpochInfomethod.Consider adding logging for error cases to aid in debugging and monitoring.
x/mint/keeper/keeper_test.go (1)
64-64: Ensure proper error handling for theepochsKeeperinitialization.Verify that the
epochsKeeperis correctly initialized and handled in all relevant parts of the test suite.x/mint/keeper/genesis_test.go (2)
62-62: Ensure proper error handling for theepochsKeeperinitialization.Verify that the
epochsKeeperis correctly initialized and handled in all relevant parts of the test suite.
Line range hint
67-82: Ensure comprehensive test coverage for the new genesis parameters.Verify that the new genesis parameters related to epochs are thoroughly tested, including edge cases and error scenarios.
x/mint/keeper/abci.go (2)
14-18: Ensure proper error handling and logging for theBeginBlocker2function.Consider adding logging for error cases to aid in debugging and monitoring.
40-126: Ensure proper error handling and logging for theBeginBlockerfunction.Consider adding logging for error cases to aid in debugging and monitoring.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (4)
- x/mint/keeper/abci.go (1 hunks)
- x/mint/keeper/genesis_test.go (2 hunks)
- x/mint/keeper/grpc_query_test.go (1 hunks)
- x/mint/keeper/keeper.go (3 hunks)
Files skipped from review as they are similar to previous changes (4)
- x/mint/keeper/abci.go
- x/mint/keeper/genesis_test.go
- x/mint/keeper/grpc_query_test.go
- x/mint/keeper/keeper.go
There was a problem hiding this comment.
Actionable comments posted: 2
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (12)
- x/mint/README.md (9 hunks)
- x/mint/depinject.go (2 hunks)
- x/mint/keeper/genesis_test.go (2 hunks)
- x/mint/keeper/grpc_query_test.go (2 hunks)
- x/mint/keeper/keeper.go (5 hunks)
- x/mint/keeper/keeper_test.go (3 hunks)
- x/mint/module.go (5 hunks)
- x/mint/simulation/genesis_test.go (1 hunks)
- x/mint/testutil/expected_keepers_mocks.go (1 hunks)
- x/mint/types/expected_keepers.go (1 hunks)
- x/mint/types/genesis.go (1 hunks)
- x/mint/types/minter.go (3 hunks)
Files skipped from review as they are similar to previous changes (8)
- x/mint/depinject.go
- x/mint/keeper/genesis_test.go
- x/mint/keeper/grpc_query_test.go
- x/mint/keeper/keeper.go
- x/mint/module.go
- x/mint/types/expected_keepers.go
- x/mint/types/genesis.go
- x/mint/types/minter.go
Additional Context Used
Path-based Instructions (4)
x/mint/keeper/keeper_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/mint/simulation/genesis_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"x/mint/testutil/expected_keepers_mocks.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/mint/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (9)
x/mint/keeper/keeper_test.go (2)
30-33: Initialization ofmintKeeper,ctx,msgServer, andbankKeeperlooks good.
50-61: TheTestAliasFunctionsfunction correctly sets up expectations and verifies the behavior of the alias functions.x/mint/simulation/genesis_test.go (2)
Line range hint
17-37: TheTestRandomizedGenStatefunction correctly initializes the simulation state and verifies the genesis state parameters.
Line range hint
40-57: TheTestRandomizedGenState1function correctly sets up scenarios that should panic and verifies the panic messages.x/mint/testutil/expected_keepers_mocks.go (2)
Line range hint
10-41: TheMockAccountKeeperstruct and methods correctly simulate the behavior of theAccountKeeperinterface.
Line range hint
43-79: TheMockBankKeeperstruct and methods correctly simulate the behavior of theBankKeeperinterface.x/mint/README.md (3)
74-78: The description ofLastReductionEpochis clear and concise.
80-97: TheBegin-Epochsection provides a clear explanation of the epoch-based minting mechanism.
156-174: The descriptions and examples of the new query commands are clear and well-documented.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- simapp/app.go (3 hunks)
- tests/integration/example/example_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- simapp/app.go
Additional Context Used
Path-based Instructions (1)
tests/integration/example/example_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
Additional comments not posted (1)
tests/integration/example/example_test.go (1)
69-70: EnsuremintKeeperinitialization includes all necessary dependencies.The
mintKeeperinitialization should include all required dependencies, such as theEpochsKeeper, if applicable. Verify that theEpochsKeeperis correctly integrated if it is part of the new epoch-based minting mechanism.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (5)
- simapp/app.go (4 hunks)
- x/mint/autocli.go (1 hunks)
- x/mint/keeper/grpc_query.go (3 hunks)
- x/mint/proto/cosmos/mint/v1beta1/mint.proto (2 hunks)
- x/mint/proto/cosmos/mint/v1beta1/query.proto (3 hunks)
Files skipped from review as they are similar to previous changes (5)
- simapp/app.go
- x/mint/autocli.go
- x/mint/keeper/grpc_query.go
- x/mint/proto/cosmos/mint/v1beta1/mint.proto
- x/mint/proto/cosmos/mint/v1beta1/query.proto
| @@ -473,7 +477,7 @@ func NewSimApp( | |||
| // CanWithdrawInvariant invariant. | |||
| // NOTE: staking module is required if HistoricalEntries param > 0 | |||
| app.ModuleManager.SetOrderBeginBlockers( | |||
| minttypes.ModuleName, | |||
| // minttypes.ModuleName, | |||
There was a problem hiding this comment.
| // minttypes.ModuleName, |
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (3)
api/cosmos/mint/v1beta1/query_grpc.pb.gois excluded by!**/*.pb.gox/mint/types/mint.pb.gois excluded by!**/*.pb.gox/mint/types/query.pb.gois excluded by!**/*.pb.go
Files selected for processing (3)
- api/cosmos/mint/v1beta1/mint.pulsar.go (28 hunks)
- api/cosmos/mint/v1beta1/query.pulsar.go (45 hunks)
- x/mint/proto/cosmos/mint/v1beta1/mint.proto (2 hunks)
Files not summarized due to errors (2)
- api/cosmos/mint/v1beta1/mint.pulsar.go: Error: Message exceeds token limit
- api/cosmos/mint/v1beta1/query.pulsar.go: Error: Message exceeds token limit
Files skipped from review as they are similar to previous changes (1)
- x/mint/proto/cosmos/mint/v1beta1/mint.proto
Additional Context Used
Path-based Instructions (2)
api/cosmos/mint/v1beta1/mint.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.api/cosmos/mint/v1beta1/query.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (16)
api/cosmos/mint/v1beta1/mint.pulsar.go (6)
22-22: New field descriptorfd_Minter_epoch_provisionsadded forepoch_provisionsin theMinterstruct.
30-30: Initialization of the new field descriptorfd_Minter_epoch_provisionsis correctly placed.
110-115: Handling of the newepoch_provisionsfield in theRangemethod is correctly implemented.
135-136: Handling of the newepoch_provisionsfield in theHasmethod is correctly implemented.
157-158: Handling of the newepoch_provisionsfield in theClearmethod is correctly implemented.
181-183: Handling of the newepoch_provisionsfield in theGetmethod is correctly implemented.api/cosmos/mint/v1beta1/query.pulsar.go (10)
1242-1243: LGTM! TheRangefunction correctly iterates over the fields and calls the provided functionf.
1264-1264: LGTM! TheHasfunction correctly checks if the field is populated.
1282-1282: LGTM! TheClearfunction correctly clears the value of the field.
1301-1301: LGTM! TheGetfunction correctly retrieves the value of the field.
1323-1323: LGTM! TheSetfunction correctly sets the value of the field.
1360-1360: LGTM! TheNewFieldfunction correctly returns a new value assignable to the field.
Line range hint
1523-1574: LGTM! TheUnmarshalfunction correctly unmarshals data into the message.
1599-1601: LGTM! TheProtoReflectfunction correctly returns the message reflection interface.
1659-1660: LGTM! TheRangefunction correctly iterates over the fields and calls the provided functionf.
1674-1681: LGTM! TheHasfunction correctly checks if the field is populated.
x/mint/keeper/hooks.go
Outdated
| } | ||
|
|
||
| // mint coins, update supply | ||
| mintedCoin := minter.EpochProvision(params) |
There was a problem hiding this comment.
how is this calculated? it should still be a function of how much is staked no? this change might be too fundamental for chains to adopt. Also it changes an economic design. We should reconsider this change and possibly revert it
x/mint/keeper/keeper.go
Outdated
| // StakingTokenSupply implements an alias call to the underlying staking keeper's | ||
| // StakingTokenSupply to be used in BeginBlocker. | ||
| func (k Keeper) StakingTokenSupply(ctx context.Context) (math.Int, error) { | ||
| return k.stakingKeeper.StakingTokenSupply(ctx) | ||
| } | ||
|
|
||
| // BondedRatio implements an alias call to the underlying staking keeper's | ||
| // BondedRatio to be used in BeginBlocker. | ||
| func (k Keeper) BondedRatio(ctx context.Context) (math.LegacyDec, error) { | ||
| return k.stakingKeeper.BondedRatio(ctx) | ||
| } | ||
|
|
There was a problem hiding this comment.
why was this removed? minting should still be a function of some supply
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (8)
- simapp/app.go (3 hunks)
- x/mint/depinject.go (2 hunks)
- x/mint/keeper/genesis_test.go (2 hunks)
- x/mint/keeper/grpc_query_test.go (1 hunks)
- x/mint/keeper/hooks.go (1 hunks)
- x/mint/keeper/keeper.go (5 hunks)
- x/mint/keeper/keeper_test.go (1 hunks)
- x/mint/types/minter.go (4 hunks)
Files skipped from review as they are similar to previous changes (8)
- simapp/app.go
- x/mint/depinject.go
- x/mint/keeper/genesis_test.go
- x/mint/keeper/grpc_query_test.go
- x/mint/keeper/hooks.go
- x/mint/keeper/keeper.go
- x/mint/keeper/keeper_test.go
- x/mint/types/minter.go
|
closing in favour of #20363 for now. We may come back to this if needed |
Description
Closes: #19952
Note: This work is referenced from osmosis
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Tests
Chores