chore: synchronize releases/v2.0.0 branch with main branch
#2010
Conversation
#1649) * fix(ledger): bump the hid package to v0.9.2 which fixes ledger on new macos versions * chore: update changelog
…argate: auth, bank, gov, tokenfactory, epochs, inflation, oracle, sudo, devgas (#1646) * fix(tokenfactory)!: Fix bug in MsgBurn on total supply tracking * chore: rm stablecoin. How does this keep getting merged lol * test: add export statements for the gRPC query service descriptions in each module * feat(wasmbinding): whitelisted stargate queries for QueryRequest::Stargate * changelog * fix changelog * refactor!: make the epoch infos name consistent * docs,test(stargate_query): leave an in-depth explainer above the function * refactor: pR comments: earlier return + remove duplicate hardcoded paths * test: proto package may have more than 3 'parts'. Use len - 1 instead * docs: fix small documentation typos
* chore(epochs): add day epoch and remove 15min epoch from default genesis * chore(epochs): remove 15min epoch from default genesis * fix(inflation): inflate strategic reserves to sudo root account * fix(inflation): test inflation amounts * chore: update changelog
* chore(oracle): curate default oracle whitelist * update changelog
Bumps [github.com/CosmWasm/wasmvm](https://github.com/CosmWasm/wasmvm) from 1.4.1 to 1.5.0. - [Release notes](https://github.com/CosmWasm/wasmvm/releases) - [Changelog](https://github.com/CosmWasm/wasmvm/blob/main/CHANGELOG.md) - [Commits](CosmWasm/wasmvm@v1.4.1...v1.5.0) --- updated-dependencies: - dependency-name: github.com/CosmWasm/wasmvm dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: Unique-Divine <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Unique Divine <[email protected]>
* chore(deps): Bump google.golang.org/grpc from 1.58.3 to 1.59.0 Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.58.3 to 1.59.0. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.58.3...v1.59.0) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Updated changelog - dependabot --------- Signed-off-by: Unique-Divine <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Unique Divine <[email protected]> Co-authored-by: Unique-Divine <[email protected]> Co-authored-by: Unique-Divine <[email protected]>
* chore(wasm): bump wasmd dependency to v0.43.0 * Update CHANGELOG.md * fix: fix linter issues --------- Co-authored-by: Matthias <[email protected]>
* feat: update wasmd to v0.44.0 * chore: lint
* chore(inflation): remove x/inflation and inflation proto files * fix: wasmbinding imports * chore(inflation): remove inflation from app keepers * update changelog
* 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 <[email protected]>
* fix: fix default inflation allocation * chore: changelog * fix: fix tests --------- Co-authored-by: Unique Divine <[email protected]>
This reverts commit ee5e29f.
k-yang
force-pushed
the
releases/v2.0.0
branch
from
19db354 to
a58899e
Compare
k-yang
changed the title
releases/v2.0.0 branch with main branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
x/epochs/keeper/grpc_query_test.go (1)
36-42: 🛠️ Refactor suggestionAdd verification for all epochs in the response.
The test only verifies the first two epochs in detail. Consider adding assertions for all epochs to ensure complete validation.
require.Equal(t, epochInfosResponse.Epochs[0].CurrentEpoch, uint64(0)) require.Equal(t, epochInfosResponse.Epochs[0].EpochCountingStarted, false) +// Verify month epoch +require.Equal(t, epochInfosResponse.Epochs[2].StartTime, chainStartTime) +require.Equal(t, epochInfosResponse.Epochs[2].CurrentEpochStartTime, chainStartTime) +require.Equal(t, epochInfosResponse.Epochs[2].Identifier, "month") +require.Equal(t, epochInfosResponse.Epochs[2].Duration, time.Hour*24*30) +require.Equal(t, epochInfosResponse.Epochs[2].CurrentEpoch, uint64(0)) +require.Equal(t, epochInfosResponse.Epochs[2].EpochCountingStarted, false) +// Verify week epoch +require.Equal(t, epochInfosResponse.Epochs[3].StartTime, chainStartTime) +require.Equal(t, epochInfosResponse.Epochs[3].CurrentEpochStartTime, chainStartTime) +require.Equal(t, epochInfosResponse.Epochs[3].Identifier, "week") +require.Equal(t, epochInfosResponse.Epochs[3].Duration, time.Hour*24*7) +require.Equal(t, epochInfosResponse.Epochs[3].CurrentEpoch, uint64(0)) +require.Equal(t, epochInfosResponse.Epochs[3].EpochCountingStarted, false)
🧹 Nitpick comments (13)
x/epochs/types/genesis.go (1)
47-55: Consider using constants for duration calculations.While the implementation is correct, using magic numbers for duration calculations could be improved for better maintainability.
Consider defining constants for common time periods:
+const ( + DaysPerMonth = 30 + HoursPerDay = 24 +) { Identifier: MonthEpochID, StartTime: startTime, - Duration: 30 * 24 * time.Hour, + Duration: DaysPerMonth * HoursPerDay * time.Hour, CurrentEpoch: 0, CurrentEpochStartHeight: 0, CurrentEpochStartTime: startTime, EpochCountingStarted: false, }x/epochs/keeper/keeper_test.go (1)
16-24: Consider extracting test setup to a helper function.The epoch configuration setup is duplicated across multiple test functions. Consider extracting it to a helper function.
+func createTestEpochInfo(identifier string, duration time.Duration) types.EpochInfo { + return types.EpochInfo{ + Identifier: identifier, + StartTime: time.Time{}, + Duration: duration, + CurrentEpoch: 0, + CurrentEpochStartHeight: 0, + CurrentEpochStartTime: time.Time{}, + EpochCountingStarted: false, + } +} -epochInfo := types.EpochInfo{ - Identifier: "bi-monthly", - StartTime: time.Time{}, - Duration: time.Hour * 24 * 30 * 2, - CurrentEpoch: 0, - CurrentEpochStartHeight: 0, - CurrentEpochStartTime: time.Time{}, - EpochCountingStarted: false, -} +epochInfo := createTestEpochInfo("bi-monthly", time.Hour * 24 * 30 * 2)x/epochs/keeper/grpc_query_test.go (1)
27-27: Improve error message for better debugging.The current error message only dumps the epochInfos. Consider adding more context about what failed.
-errMsg := fmt.Sprintf("epochInfos: %v\n", epochInfos) +errMsg := fmt.Sprintf("Test failed. Expected epochs to match. Got epochInfos: %+v\n", epochInfos)x/epochs/types/epochinfo_test.go (1)
41-49: Consider using a more accurate month duration.The current implementation uses a fixed 30-day duration for monthly epochs. This simplified approximation could lead to drift over time as actual months vary between 28-31 days.
Consider using a dynamic calculation that accounts for varying month lengths:
- Duration: 30 * 24 * time.Hour, + Duration: time.Hour * 24 * 30.44, // Average month length (365.24/12)x/epochs/genesis_test.go (1)
48-54: Consider parameterizing the test assertions.The test has repetitive assertion blocks for each epoch. This could be refactored to reduce duplication and make maintenance easier.
Consider using a test helper function:
func assertEpochInfo(t *testing.T, epoch types.EpochInfo, identifier string, duration time.Duration, chainStartTime time.Time, errMsg string) { require.Equal(t, epoch.Identifier, identifier) require.Equal(t, epoch.StartTime, chainStartTime, errMsg) require.Equal(t, epoch.Duration, duration) require.Equal(t, epoch.CurrentEpoch, uint64(0)) require.Equal(t, epoch.CurrentEpochStartHeight, int64(0)) require.Equal(t, epoch.CurrentEpochStartTime, chainStartTime, errMsg) require.Equal(t, epoch.EpochCountingStarted, false) }Then use it in the test:
- require.Equal(t, genesis.Epochs[2].Identifier, "month") - require.Equal(t, genesis.Epochs[2].StartTime, chainStartTime, errMsg) - require.Equal(t, genesis.Epochs[2].Duration, time.Hour*24*30) - require.Equal(t, genesis.Epochs[2].CurrentEpoch, uint64(0)) - require.Equal(t, genesis.Epochs[2].CurrentEpochStartHeight, int64(0)) - require.Equal(t, genesis.Epochs[2].CurrentEpochStartTime, chainStartTime, errMsg) - require.Equal(t, genesis.Epochs[2].EpochCountingStarted, false) + assertEpochInfo(t, genesis.Epochs[2], "month", time.Hour*24*30, chainStartTime, errMsg)Also applies to: 56-62
app/keepers.go (1)
658-658: Redundant comment
The inline comment "// evm" does not convey meaningful information. Consider removing or expanding it to clarify usage.app/upgrades/v1_0_3/constants.go (1)
15-15: Improve comment clarity.The comment could be more descriptive about why this upgrade is needed beyond testing purposes.
-// a no-op store upgrade to test the upgrade process and include the newer version cosmos-sdk +// A no-op store upgrade that serves two purposes: +// 1. Test the upgrade process +// 2. Include the newer version of cosmos-sdkapp/upgrades/v2_0_0/constants.go (1)
16-26: Add documentation for the upgrade purpose.Consider adding a comment explaining why this upgrade is needed and what changes it introduces, particularly regarding the addition of the EVM module.
+// Upgrade handler for v2.0.0 that: +// 1. Runs module migrations +// 2. Adds the EVM module to the store var Upgrade = upgrades.Upgrade{The upgrade handler implementation itself looks good:
- Properly defines the upgrade name
- Correctly implements the upgrade handler
- Appropriately adds the EVM module to store upgrades
app/upgrades/v1_0_1/constants.go (1)
15-15: Improve comment clarity.The comment could be more descriptive about why this upgrade is needed beyond testing purposes.
-// pretty much a no-op store upgrade to test the upgrade process and include the newer version of rocksdb +// A no-op store upgrade that serves two purposes: +// 1. Test the upgrade process +// 2. Include the newer version of RocksDBx/oracle/keeper/edit_params_test.go (2)
26-31: Consider using constants for test parameters.The VotePeriod value of 100 is a magic number. Consider defining it as a constant to improve test maintainability and make the test's intent clearer.
+const ( + testVotePeriod uint64 = 100 +) msg := types.MsgEditOracleParams{ Sender: alice.String(), Params: &types.OracleParamsMsg{ - VotePeriod: 100, + VotePeriod: testVotePeriod, }, }
16-54: Add test case for nil params.The test suite should include a case for handling nil params to ensure robust error handling.
+// Case 3: nil params should return error +msg = types.MsgEditOracleParams{ + Sender: alice.String(), + Params: nil, +} + +_, err = msgServer.EditOracleParams(goCtx, &msg) +require.Error(t, err) +require.Contains(t, err.Error(), "params cannot be nil")x/oracle/cli/tx.go (1)
73-76: Fix typo in variable name.The variable name has a typo:
chagedWhitelistshould bechangedWhitelist.- chagedWhitelist := []asset.Pair{ + changedWhitelist := []asset.Pair{proto/nibiru/oracle/v1/tx.proto (1)
95-102: Restructure MsgEditOracleParams with Nested OracleParamsMsg.
The messageMsgEditOracleParamsnow encapsulates its parameters within anOracleParamsMsgfield. Note that this change alters the field’s cardinality from “optional with implicit presence” to “optional with explicit presence,” as flagged by static analysis. Make sure all clients and downstream logic that serialize or deserialize this message are updated to accommodate this breaking change.🧰 Tools
🪛 GitHub Check: break-check
[failure] 95-95:
Previously present field "10" with name "min_voters" on message "MsgEditOracleParams" was deleted.
[failure] 95-95:
Previously present field "11" with name "validator_fee_ratio" on message "MsgEditOracleParams" was deleted.
[failure] 95-95:
Previously present field "3" with name "vote_threshold" on message "MsgEditOracleParams" was deleted.
[failure] 95-95:
Previously present field "4" with name "reward_band" on message "MsgEditOracleParams" was deleted.
[failure] 95-95:
Previously present field "5" with name "whitelist" on message "MsgEditOracleParams" was deleted.
[failure] 95-95:
Previously present field "6" with name "slash_fraction" on message "MsgEditOracleParams" was deleted.
[failure] 95-95:
Previously present field "7" with name "slash_window" on message "MsgEditOracleParams" was deleted.
[failure] 95-95:
Previously present field "8" with name "min_valid_per_window" on message "MsgEditOracleParams" was deleted.
[failure] 95-95:
Previously present field "9" with name "twap_lookback_window" on message "MsgEditOracleParams" was deleted.
[failure] 101-101:
Field "2" with name "params" on message "MsgEditOracleParams" changed cardinality from "optional with implicit presence" to "optional with explicit presence".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
x/oracle/types/tx.pb.gois excluded by!**/*.pb.gox/oracle/types/tx.pb.gw.gois excluded by!**/*.pb.gw.go
📒 Files selected for processing (38)
CHANGELOG.md(2 hunks)Dockerfile(1 hunks)app/app.go(2 hunks)app/appconst/appconst.go(2 hunks)app/keepers.go(4 hunks)app/keepers/all_keepers.go(3 hunks)app/upgrades.go(1 hunks)app/upgrades/types.go(0 hunks)app/upgrades/v1_0_1/constants.go(1 hunks)app/upgrades/v1_0_2/constants.go(1 hunks)app/upgrades/v1_0_3/constants.go(1 hunks)app/upgrades/v1_5_0/constants.go(1 hunks)app/upgrades/v2_0_0/constants.go(1 hunks)cmd/nibid/cmd/init.go(1 hunks)contrib/docker/chaosnet.Dockerfile(1 hunks)contrib/scripts/release_pre_linux.sh(1 hunks)proto/nibiru/oracle/v1/tx.proto(3 hunks)x/devgas/v1/module.go(1 hunks)x/epochs/genesis_test.go(2 hunks)x/epochs/keeper/grpc_query_test.go(1 hunks)x/epochs/keeper/keeper_test.go(2 hunks)x/epochs/types/epochinfo_test.go(1 hunks)x/epochs/types/genesis.go(1 hunks)x/epochs/types/identifier.go(1 hunks)x/oracle/cli/tx.go(3 hunks)x/oracle/keeper/edit_params_test.go(1 hunks)x/oracle/keeper/msg_server.go(2 hunks)x/oracle/keeper/params.go(1 hunks)x/oracle/keeper/params_test.go(2 hunks)x/oracle/keeper/sudo.go(0 hunks)x/oracle/keeper/sudo_test.go(0 hunks)x/oracle/keeper/test_utils.go(2 hunks)x/oracle/keeper/update_exchange_rates_test.go(1 hunks)x/oracle/module.go(4 hunks)x/oracle/types/params.go(1 hunks)x/sudo/keeper/keeper_test.go(1 hunks)x/sudo/module.go(1 hunks)x/tokenfactory/module.go(1 hunks)
💤 Files with no reviewable changes (3)
- app/upgrades/types.go
- x/oracle/keeper/sudo_test.go
- x/oracle/keeper/sudo.go
✅ Files skipped from review due to trivial changes (5)
- Dockerfile
- contrib/scripts/release_pre_linux.sh
- x/devgas/v1/module.go
- cmd/nibid/cmd/init.go
- app/app.go
🧰 Additional context used
🪛 GitHub Check: break-check
proto/nibiru/oracle/v1/tx.proto
[failure] 101-101:
Field "2" with name "params" on message "MsgEditOracleParams" changed cardinality from "optional with implicit presence" to "optional with explicit presence".
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: integration-tests
- GitHub Check: integration-tests
🔇 Additional comments (34)
x/tokenfactory/module.go (1)
44-44: LGTM! Consistent with other module changes.The consensus version change from 2 to 1 aligns with similar changes in other modules, maintaining consistency across the codebase as part of the release branch synchronization.
x/epochs/types/identifier.go (1)
19-20: LGTM! FifteenMinuteEpochID reintroduced.The reintroduction of FifteenMinuteEpochID is consistent with the epoch management system.
x/epochs/genesis_test.go (1)
29-29: LGTM!The test correctly updates the expected number of epochs to 4, aligning with the addition of the monthly epoch.
app/keepers.go (6)
71-72: Introduce 'ibcwasm' 08-wasm client imports
Everything looks consistent with the usage of the 08-wasm IBC light client inapp.WasmClientKeeper.
74-78: Adoption of ICS-27 interchain accounts
The new references to multiple 'icacontroller' and 'icacontrollertypes' packages indicate advanced interchain account functionalities. Verify that the code references remain consistent across the codebase.
80-81: Include ICA host type references
The additional 'icahosttypes' and 'icatypes' imports appear consistent with ICS-27 usage.
116-116: Add Oracle module alias
No issues found with introducing 'oracle' alias for the nibiru's x/oracle module.
122-122: Add TokenFactory module alias
The newly introduced 'tokenfactory' alias is consistent with nibiru's x/tokenfactory usage.
644-644: Oracle module now depends on SudoKeeper
The addition ofapp.SudoKeeperas a parameter indicates a new dependency, which can introduce new functionalities or permission checks in the Oracle module. Confirm that all references are updated accordingly.app/upgrades/v1_5_0/constants.go (1)
1-23: New upgrade step for v1.5.0
This implements a standard approach for the App upgrade, withRunMigrationsinvoked as expected. The emptyStoreUpgradessuggests no store changes are needed for this version.app/upgrades/v1_0_2/constants.go (1)
1-24: No-op upgrade to v1.0.2
This upgrade uses a standard pattern by invokingRunMigrations. With no store changes, it appears straightforward and safe.app/upgrades/v1_0_3/constants.go (1)
16-24: LGTM!The upgrade handler implementation is correct and follows the standard pattern:
- Properly defines the upgrade name
- Correctly implements the upgrade handler
- Appropriately specifies an empty store upgrade
app/upgrades/v1_0_1/constants.go (1)
16-24: LGTM!The upgrade handler implementation is correct and follows the standard pattern:
- Properly defines the upgrade name
- Correctly implements the upgrade handler
- Appropriately specifies an empty store upgrade
app/upgrades.go (1)
9-17: LGTM! The upgrade handlers are properly sequenced.The addition of missing upgrade handlers from v1.5.0 release is well-organized and maintains the correct version sequence.
Also applies to: 22-30
app/appconst/appconst.go (1)
58-58:❓ Verification inconclusive
Verify impact of chain ID changes on existing testnet deployments.
The ETH chain ID for testnet-1 has been updated from 6910 to 7210. This change could affect:
- Existing smart contracts that might have the chain ID hardcoded
- External integrations that rely on the previous chain ID
Also applies to: 77-77
🏁 Script executed:
#!/bin/bash # Search for hardcoded references to the old chain ID rg -l "6910" --type goLength of output: 22
Below is an enhanced verification step. Although the original script searching for
"6910"in Go files produced no matches, we want to ensure that no references to the old chain ID exist elsewhere in the repository. Please run the following script to search the entire codebase:After running this global search, please manually verify that:
- No configuration files, scripts, or documentation contain the old chain ID.
- External integrations or smart contracts aren’t relying on the previous value.
Once confirmed, you can safely conclude the impact of this change on the testnet deployments.
🏁 Script executed:
[run_scripts] #!/bin/bash # Search for any references to the old chain ID "6910" across the entire repository rg -l "6910"Length of output: 166
Chain ID update verified with minor caution on testdata reference
The change to update
ETH_CHAIN_ID_TESTNET_1from6910to7210inapp/appconst/appconst.goappears correct in production code. A global search for"6910"showed no references in production sources, aside from one occurrence in the testdata filex/common/ewma/testdata/ewma.csv. Please verify that this test data reference is intentional and does not indicate any unintended dependencies on the old chain ID in tests or external configurations.app/keepers/all_keepers.go (1)
14-14: LGTM! Import organization and keeper type updates are well-structured.The changes properly restore the 08-wasm IBC client and improve code organization through consistent import aliasing.
Also applies to: 30-30, 62-62
x/oracle/types/params.go (1)
47-47:❓ Verification inconclusive
Verify the impact of removing BNB-USD pair from DefaultWhitelist.
The BNB-USD pair has been removed from the default whitelist. This change could affect existing systems that rely on this pair.
Run the following script to check for any dependencies on the BNB-USD pair:
🏁 Script executed:
#!/bin/bash # Description: Search for BNB-USD pair usage in the codebase # Search for direct references to BNB-USD pair rg -A 5 "BNB.*USD|USD.*BNB" # Search for tests that might be affected rg -A 5 "TestDefaultWhitelist|TestWhitelist"Length of output: 4211
Action Required: Confirm External Dependencies for Removed BNB-USD Pair
After executing the search, it appears that the removal of the BNB-USD pair from the default whitelist in
x/oracle/types/params.go(line 47) is isolated in the codebase. Specifically:
- Configuration Impact: The Docker Compose configuration files (e.g.,
contrib/docker-compose/docker-compose-chaosnet.yml) still reference"BNBUSD"in theEXCHANGE_SYMBOLS_MAP, suggesting that external systems or deployments might rely on this pair.- Asset Registry: The asset registry in
x/common/asset/registry.gocontinues to list BNB with other currency pairs, though it no longer explicitly pairs BNB with USD in the whitelist.- Test References: Test files such as those in
x/oracle/keeper/do not show explicit dependency on a BNB-USD whitelist entry.Recommendation:
- Review Deployment Configurations: Ensure that external systems or configuration files expecting a BNB-USD pair are updated to reflect this change.
- Confirm External Dependencies: Verify with deployment and operations teams that the removal from the default whitelist does not cause unintended side effects in environments using Docker Compose or similar configurations.
x/oracle/keeper/params.go (1)
75-123: LGTM! Well-structured parameter merging logic.The
mergeOracleParamsfunction correctly handles different parameter types and preserves existing values when new values are not provided. The implementation is robust and follows best practices for parameter updates.x/oracle/keeper/msg_server.go (3)
19-19: LGTM! Added SudoKeeper for permission management.The addition of SudoKeeper field enables proper permission management for privileged operations.
24-26: LGTM! Updated constructor to include SudoKeeper.The constructor now properly initializes both Keeper and SudoKeeper fields.
176-199: LGTM! Improved EditOracleParams implementation.The updated implementation includes:
- Proper error handling for address validation
- Permission checks using SudoKeeper
- Parameter merging using the new mergeOracleParams function
x/oracle/module.go (1)
8-9: LGTM! Consistent integration of SudoKeeper.The changes properly integrate the sudo functionality throughout the module:
- Added necessary import
- Added field to AppModule struct
- Updated constructor and initialization
- Updated service registration
Also applies to: 99-99, 108-108, 115-115, 127-127
x/oracle/keeper/params_test.go (1)
59-435: LGTM! Comprehensive test coverage for oracle parameter merging.The test function is well-structured and covers all essential scenarios:
- Tests both valid and invalid cases for each parameter
- Verifies that empty or zero values don't override existing values
- Includes proper error handling for invalid inputs
x/oracle/cli/tx.go (1)
213-352: LGTM! Well-structured CLI command for editing oracle parameters.The command implementation is thorough with proper input validation and error handling for each parameter.
x/oracle/keeper/test_utils.go (1)
132-132: LGTM! Proper integration of SudoKeeper functionality.The changes correctly integrate the
SudoKeeperinto the test utilities, ensuring proper authorization checks in the oracle module.Also applies to: 327-327
x/oracle/keeper/update_exchange_rates_test.go (1)
138-138: LGTM! Consistent update to message server instantiation.The change correctly updates the message server instantiation to include the
SudoKeeperparameter, maintaining consistency with the broader changes.contrib/docker/chaosnet.Dockerfile (2)
32-33: Confirm Script Copy Operation.
The addition of the copy command forchaosnet.shensures that the script is brought into the container’s working directory from./contrib/scripts/. Please verify that the relative path is correct and that the file exists at the specified location.
34-40: Ensure Executable Permission and Correct Environment Variable Usage.
The new lineRUN chmod +x ./chaosnet.shproperly sets the executable permission for the script, and the subsequentRUNcommand passes environment variables inline to execute the script. Confirm that the shell script is designed to properly handle these environment variables.proto/nibiru/oracle/v1/tx.proto (4)
6-6: Addition of Protobuf Duration Import.
The import of"google/protobuf/duration.proto"is necessary to support the newDurationtype used for thetwap_lookback_windowfield. This change improves the clarity and correctness of time interval definitions in the message.
35-38: Update HTTP Endpoint for EditOracleParams RPC.
The HTTP POST option for theEditOracleParamsRPC is updated to"/nibiru/oracle/params". Please ensure that this new endpoint is consistent with the overall API design and that any client integrations or documentation reflect this change accordingly.
104-105: Simplify MsgEditOracleParamsResponse Structure.
The response message for editing oracle parameters has been simplified to an empty message. Ensure that consumers of this RPC are not expecting returned parameters and that any associated documentation is updated accordingly.
106-191: Comprehensive Definition for OracleParamsMsg.
The newOracleParamsMsgmessage is well structured and defines a comprehensive set of oracle parameters—including numeric fields, string types with custom decoding for decimals, and a duration type fortwap_lookback_window. This design improves type safety and clarity. Please double-check that all new fields and corresponding tags meet the versioning and compatibility requirements for clients using this proto definition.CHANGELOG.md (2)
53-53: Addition of Feature Entry for Wasm Light Clients on IBC.
A new changelog entry for pull request [#2068] has been added to document the feature enabling wasm light clients on IBC. Ensure that the description accurately captures the changes introduced in the code and that it aligns with the broader release notes.
61-61: Updated Section Header for v2.0.0 Release.
The section header is now reformatted as## [v2.0.0](https://github.com/NibiruChain/nibiru/releases/tag/v2.0.0) - 2025-02-10, which improves linkability and readability. Confirm that the release date is correct and that this header complies with the changelog guidelines.
| @@ -143,7 +143,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw | |||
| } | |||
|
|
|||
| // ConsensusVersion implements ConsensusVersion. | |||
| func (AppModule) ConsensusVersion() uint64 { return 3 } | |||
| func (AppModule) ConsensusVersion() uint64 { return 1 } | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the consensus version rollback.
The consensus version is being rolled back from 3 to 1, which is part of a coordinated change across multiple modules to synchronize release branches. This change could impact module compatibility and upgrades.
Run the following script to verify the consensus version changes across all modules:
🏁 Script executed:
#!/bin/bash
# Description: Check consensus version changes across all modules.
# Search for ConsensusVersion declarations and their values
rg -A 1 'ConsensusVersion.*=|func.*ConsensusVersion\(\).*uint64'Length of output: 1076
Consensus Version Coordination Issue
The update in x/sudo/module.go now returns 1, consistent with the intended rollback. However, our search across the codebase shows discrepancies:
- x/inflation/module.go is still returning
3. - x/genmsg/module.go returns
0. - x/evm/evmmodule/module.go uses a variable (
consensusVersion), so its effective value isn’t immediately clear.
Please verify whether the rollback from consensus version 3 to 1 is meant to be applied uniformly across all modules. If the intent is a coordinated rollback, update the mismatched modules accordingly or provide documentation justifying their deviations.
| // WeekEpochID defines the identifier for weekly epochs | ||
| MonthEpochID = "month" | ||
| // WeekEpochID defines the identifier for weekly epochs | ||
| WeekEpochID = "week" |
There was a problem hiding this comment.
Fix duplicate comment for WeekEpochID.
The comment for WeekEpochID appears twice. Remove the duplicate comment at line 9.
- // WeekEpochID defines the identifier for weekly epochs
MonthEpochID = "month"📝 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.
| // WeekEpochID defines the identifier for weekly epochs | |
| MonthEpochID = "month" | |
| // WeekEpochID defines the identifier for weekly epochs | |
| WeekEpochID = "week" | |
| MonthEpochID = "month" | |
| // WeekEpochID defines the identifier for weekly epochs | |
| WeekEpochID = "week" |
| @@ -25,7 +25,7 @@ func TestCheckPermissions(t *testing.T) { | |||
| Contracts: mockContractAddrStrs, | |||
| }) | |||
|
|
|||
| err := nibiru.SudoKeeper.CheckPermissions(sdk.AccAddress([]byte("addrbbb")), ctx) | |||
| err := nibiru.SudoKeeper.CheckPermissions(sdk.AccAddress("addrbbb"), ctx) | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the address construction method.
The change from sdk.AccAddress([]byte("addrbbb")) to sdk.AccAddress("addrbbb") might not be correct. The sdk.AccAddress type typically expects a byte slice.
Let's verify the correct usage of sdk.AccAddress:
🌐 Web query:
What is the correct way to create a Cosmos SDK AccAddress from a string?
💡 Result:
To create a Cosmos SDK AccAddress from a string, there are two primary methods depending on the input format:
1. From Bech32-encoded strings
This is the standard method for user interaction. Use AccAddressFromBech32() with addresses starting with prefixes like cosmos:
bechAddress := "cosmos1huydeevpcwqjv8xyg3eu0m5ln3w5y56d3fmsd5"
addr, err := sdk.AccAddressFromBech32(bechAddress)2. From hexadecimal strings
For raw hex strings (without Bech32 prefixes), use AccAddressFromHexUnsafe():
hexAddr := "6E347F6E0D8A0C3D5FF7D42EC966A3F0"
addr, err := sdk.AccAddressFromHexUnsafe(hexAddr)Note: This method is marked "unsafe" because hexadecimal representations can be confused with transaction hashes[1][4].
Key Details
| Method | Input Format | Safety | Common Use Case |
|---|---|---|---|
AccAddressFromBech32 |
Human-readable (e.g., cosmos1...) |
Safe | User-facing addresses |
AccAddressFromHexUnsafe |
Raw hex | Potentially unsafe | Low-level/internal operations |
For production systems:
- Always prefer Bech32 as it includes error-checking codes and clear prefix identifiers[1][3]
- Use configuration prefixes matching your chain (e.g.,
osmofor Osmosis instead ofcosmos)[4] - Validate addresses before use with
.Empty()checks[4]
Example validation:
if addr.Empty() {
return errors.New("invalid address")
}Citations:
- 1: https://docs.cosmos.network/main/learn/beginner/accounts
- 2: https://pkg.go.dev/github.com/litegraph/cosmos-sdk/types
- 3: https://docs.cosmos.network/v0.46/basics/accounts.html
- 4: https://github.com/cosmos/cosmos-sdk/blob/main/types/address.go
- 5: https://docs.cosmos.network/main/learn/advanced/encoding
- 6: https://pkg.go.dev/github.com/cosmos/cosmos-sdk/x/distribution/types
- 7: Convert sdk.AccAddress to be a string, not a []byte cosmos/cosmos-sdk#10838
- 8: https://docs.cosmos.network/v0.46/core/encoding.html
- 9: https://docs.cosmos.network/v0.45/core/encoding.html
- 10: https://docs.cosmos.network/v0.46/modules/auth/02_state.html
Fix Address Construction in keeper_test.go
The current change to use sdk.AccAddress("addrbbb") is problematic because the Cosmos SDK expects addresses to be constructed either by converting a byte slice (as in sdk.AccAddress([]byte("addrbbb"))) or, preferably, by using one of the dedicated helper functions. For production or more realistic test cases, you should use methods such as:
-
For Bech32-encoded addresses:
addr, err := sdk.AccAddressFromBech32("cosmos1...")
-
For hexadecimal strings (with caution):
addr, err := sdk.AccAddressFromHexUnsafe("6E347F6E0D8A0C3D5FF7D42EC966A3F0")
If the intent is solely to generate a test address and no particular address format is required, reverting to the byte slice conversion (i.e., sdk.AccAddress([]byte("addrbbb"))) is the simplest correct approach.
| msg = types.MsgEditOracleParams{ | ||
| Sender: alice.String(), | ||
| Params: &types.OracleParamsMsg{ | ||
| VotePeriod: 100, | ||
| }, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify the actual parameter changes.
The test only checks for authorization but doesn't verify if the parameters were actually updated. Consider adding assertions to check the final parameter values.
_, err = msgServer.EditOracleParams(goCtx, &msg)
require.NoError(t, err)
+
+// Verify the parameter was actually updated
+params := app.OracleKeeper.GetParams(ctx)
+require.Equal(t, testVotePeriod, params.VotePeriod)
Overview
Merges the
v2.0.0release branch intomainto converge the release branches. Following this, we'll deploy directly frommainbranch and have no more need for release branches.Changes
v1.5.0releaseMsgEditOracleParamsback to how it was onreleases/v1.5.0, overwriting the changes made onmainbranchv1.1.008-wasmibc client