feat: add inflation events detailed distribution #1695
Conversation
|
Walkthrough: Changes:
Assessment against linked issues:
Poem:
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: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- x/inflation/types/event.pb.go
Files selected for processing (2)
- proto/nibiru/inflation/v1/event.proto (1 hunks)
- x/inflation/keeper/inflation.go (1 hunks)
Additional comments: 2
proto/nibiru/inflation/v1/event.proto (1)
- 1-26: The
InflationDistributionEventprotobuf message has been correctly defined with the required fields and serialization metadata.x/inflation/keeper/inflation.go (1)
- 92-98: The addition of the
InflationDistributionEventemission is consistent with the PR objectives and the summary provided. It correctly includes thestaking,strategic, andcommunityfields as per the new protocol buffer message definition.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional comments: 1
CHANGELOG.md (1)
- 66-68: The addition of PR feat: add inflation events detailed distribution #1695 under the "Non-breaking/Compatible Improvements" section correctly documents the new feature related to inflation distribution events.
There was a problem hiding this comment.
--- FAIL: TestMintAndAllocateInflation (0.10s)
--- FAIL: TestMintAndAllocateInflation/Case_pass_-no_root_account (0.04s)
inflation_test.go:75: setting root account to
inflation_test.go:82:
Error Trace: /home/runner/work/nibiru/nibiru/x/inflation/keeper/inflation_test.go:82
Error: Received unexpected error:
empty address string is not allowed
Test: TestMintAndAllocateInflation/Case_pass-_no_root_account
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- x/inflation/types/event.pb.go
- x/sudo/types/state.pb.go
Files selected for processing (12)
- CHANGELOG.md (1 hunks)
- proto/nibiru/inflation/v1/event.proto (1 hunks)
- x/common/testutil/testapp/testapp.go (1 hunks)
- x/inflation/keeper/inflation.go (2 hunks)
- x/inflation/keeper/inflation_test.go (2 hunks)
- x/inflation/types/interfaces.go (1 hunks)
- x/sudo/genesis.go (1 hunks)
- x/sudo/keeper/keeper.go (1 hunks)
- x/sudo/keeper/msg_server_test.go (3 hunks)
- x/sudo/types/errors.go (1 hunks)
- x/sudo/types/genesis.go (1 hunks)
- x/sudo/types/state.go (1 hunks)
Files skipped from review due to trivial changes (1)
- proto/nibiru/inflation/v1/event.proto
Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- x/inflation/keeper/inflation.go
Additional comments: 11
x/common/testutil/testapp/testapp.go (1)
- 32-49: The changes made to
x/common/testutil/testapp/testapp.gocorrectly implement the logic to set a happy genesis state for the epochs and sudo modules, as described in the PR objectives and the AI-generated summaries. This will facilitate testing by providing a default, valid genesis state for these modules.x/inflation/keeper/inflation_test.go (3)
81-87: The error handling logic has been updated to check for the presence of
tc.rootAccount. Iftc.rootAccountis not empty, it requires that no error occurs, otherwise, it requires that an error does occur. This is a logical way to handle the test cases based on whether a root account is provided or not.88-90: The balance assertions have been updated to compare the expected amounts directly without converting them to strings. This is a more accurate way to compare balances as it avoids potential issues with string formatting and comparison.
115-124: The balance comparison assertions have been updated to compare string representations of the balances. This change is consistent with the summary provided and is a valid way to compare balances, ensuring that the comparison is not affected by any internal representation differences of the
sdk.Coinandsdk.DecCoinstypes.x/inflation/types/interfaces.go (1)
- 43-44: The change from
GetRoottoGetRootAddrin theSudoKeeperinterface is consistent with the PR objectives and the provided summaries.x/sudo/genesis.go (1)
- 31-35: The comment added to the
DefaultGenesisfunction provides clarity on the invalidity of the default genesis state due to the absence of "Sudoers.Root". This is a good practice as it helps developers understand the expected structure of a valid genesis state.x/sudo/keeper/keeper.go (1)
- 29-33: The function
GetRoothas been successfully renamed toGetRootAddr. Ensure that all calls to this function throughout the codebase have been updated to match the new function name.
The function
GetRoothas been successfully renamed toGetRootAddr, and references to the new function name have been confirmed in the codebase.Commands executed:
rg 'GetRoot\('rg 'GetRootAddr\('Inference:
- The first command did not find any occurrences of the old function name, indicating all references have likely been updated.
- The second command found multiple references to the new function name, confirming the updates.
x/sudo/keeper/msg_server_test.go (2)
26-28: The refactoring of the
setupfunction to usetestapp.NewNibiruTestAppAndContextAtTimesimplifies the test setup process and ensures consistency across tests. Ensure that the new method provides the necessary context setup for all tests that rely on it.37-41: The test case has been updated to use
panic: trueinstead of anemptyfield. Verify that this change accurately reflects the intended behavior of the test case and that the test is still valid and meaningful.x/sudo/types/genesis.go (1)
- 9-15: The changes to the
Validatefunction correctly implement the use of the custom error typeErrGenesisfor more specific error handling. This aligns with the PR objectives to enhance error handling.x/sudo/types/state.go (1)
- 9-18: The changes to the
Validatemethod correctly introduce additional validation for the root address and contract addresses, ensuring they are valid Bech32 addresses. This enhances the robustness of the validation logic.
Unique-Divine
force-pushed
the
mat/event-inflation-detail
branch
from
92b20dd to
e80ea3f
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (6)
- x/common/testutil/testapp/testapp.go (3 hunks)
- x/oracle/integration/app_test.go (2 hunks)
- x/sudo/types/errors.go (1 hunks)
- x/tokenfactory/keeper/msg_server_test.go (1 hunks)
- x/tokenfactory/types/state_test.go (3 hunks)
- x/tokenfactory/types/tx_msgs.go (2 hunks)
Files skipped from review due to trivial changes (2)
- x/tokenfactory/keeper/msg_server_test.go
- x/tokenfactory/types/state_test.go
Files skipped from review as they are similar to previous changes (3)
- x/common/testutil/testapp/testapp.go
- x/sudo/types/errors.go
- x/tokenfactory/types/tx_msgs.go
Additional comments: 3
x/oracle/integration/app_test.go (3)
11-17: The addition of the
testutilpackage import and related packages is consistent with the PR objectives and the AI-generated summaries.27-29: The addition of the
SetupSuitemethod to theIntegrationTestSuitetype is consistent with the PR objectives and the AI-generated summaries.31-33: The changes in the
SetupTestmethod are not mentioned in the AI-generated summaries, but since they are not related to the declarations of exported entities, this is acceptable.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- app/ibc_test.go (1 hunks)
- x/common/testutil/testapp/testapp.go (4 hunks)
Additional comments: 4
app/ibc_test.go (1)
- 34-39: The addition of
testapp.SetDefaultSudoGenesis(genesisState)inSetupNibiruTestingAppaligns with the PR's objective to enhance the inflation event details and improve the testing setup. Ensure that this new default genesis setting is correctly integrated with the rest of the test suite and does not introduce any inconsistencies.x/common/testutil/testapp/testapp.go (3)
25-27: The addition of the
initfunction to set the Bech32 prefix is a good practice to ensure consistency across tests.36-52: The logic added to set happy genesis for epochs and sudo modules is correctly implemented and will help in initializing the test app with a consistent state.
83-93: The summary does not mention the addition of the
SetDefaultSudoGenesisfunction, which sets the sudo module genesis state to a valid default. This function is important for initializing the genesis state and should be included in the summary.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (8)
- wasmbinding/test/cli_test.go (2 hunks)
- x/common/testutil/genesis/genesis.go (2 hunks)
- x/common/testutil/testapp/testapp.go (4 hunks)
- x/oracle/integration/app_test.go (2 hunks)
- x/perp/v2/client/cli/cli_test.go (2 hunks)
- x/spot/client/testutil/cli_test.go (2 hunks)
- x/sudo/cli/cli_test.go (2 hunks)
- x/tokenfactory/cli/cli_test.go (1 hunks)
Additional comments: 20
wasmbinding/test/cli_test.go (2)
21-27: The addition of the
testappimport and related packages is consistent with the changes described in the summary.43-48: The replacement of
app.SetPrefixes(app.AccountAddressPrefix)withtestapp.EnsureNibiruPrefix()is consistent with the summary and seems to be a refactoring to use a more specific function from thetestapppackage.x/common/testutil/genesis/genesis.go (3)
9-13: The import statement for
github.com/NibiruChain/nibiru/x/common/testutil/testappis correctly added to support the new function call.29-34: The call to
testapp.SetDefaultSudoGenesis(genState)is correctly placed to set up the default genesis state for the sudo module in the test environment. Ensure that theSetDefaultSudoGenesisfunction is properly defined and exported in thetestapppackage.
The call to
testapp.SetDefaultSudoGenesis(genState)is correctly placed to set up the default genesis state for the sudo module in the test environment. TheSetDefaultSudoGenesisfunction is confirmed to be properly defined and exported in thetestapppackage.
- 29-30: The modification to the governance module's genesis state to set a minimum deposit is a logical change for test setup, ensuring that tests can be run with a consistent and expected environment.
x/common/testutil/testapp/testapp.go (4)
25-27: The addition of the
init()function to ensure a specific prefix is a good practice to avoid runtime errors related to address prefixes.36-48: The logic added to set up the genesis state for the
epochsandsudomodules is correct and aligns with the summary.83-93: The addition of the
SetDefaultSudoGenesisfunction to set the sudo module genesis state to a valid default is a good practice for maintainability and ensuring a consistent test environment.108-115: The
NewNibiruTestAppfunction now includes a call toSetDefaultSudoGenesisto ensure the sudo module has a valid genesis state upon app initialization. This change was not mentioned in the summary and is important for ensuring a consistent test environment.x/oracle/integration/app_test.go (1)
- 25-26: The summary does not mention the addition of the
networkfield to theIntegrationTestSuitetype. This field is crucial for the setup and execution of integration tests, as it represents the test network being used.x/perp/v2/client/cli/cli_test.go (3)
18-21: The addition of the
testappimport is consistent with the PR objectives and the summary provided.38-38: The change from
app.SetPrefixes(app.AccountAddressPrefix)totestapp.EnsureNibiruPrefix()aligns with the PR objectives and the summary provided.18-24: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [18-40]
No further issues or inconsistencies found in the provided hunks.
x/spot/client/testutil/cli_test.go (3)
10-13: The addition of the
testappimport is consistent with the PR objectives and the summary provided.28-28: Refactoring to use
testapp.EnsureNibiruPrefix()improves maintainability by centralizing the prefix setup logic.31-31: The call to
WhitelistGenesisAssetsmodifies the genesis state, which is a significant change not mentioned in the summary. This should be documented as it affects the test setup.x/sudo/cli/cli_test.go (3)
25-30: The addition of the
testappimport is consistent with the summary provided.104-106: The replacement of
app.SetPrefixes(app.AccountAddressPrefix)withtestapp.EnsureNibiruPrefix()in theSetupSuitemethod aligns with the summary provided.104-106: Please verify that the change from
app.SetPrefixestotestapp.EnsureNibiruPrefix()does not affect other tests or parts of the codebase that may rely on the address prefixes being set in a specific way.
The change from
app.SetPrefixestotestapp.EnsureNibiruPrefix()is localized to theSetupSuitemethod inx/sudo/cli/cli_test.go. Other occurrences ofapp.SetPrefixesare found in different files, indicating that the change does not globally affect the setting of address prefixes in the codebase. However, it's important to ensure that the new methodtestapp.EnsureNibiruPrefix()sets the prefixes in a compatible manner withapp.SetPrefixes.x/tokenfactory/cli/cli_test.go (1)
- 42-42: The summary indicates that the line
testutil.BeforeIntegrationSuite(s.T())was removed, but it is still present in the provided code. Please verify if the summary is outdated or if the code change was not applied as intended.
* feat: add inflation events detailed distribution * chore: changelog * fix: improve error handling * fix: broken unit test + make x/sudo safer by making blank genesis invalid * refactor: run gofumpt formatter --------- Co-authored-by: Unique-Divine <realuniquedivine@gmail.com>
Description
Purpose
Why is this PR important?
Summary by CodeRabbit
New Features
Enhancements
Refactor
GetRoottoGetRootAddr.Documentation
Bug Fixes
Style