chore!: bump poa module to d321226
#87
Conversation
- POA params were removed in `d321226` - `GetPoAAdmin()` now panics if `POA_ADMIN_ADDRESS` is not set - Fix tests, parameters and documentation to reflect the update
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #87 +/- ##
==========================================
- Coverage 77.20% 76.74% -0.46%
==========================================
Files 34 34
Lines 1794 1802 +8
==========================================
- Hits 1385 1383 -2
- Misses 320 327 +7
- Partials 89 92 +3 ☔ View full report in Codecov by Sentry. |
[skip_ci]
WalkthroughThe recent changes streamline the blockchain application by removing hardcoded parameters for Proof of Authority (PoA) administrators, enhancing dynamic address retrieval, and updating module dependencies. The documentation is refined by eliminating outdated sections, while financial parameters in the genesis configuration are reset, indicating a shift towards a simpler governance model. 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 Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.sumis excluded by!**/*.suminterchaintest/go.sumis excluded by!**/*.sum
Files selected for processing (13)
- MODULE.md (2 hunks)
- app/app.go (2 hunks)
- app/test_helpers.go (2 hunks)
- chains/chain.json (1 hunks)
- go.mod (4 hunks)
- interchaintest/go.mod (4 hunks)
- interchaintest/mainfest_test.go (1 hunks)
- interchaintest/poa_group_test.go (8 hunks)
- interchaintest/setup.go (4 hunks)
- network/manifest-1/README.md (1 hunks)
- network/manifest-1/genesis.json (2 hunks)
- network/manifest-1/set-genesis-params.sh (1 hunks)
- scripts/test_node.sh (2 hunks)
Files skipped from review due to trivial changes (3)
- MODULE.md
- interchaintest/mainfest_test.go
- network/manifest-1/README.md
Additional comments not posted (30)
chains/chain.json (1)
Line range hint
1-1:
Verify the new governance model after the removal of the"admins"key.The removal of the
"app_state.poa.params.admins"key simplifies the configuration. Ensure that the new governance model is well-documented and tested.interchaintest/setup.go (2)
4-5: Importfmtis necessary for formatted I/O operations.The addition of the
fmtpackage is necessary for usingfmt.Sprintf.
60-62: Dynamic configuration ofPOA_ADMIN_ADDRESSimproves flexibility.The addition of
fmt.Sprintf("POA_ADMIN_ADDRESS=%s", accAddr)to theEnvslice allows for dynamic configuration of the PoA admin address, improving flexibility.network/manifest-1/set-genesis-params.sh (1)
Line range hint
1-1:
Verify the new governance model after the removal of the"admins"parameter.The removal of the
"admins"parameter from the"poa"section simplifies the genesis configuration. Ensure that the new governance model is well-documented and tested.network/manifest-1/genesis.json (8)
48-48: LGTM! Community tax set to zero.The change aligns with the objective of simplifying the economic model.
192-192: LGTM! Inflation set to zero.The change eliminates inflation in the token supply, aligning with the objective of removing inflationary mechanisms.
196-196: LGTM! Mint denomination set to "notused".The change indicates that the previous denomination is no longer applicable.
197-197: LGTM! Inflation rate change set to zero.The change aligns with the objective of removing inflationary mechanisms.
198-198: LGTM! Inflation max set to zero.The change aligns with the objective of removing inflationary mechanisms.
199-199: LGTM! Inflation min set to zero.The change aligns with the objective of removing inflationary mechanisms.
210-210: LGTM! Slash fraction for double sign set to zero.The change removes penalties for double signing, aligning with the objective of simplifying the governance model.
211-211: LGTM! Slash fraction for downtime set to zero.The change removes penalties for downtime, aligning with the objective of simplifying the governance model.
scripts/test_node.sh (2)
6-6: LGTM! Updated command-line arguments.The change suggests a shift in the configuration for running the script, potentially indicating a different environment or setup for testing.
6-6: LGTM! Removed outdated POA configuration comment.The removal of this comment implies that the POA configuration is no longer relevant or necessary in the current context of the script.
go.mod (8)
25-25: LGTM! Updatedcosmossdk.io/depinjectto stable version.The change reflects a move away from alpha releases to a stable version.
50-50: LGTM! Updatedgithub.aaakk.us.kg/strangelove-ventures/poa.The change aligns with the PR objective of bumping the POA module to the specified commit.
55-55: LGTM! Updatedgoogle.golang.org/protobuf.The change ensures compatibility and stability by updating to the latest version.
202-202: LGTM! Updatedgolang.org/x/crypto.The change ensures compatibility and stability by updating to the latest version.
204-204: LGTM! Updatedgolang.org/x/net.The change ensures compatibility and stability by updating to the latest version.
207-207: LGTM! Updatedgolang.org/x/sys.The change ensures compatibility and stability by updating to the latest version.
208-208: LGTM! Updatedgolang.org/x/term.The change ensures compatibility and stability by updating to the latest version.
209-209: LGTM! Updatedgolang.org/x/text.The change ensures compatibility and stability by updating to the latest version.
app/test_helpers.go (2)
40-40: LGTM!The new import statement for
testdatafrom the Cosmos SDK test utilities is appropriate.
183-189: LGTM! But verify the environment variable setting.The changes in the
setupfunction ensure that a valid POA admin address is always available during tests, improving the robustness of the testing environment.However, ensure that the environment variable
POA_ADMIN_ADDRESSis correctly set and handled throughout the codebase.Verification successful
Environment Variable Handling Verified
The
POA_ADMIN_ADDRESSenvironment variable is correctly set and handled throughout the codebase. The variable is consistently set in test setups and configurations, and it is checked to return the admin address where needed.
interchaintest/setup.go: Set in local chain environment configuration.interchaintest/poa_group_test.go: Set in test group environment configuration.app/app.go: Checked in theGetPoAAdminfunction.app/test_helpers.go: Set if not already set during test setup.app/sim_test.go: Set in thesetPOAAdminfunction for simulation tests.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `POA_ADMIN_ADDRESS` environment variable in the codebase. # Test: Search for the usage of the `POA_ADMIN_ADDRESS` environment variable. Expect: Correct handling of the environment variable. rg --type go 'POA_ADMIN_ADDRESS'Length of output: 688
Script:
#!/bin/bash # Description: Gather context around the usage of the `POA_ADMIN_ADDRESS` environment variable in the codebase. # Get context for each file where `POA_ADMIN_ADDRESS` is used files=( "interchaintest/setup.go" "interchaintest/poa_group_test.go" "app/app.go" "app/test_helpers.go" "app/sim_test.go" ) for file in "${files[@]}"; do echo "File: $file" rg --type go --context 10 'POA_ADMIN_ADDRESS' "$file" echo "----------------------------------------" doneLength of output: 4758
interchaintest/go.mod (3)
27-27: LGTM!The update to the
poamodule version is appropriate and aligns with the PR objectives.
43-43: LGTM!The update to the
cosmossdk.io/depinjectversion fromv1.0.0-alpha.4tov1.0.0is appropriate and aligns with the PR objectives.
241-255: LGTM!The updates to the versions of
golang.org/x/crypto,golang.org/x/net,golang.org/x/sys,golang.org/x/term, andgoogle.golang.org/genproto/googleapis/rpcare appropriate and align with the PR objectives.interchaintest/poa_group_test.go (1)
116-116: LGTM! But verify the core functionalities are still tested adequately.The removal of functions and variables related to POA parameters simplifies the test suite.
However, ensure that the core functionalities are still tested adequately.
Verification successful
Core functionalities are still tested adequately.
The test functions present in
interchaintest/poa_group_test.gocover a wide range of core functionalities, including software upgrades, stakeholder payouts, bank sends (both legal and illegal), and token operations. The removal of POA-specific parameters does not seem to have compromised the comprehensiveness of the tests.
testSoftwareUpgradetestManifestStakeholdersPayouttestBankSendtestBankSendIllegaltestTokenCreateScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the core functionalities are still tested adequately. # Test: Search for the remaining test functions. Expect: Adequate test coverage for core functionalities. rg --type go 'func test' interchaintest/poa_group_test.goLength of output: 709
Script:
#!/bin/bash # Extract the bodies of the test functions to review their comprehensiveness ast-grep --lang go --pattern 'func $_($_, $_, $_, $_, $_) { $$$ }' interchaintest/poa_group_test.goLength of output: 13230
app/app.go (2)
143-150: Verify the function usage in the codebase.Ensure that all function calls to
GetPoAAdminare consistent with the new implementation.Verification successful
Verify the function usage in the codebase.
The function
GetPoAAdminis used multiple times in theapp/app.gofile, and each usage appears consistent with the new implementation. No issues were found with the function usage.
- File:
app/app.go
- Lines: Multiple instances where
GetPoAAdminis called.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `GetPoAAdmin` are consistent with the new implementation. # Test: Search for the function usage. Expect: Only occurrences of the new implementation. rg --type go -A 5 $'GetPoAAdmin'Length of output: 4755
441-441: Verify the function usage in the codebase.Ensure that all function calls to
GetPoAAdminare consistent with the new implementation.
|
ictest-manifest fails because of docker hub "too many request". I confirm it passes locally. |
d321226GetPoAAdmin()now returns thex/govmodule address ifPOA_ADMIN_ADDRESSis not setFixes #21
Summary by CodeRabbit
Documentation
README.mdto reflect the removal of references to PoA administrators in genesis file descriptions.New Features
Configuration Changes
genesis.json, including setting community tax and inflation to zero.Bug Fixes