[Mint] 🤑 New token model implementation #439
Conversation
bdeneux
force-pushed
the
feat/new-token-model
branch
2 times, most recently
from
61b966a to
6f10d0d
Compare
Codecov Report
@@ Coverage Diff @@
## main #439 +/- ##
==========================================
- Coverage 64.94% 64.72% -0.22%
==========================================
Files 62 61 -1
Lines 2707 2727 +20
==========================================
+ Hits 1758 1765 +7
- Misses 870 883 +13
Partials 79 79
|
bdeneux
force-pushed
the
feat/new-token-model
branch
2 times, most recently
from
030fbe2 to
b81906b
Compare
|
@coderabbitai review |
|
Note Reviews PausedUse the following commands to manage reviews:
WalkthroughThe changes revolve around a significant update to the token model, particularly within the Changes
Assessment against linked issues
Poem
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 X ? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Configuration used: CodeRabbit UI
Files ignored due to filter (3)
- proto/mint/docs.yaml
- x/mint/migrations/v3/types/minter.pb.go
- x/mint/types/mint.pb.go
Files selected for processing (23)
- Makefile (2 hunks)
- app/app.go (1 hunks)
- app/upgrades.go (3 hunks)
- app/upgrades/v6/upgrade.go (1 hunks)
- docs/proto/mint.md (2 hunks)
- proto/mint/v1beta1/mint.proto (1 hunks)
- x/mint/abci.go (2 hunks)
- x/mint/client/cli/query_test.go (1 hunks)
- x/mint/keeper/genesis_test.go (2 hunks)
- x/mint/keeper/keeper_test.go (1 hunks)
- x/mint/keeper/migrations.go (1 hunks)
- x/mint/keeper/msg_server_test.go (2 hunks)
- x/mint/migrations/v3/migrations.go (1 hunks)
- x/mint/migrations/v3/types/params.go (1 hunks)
- x/mint/module.go (4 hunks)
- x/mint/simulation/decoder_test.go (1 hunks)
- x/mint/simulation/genesis.go (2 hunks)
- x/mint/simulation/genesis_test.go (1 hunks)
- x/mint/simulation/proposals.go (1 hunks)
- x/mint/simulation/proposals_test.go (1 hunks)
- x/mint/types/minter.go (2 hunks)
- x/mint/types/minter_test.go (6 hunks)
- x/mint/types/params.go (3 hunks)
Files skipped from review due to trivial changes (8)
- Makefile
- app/app.go
- proto/mint/v1beta1/mint.proto
- x/mint/client/cli/query_test.go
- x/mint/keeper/genesis_test.go
- x/mint/keeper/migrations.go
- x/mint/module.go
- x/mint/simulation/decoder_test.go
Additional comments: 39
app/upgrades/v6/upgrade.go (1)
- 1-30: The upgrade handler
CreateUpgradeHandleris correctly implemented to run the migrations for the modules. TheStoreUpgradesvariable correctly lists the new store "feeibc" that has been added in this upgrade. The logger is used appropriately to log the upgrade process. However, the comment on line 19 mentions that the mint module is migrated with new parameters, but there is no explicit code in this hunk that does this. Ensure that the mint module's migration is handled in themm.RunMigrationsmethod or elsewhere in the codebase.app/upgrades.go (3)
12-12: The import for the v6 upgrade package has been added. Ensure that the package exists and is accessible from this location.
31-34: A new upgrade handler for v6 has been set. Ensure that the
CreateUpgradeHandlerfunction in the v6 package is implemented correctly and that it accepts the correct parameters.53-54: The
storeUpgradesvariable is being set tov6.StoreUpgradeswhen the upgrade name isv6.UpgradeName. Ensure thatv6.StoreUpgradesis defined and correctly implemented in the v6 package.docs/proto/mint.md (3)
9-43: The new documentation provides a clear explanation of the new inflation calculation method, including the formula used and the meaning of each variable. It also explains how the yearly inflation rate is converted into a block-level token minting amount. This is a significant change from the previous static inflation calculation method, which should be noted for users of the system.
81-94: The
Mintersection has been updated to reflect the new dynamic inflation calculation method. Thetarget_supplyfield has been removed, which is consistent with the removal of thetotalSupplyparameter from theBlockProvisionfunction.99-107: The
Paramssection has been updated with new fields for the inflation coefficient, bonding adjustment, and target bonding ratio. Theannual_reduction_factorfield has been removed, which is consistent with the changes to the inflation calculation method.x/mint/keeper/keeper_test.go (1)
- 80-117: The test cases for setting invalid and valid parameters have been updated to include the new parameters
BondingAdjustment,TargetBondingRatio, andInflationCoef. The test cases cover scenarios where these parameters are set to negative values, which are expected to cause errors, and scenarios where they are set to valid values. The test cases are comprehensive and cover all the new parameters. The changes are consistent with the updatedParamsmessage in themintmodule.x/mint/keeper/msg_server_test.go (4)
23-35: The test case "set invalid params for bonding adjustment (negative value)" is correctly checking for an error when a negative value is set for the
BondingAdjustmentparameter. This is a good practice as it ensures that the system behaves as expected when invalid input is provided.37-49: The test case "set invalid params for target bonding ratio (negative value)" is correctly checking for an error when a negative value is set for the
TargetBondingRatioparameter. This is a good practice as it ensures that the system behaves as expected when invalid input is provided.51-59: The test case "set invalid params for inflation coef (negative value)" is correctly checking for an error when a negative value is set for the
InflationCoefparameter. This is a good practice as it ensures that the system behaves as expected when invalid input is provided.69-73: The test case for setting valid parameters is correctly checking that no error is returned when valid values are set for the
BondingAdjustment,TargetBondingRatio, andInflationCoefparameters. This is a good practice as it ensures that the system behaves as expected when valid input is provided.x/mint/types/minter.go (5)
13-17: The
NewMinterfunction no longer acceptstargetSupplyas a parameter. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.21-26: The
InitialMinterfunction no longer acceptstargetSupplyas a parameter. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.30-32: The default inflation rate has been changed from 15% to 18%. Ensure that this change is intentional and that it does not negatively impact the system.
45-48: The
NextInflationfunction now calculates the inflation rate differently, taking into account the bonding ratio and the inflation coefficient. Ensure that this new calculation method is correct and that it provides the expected results.59-60: The
BlockProvisionfunction no longer acceptstotalSupplyas a parameter and no longer checks if the future supply exceeds the target supply. Ensure that all calls to this function throughout the codebase have been updated to match the new signature and that the removal of the future supply check does not introduce any issues.x/mint/types/params.go (2)
13-32: The
NewParamsandDefaultParamsfunctions have been updated to include new parameters:inflationCoef,bondingAdjustment, andtargetBondingRatio. Ensure that these new parameters are correctly used throughout the codebase and that their default values are appropriate.41-47: The
Validatefunction has been updated to validate the new parameters. This is a good practice to ensure that the parameters are within the expected range and format.x/mint/types/minter_test.go (6)
18-32: The test cases for
NextInflationhave been updated to reflect the new inflation calculation method. The new test cases consider different bonded ratios and their expected inflation rates. Ensure that the expected inflation rates are calculated correctly according to the new inflation calculation method.45-54: The test cases for
BlockProvisionhave been simplified. ThetotalSupplyandtargetSupplyparameters are no longer considered in the test cases. Ensure that this change is consistent with the updatedBlockProvisionfunction.65-85: A new test function
TestNextAnnualProvisionhas been added. This function tests theNextAnnualProvisionsfunction with different inflation rates and total supplies. Ensure that the expected annual provisions are calculated correctly according to the new annual provision calculation method.101-128: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [95-105]
The benchmark function
BenchmarkBlockProvisionhas been updated. ThetotalSupplyparameter is no longer considered in the benchmark. Ensure that this change is consistent with the updatedBlockProvisionfunction.
112-119: The benchmark function
BenchmarkNextInflationhas been updated to consider a bonded ratio. Ensure that this change is consistent with the updatedNextInflationfunction.125-125: The benchmark function
BenchmarkNextAnnualProvisionshas been updated. ThetargetSupplyparameter is no longer considered in the benchmark. Ensure that this change is consistent with the updatedNextAnnualProvisionsfunction.x/mint/simulation/proposals_test.go (1)
- 46-46: Ensure that the
MintDenomparameter is correctly set to "xKGLwQvuyN" throughout the codebase.x/mint/migrations/v3/types/params.go (6)
20-28: The function
NewParamsis used to create a newParamsobject. It takes three parameters:mintDenomwhich is a string,annualReductionFactorwhich is of typesdk.Dec, andblocksPerYearwhich is of typeuint64. The function returns aParamsobject with these values set. This function is straightforward and does not seem to have any issues.31-37: The function
DefaultParamsis used to create aParamsobject with default values. TheMintDenomis set tosdk.DefaultBondDenom, theAnnualReductionFactoris set to20%per year, andBlocksPerYearis set assuming 5-second block times. This function is straightforward and does not seem to have any issues.40-49: The
Validatefunction is used to validate theParamsobject. It checks if theMintDenom,AnnualReductionFactor, andBlocksPerYearare valid. If any of these checks fail, it returns an error. This function is straightforward and does not seem to have any issues.58-68: The function
validateMintDenomis used to validate theMintDenomparameter. It checks if the parameter is of type string and if it is not blank. If these checks pass, it validates the denom usingsdk.ValidateDenom. If any of these checks fail, it returns an error. This function is straightforward and does not seem to have any issues.71-84: The function
validateAnnualReductionFactoris used to validate theAnnualReductionFactorparameter. It checks if the parameter is of typesdk.Dec, if it is not negative, and if it is not greater than 1. If any of these checks fail, it returns an error. This function is straightforward and does not seem to have any issues.86-97: The function
validateBlocksPerYearis used to validate theBlocksPerYearparameter. It checks if the parameter is of typeuint64and if it is not zero. If any of these checks fail, it returns an error. This function is straightforward and does not seem to have any issues.x/mint/simulation/genesis.go (3)
18-22: The simulation parameter constants have been updated to reflect the new parameters introduced in the mint module. The
AnnualReductionFactorconstant has been replaced withInflationCoef,BondingAdjustment, andTargetBondingRatio. Ensure that these new constants are used correctly throughout the codebase.57-72: The
RandomizedGenStatefunction has been updated to generate random values for the new parameters. TheGetOrGeneratefunction is used to either get the existing value for a parameter or generate a new one. This is done forInflationCoef,TargetBondingRatio, andBondingAdjustment. The generated values are then used to create a newParamsobject.76-76: The
NewParamsfunction has been updated to include the new parameters. Ensure that the function signature has been updated everywhere it is used.x/mint/simulation/genesis_test.go (2)
52-54: The test now checks the new parameters
InflationCoef,BondingAdjustment, andTargetBondingRatioin themintGenesis.Paramsstruct. Ensure these values are correctly set in theRandomizedGenStatefunction.60-60: The
NextInflationfunction now takes an additional argument,sdk.NewDecWithPrec(66, 2). Ensure that this change is reflected in all calls toNextInflationthroughout the codebase.x/mint/abci.go (2)
- 21-26: The new inflation calculation now considers the total token supply, inflation coefficient, bonding ratio adjustment coefficient, current bonding ratio, and target bonding ratio. The
BeginBlockerfunction has been refactored to recalculate the inflation rate and calculate the bonded ratio. TheBlockProvisionfunction now operates without thetotalSupplyparameter. This change simplifies the code and makes it easier to understand. However, ensure that the new inflation calculation logic is correct and that it doesn't introduce any potential issues or bugs.- if uint64(ctx.BlockHeight()) == 1 { - minter.AnnualProvisions = minter.NextAnnualProvisions(params, totalSupply) - minter.TargetSupply = totalSupply.Add(minter.AnnualProvisions.TruncateInt()) - k.SetMinter(ctx, minter) - } - - // If we have reached the end of the year by reaching the targeted supply for the year - // We need to re-calculate the next inflation for the next year. - if totalSupply.GTE(minter.TargetSupply) { - minter.Inflation = minter.NextInflation(params) - minter.AnnualProvisions = minter.NextAnnualProvisions(params, totalSupply) - minter.TargetSupply = totalSupply.Add(minter.AnnualProvisions.TruncateInt()) - k.SetMinter(ctx, minter) - } + // recalculate inflation rate + totalSupply := k.TokenSupply(ctx, params.MintDenom) + bondedRatio := k.BondedRatio(ctx) + minter.Inflation = minter.NextInflation(params, bondedRatio) + minter.AnnualProvisions = minter.NextAnnualProvisions(params, totalSupply) + k.SetMinter(ctx, minter)Committable suggestion (Beta)
// recalculate inflation rate totalSupply := k.TokenSupply(ctx, params.MintDenom) bondedRatio := k.BondedRatio(ctx) minter.Inflation = minter.NextInflation(params, bondedRatio) minter.AnnualProvisions = minter.NextAnnualProvisions(params, totalSupply) k.SetMinter(ctx, minter)
- 50-50: The addition of the
AttributeKeyBondedRatioattribute to the event provides more information about the state of the system at the time of minting. This could be useful for debugging or for tracking the state of the system over time.+ sdk.NewAttribute(types.AttributeKeyBondedRatio, bondedRatio.String()),Committable suggestion (Beta)
sdk.NewAttribute(types.AttributeKeyBondedRatio, bondedRatio.String()),
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- x/mint/types/mint.pb.go
Files selected for processing (16)
- proto/mint/v1beta1/mint.proto (1 hunks)
- x/logic/predicate/uri.go (1 hunks)
- x/mint/abci.go (2 hunks)
- x/mint/client/cli/query_test.go (1 hunks)
- x/mint/keeper/genesis_test.go (2 hunks)
- x/mint/keeper/keeper_test.go (1 hunks)
- x/mint/keeper/msg_server_test.go (2 hunks)
- x/mint/simulation/genesis.go (2 hunks)
- x/mint/simulation/genesis_test.go (1 hunks)
- x/mint/simulation/proposals.go (1 hunks)
- x/mint/simulation/proposals_test.go (1 hunks)
- x/mint/types/errors.go (1 hunks)
- x/mint/types/genesis.go (1 hunks)
- x/mint/types/minter.go (1 hunks)
- x/mint/types/minter_test.go (3 hunks)
- x/mint/types/params.go (2 hunks)
Files skipped from review due to trivial changes (2)
- x/logic/predicate/uri.go
- x/mint/client/cli/query_test.go
Additional comments: 28
proto/mint/v1beta1/mint.proto (4)
17-29: The summary incorrectly states that the
annual_provisionsfield was renamed toinflation. Both fields are present and distinct in theMintermessage.32-45: The summary incorrectly states that the
annual_reduction_factorfield was renamed toinflation_coef. There is noannual_reduction_factorfield present in the hunk, andinflation_coefappears to be a new addition.32-45: The summary incorrectly states that the
blocks_per_yearfield was renamed toEstimatedBlocksPerYear. The field name remainsblocks_per_yearin the hunk.32-45: The summary incorrectly mentions
mint_denomas a new field in theParamsmessage, but it is not new and remains unchanged.x/mint/abci.go (2)
14-38: The refactoring of the
BeginBlockerfunction to include the new inflation calculation logic and the removal of the block height condition for minter data update align with the PR objectives. The recalculated inflation rate and bonded ratio each block, as well as the refactored minter creation and setting logic, are consistent with the intended changes. The error handling with panic is appropriate given the context of a blockchain application where such errors are critical.50-56: The event emission in the
BeginBlockerfunction has been updated to include new attributes related to the bonded ratio, inflation, and annual provisions, which aligns with the PR objectives. This change enhances the transparency and traceability of minting operations.x/mint/keeper/genesis_test.go (2)
- 6-11: > 💡 NOTE
Codebase verification is a beta feature.
The removal of the
"cosmossdk.io/math"import fromx/mint/keeper/genesis_test.godoes not appear to cause issues within that file, as no other parts of the file were using it. However, the import is still in use in various other files within the codebase. Ensure that the removal does not affect the functionality of those files where the import remains.
- 61-61: The change in the
NewMinterfunction call in hunk 1 is consistent with the summary and the PR objectives, which indicate an update to the minting module's logic.x/mint/keeper/keeper_test.go (2)
77-94: The hunk shows updates to the
InflationCoeffield within thetypes.Paramsstructure in theTestParamsfunction. However, the summary mentions changes to theAnnualReductionFactorfield, which is not present in the hunk. Please verify if the summary is accurate or if there are additional changes not reflected in the hunk.82-82: The
BlocksPerYearfield is used in the test cases, but according to the PR objectives, it should have been renamed toEstimatedBlocksPerYear. Please confirm if the field name change is correctly implemented across the codebase.x/mint/keeper/msg_server_test.go (3)
23-32: The test case "set invalid params for inflation coef (negative value)" correctly sets up a scenario to test the system's behavior with a negative inflation coefficient, which should be invalid according to the PR objectives. This is a good test to ensure that the new validation logic is working as expected.
36-44: The test case "set full valid params" is designed to test the system's behavior with a set of valid parameters, including a positive
InflationCoef. This test case is important to confirm that the system accepts valid configurations and the new parameter fields are functioning correctly.29-29: The PR summary mentions that the
BlocksPerYearfield has been renamed toEstimatedBlocksPerYear, but the code still usesBlocksPerYear. This inconsistency should be resolved to match the PR objectives and the updated codebase.Also applies to: 41-41
x/mint/simulation/genesis.go (2)
62-62: The
BlocksPerYearfield should be renamed toEstimatedBlocksPerYearto align with the PR objectives.62-62: The summary does not mention the renaming of
BlocksPerYeartoEstimatedBlocksPerYear, which is inconsistent with the PR objectives.x/mint/simulation/genesis_test.go (1)
- 44-58: The test assertions have been updated to reflect the new token model implementation, including the use of
NewMinterWithInflationCoefto initialize theminterwithinflationCoefandbondedRatio. The assertions check theBlocksPerYear,InflationCoef,MintDenom,BlockProvision,Inflation, andAnnualProvisionsof bothmintGenesisandminterobjects, which align with the PR objectives and the new token model.x/mint/simulation/proposals.go (2)
37-42: The
Paramsstructure in the hunk still usesBlocksPerYearinstead ofEstimatedBlocksPerYearas mentioned in the summary. Please verify if the field should be renamed or if the summary contains outdated information.37-42: The changes to the
Paramsstructure and theSimulateMsgUpdateParamsfunction align with the PR objectives and the provided summary, indicating a shift in the economic model or inflation calculation logic.x/mint/simulation/proposals_test.go (2)
42-42: The summary indicates that the
BlocksPerYearfield has been renamed toEstimatedBlocksPerYear, but the hunk still usesBlocksPerYear. Please verify if the field name in the code should be updated to match the summary or if the summary contains outdated information.41-44: The test assertions for
MsgUpdateParamsparameters appear to be correct and align with the PR objectives of updating the mint module with a new token model.x/mint/types/genesis.go (1)
- 26-26: The change from a standalone validation function to a method on the
Mintertype is consistent with the PR objectives and the provided summary. It simplifies the validation process and encapsulates the logic within theMintertype.x/mint/types/minter.go (1)
- 10-71: The changes in the
x/mint/types/minter.gofile align with the PR objectives and the summary of changes. The function signatures and logic have been updated to reflect the new token model, and the code is consistent with the described updates.x/mint/types/minter_test.go (3)
16-80: The refactoring of
TestNextInflationto use theConveyfunction and the table-driven test cases align with best practices for readability and maintainability. The error handling within the test cases is consistent with the PR objectives.87-93: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [87-101]
The benchmark function
BenchmarkBlockProvisionhas been updated to useNewMinterWithInitialInflationfor creating the minter instance, which is consistent with the PR summary. The use of random values forAnnualProvisionsis appropriate for benchmarking purposes.
- 105-119: The benchmark function
BenchmarkNextInflationhas been updated to useNewMinterWithInflationCoef, which is consistent with the PR summary. The panic on error is acceptable in the context of a benchmark test, as it indicates a critical failure that should not be ignored.x/mint/types/params.go (3)
13-30: The changes to the
NewParamsandDefaultParamsfunctions align with the PR objectives to update the parameter configuration with the new inflation model.33-41: The update to the
Validatefunction to usevalidateInflationCoefinstead ofvalidateAnnualReductionFactoris in line with the PR objectives and the summary provided.64-79: The
validateInflationCoeffunction correctly checks for negative values and values greater than one, which is appropriate for an inflation coefficient.
ccamel
force-pushed
the
feat/new-token-model
branch
from
bfdf5d1 to
3c80c97
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (4)
- go.mod
- proto/mint/docs.yaml
- x/mint/migrations/v3/types/minter.pb.go
- x/mint/types/mint.pb.go
Files selected for processing (25)
- Makefile (2 hunks)
- app/app.go (1 hunks)
- app/upgrades.go (3 hunks)
- app/upgrades/v6/upgrade.go (1 hunks)
- docs/proto/mint.md (2 hunks)
- proto/mint/v1beta1/mint.proto (1 hunks)
- x/mint/abci.go (2 hunks)
- x/mint/client/cli/query_test.go (1 hunks)
- x/mint/keeper/genesis_test.go (2 hunks)
- x/mint/keeper/keeper_test.go (1 hunks)
- x/mint/keeper/migrations.go (1 hunks)
- x/mint/keeper/msg_server_test.go (2 hunks)
- x/mint/migrations/v3/migrations.go (1 hunks)
- x/mint/migrations/v3/types/params.go (1 hunks)
- x/mint/module.go (4 hunks)
- x/mint/simulation/decoder_test.go (1 hunks)
- x/mint/simulation/genesis.go (2 hunks)
- x/mint/simulation/genesis_test.go (1 hunks)
- x/mint/simulation/proposals.go (1 hunks)
- x/mint/simulation/proposals_test.go (1 hunks)
- x/mint/types/errors.go (1 hunks)
- x/mint/types/genesis.go (1 hunks)
- x/mint/types/minter.go (1 hunks)
- x/mint/types/minter_test.go (3 hunks)
- x/mint/types/params.go (2 hunks)
Files skipped from review due to trivial changes (3)
- proto/mint/v1beta1/mint.proto
- x/mint/keeper/genesis_test.go
- x/mint/types/params.go
Files skipped from review as they are similar to previous changes (11)
- x/mint/abci.go
- x/mint/client/cli/query_test.go
- x/mint/keeper/keeper_test.go
- x/mint/keeper/msg_server_test.go
- x/mint/simulation/genesis.go
- x/mint/simulation/genesis_test.go
- x/mint/simulation/proposals.go
- x/mint/simulation/proposals_test.go
- x/mint/types/errors.go
- x/mint/types/genesis.go
- x/mint/types/minter.go
Additional comments: 25
Makefile (6)
269-275: The summary indicates that additional fields
title,summary, andmetadataare added to a JSON object within anechostatement, but the hunk shows that these fields are added within ajqcommand, not anechostatement. This discrepancy should be corrected in the summary.289-289: The change from
blocktosyncfor the broadcast mode in thetx gov depositcommand aligns with the PR objectives to change the block mode to sync mode for certain transactions.307-307: The change from
blocktosyncfor the broadcast mode in thetx gov votecommand aligns with the PR objectives to change the block mode to sync mode for certain transactions.276-276: The update to the
voting_periodwithin the genesis configuration file is a significant change that should be included in the summary.291-291: The introduction of a delay before executing the
tx gov depositcommand with thesleepcommand is a significant change that should be included in the summary.300-300: The introduction of a delay before executing the
tx gov votecommand with thesleepcommand is a significant change that should be included in the summary.app/app.go (1)
- 683-686: The changes to the
mint.NewAppModulefunction call inapp/mminstantiation are consistent with the PR objectives and the summary provided. The removal of theapp.GetSubspace(minttypes.ModuleName)parameter aligns with the new token model implementation that does not require a subspace for the mint module.app/upgrades.go (3)
9-12: The addition of the
v6import aligns with the PR objective of introducing a new token model and handling the migration of thex/mintmodule state.31-34: The addition of the upgrade handler for version 6 is consistent with the PR objectives, ensuring the new token model and inflation calculation are incorporated into the upgrade process.
53-54: The inclusion of
v6.StoreUpgradesis in line with the PR objectives, which require storage migration to support the new token model.app/upgrades/v6/upgrade.go (2)
12-16: The addition of the "feeibc" store is noted and appears to align with the PR objectives of introducing new functionality.
18-30: The
CreateUpgradeHandlerfunction correctly sets up the migration fromv5.0.0tov6.0.0and appears to be consistent with the PR objectives.docs/proto/mint.md (2)
6-44: The documentation updates in hunk 0 correctly describe the new token model's calculation mechanism and block-level token minting process, aligning with the PR objectives.
78-107: The documentation updates in hunk 1 correctly reflect the changes to the
MinterandParamsstructures, including the new fields and updated descriptions, which are in line with the PR objectives.x/mint/keeper/migrations.go (1)
- 3-21: The changes in the
keeperpackage, including the updatedMigratorstruct and the newMigrate2to3function, are consistent with the PR objectives and the provided summary. The removal oflegacySubspaceand the use of thev3package align with the migration from version 2 to version 3 of thex/mintmodule.x/mint/migrations/v3/migrations.go (2)
26-31: The previous comment about adding a nil check before unmarshalling
ParamsKeyis still valid and the proposed fix should be applied.44-49: The previous comment about adding a nil check before unmarshalling
MinterKeyis still valid and the proposed fix should be applied.x/mint/migrations/v3/types/params.go (1)
- 1-97: The provided code does not reflect the changes mentioned in the PR objectives and the summary. The
Paramsstruct and related functions do not include the new fields such asinflation_coef,bonding_adjustment,target_bonding_ratio, and the new error variableErrBondedRatioisZero. Additionally, theMinterstruct and methods likeBlockProvisionandMigrateStoreare not present. Please ensure that the code reflects all the changes described in the PR objectives and the summary.x/mint/module.go (4)
86-98: The
NewAppModulefunction has been updated to remove thelegacySubspaceparameter, which aligns with the PR objectives of updating the mint module to reflect the new token model. The removal oflegacySubspacefrom theAppModulestruct is consistent with the changes in the function signature.111-123: The migration logic in
RegisterServiceshas been updated to include a migration from version 2 to 3, which is in line with the PR objectives of updating the mint module for the new token model. This change is crucial for ensuring a smooth transition for existing data to the new model.146-150: The increment of the consensus version to
3in theConsensusVersionfunction is appropriate given the significant changes to the mint module, including the new token model and inflation calculation logic.151-152: The
BeginBlockfunction callsBeginBlockerwith thekeeperas a parameter, which is standard for module operations. This change does not appear to be directly related to the new token model implementation but is part of the module's routine operations.x/mint/simulation/decoder_test.go (1)
- 23-23: The change in the initialization of the
mintervariable from three arguments to two is consistent with the PR objectives and the summary provided. This reflects the updatedNewMinterfunction signature.x/mint/types/minter_test.go (2)
44-48: The previous comment about the hardcoded error message is still valid and does not need to be repeated.
96-119: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [87-117]
The benchmark tests have been updated to reflect the new inflation calculation logic, which is consistent with the PR objectives.
ccamel
force-pushed
the
feat/new-token-model
branch
2 times, most recently
from
ea38e07 to
65ed2a8
Compare
|
@coderabbitai pause |
ccamel
force-pushed
the
feat/new-token-model
branch
from
65ed2a8 to
d43a9a7
Compare
|
@amimart I think we're good now. Can you review the PR again? Thanks 🙏 |
Co-authored-by: Arnaud Mimart <[email protected]>
⛏️ Mint
Update the mint module with the token model v2 specified by axone-protocol/docs#356, addressing issue #433.
I kept the yearly inflation calculation like is done in the cosmos-sdk
mintimplementation, it's allow kept the GRPC endpoints and make it compatible with most of explorer.🧮 Calculation
Inflation calculation is made in the minter on NextInflation method.
Where:
r_inflationis the inflation rate of KNOW, representing the rate at which new tokens are created and introduced into circulation.ρ_bondedis the bonded ratio, i.e., the ratio between the number of tokens staked and the number of existing tokens.c_inflationis the inflation coefficient, a constant factor that adjusts the influence of the bonded ratio on the inflation rate.Once yearly inflation rate is known, we can calculate the yearly minted token divided by the number of block in a years to know how many token is minted for each block.
🚧 Todo
Close #433
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Refactor
Tests
Chores