Remove the GMP default params from genesis #727
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughWalkthroughThe recent changes involve the removal of several functions and default parameters related to the GMP module within the project. Key modifications include the elimination of the Changes
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 Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
Closed
Pitasi
approved these changes
Aug 28, 2024
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- CHANGELOG.md (1 hunks)
- warden/x/gmp/module/module.go (1 hunks)
- warden/x/gmp/types/genesis.go (1 hunks)
- warden/x/gmp/types/msgs.go (1 hunks)
- warden/x/gmp/types/params.go (1 hunks)
Files skipped from review due to trivial changes (2)
- warden/x/gmp/module/module.go
- warden/x/gmp/types/genesis.go
Additional context used
Path-based instructions (3)
warden/x/gmp/types/params.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/gmp/types/msgs.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (2)
warden/x/gmp/types/params.go (1)
8-8: EnsureValidatemethod is still appropriate.Given the removal of default parameters, the unchanged
Validatemethod may now lead to more frequent validation errors. It is important to review whether the current validation logic is still appropriate or if adjustments are needed to accommodate the absence of default values.Consider revising the validation logic to better align with the new configuration approach.
CHANGELOG.md (1)
60-60: Approve the changelog entry with a minor correction.The entry about removing the GMP default parameters from genesis is clear and informative. However, ensure it is grammatically correct and aligns with the rest of the changelog's style.
The changelog entry is approved with a suggestion to review it for minor grammatical improvements.
| GmpTimeout: DefaultTimeout, | ||
| FeeRecipient: DefaultFeeRecipient, | ||
| } | ||
| return Params{} |
There was a problem hiding this comment.
Review the impact of removing default parameters.
The change to return an empty Params struct in DefaultParams is consistent with the PR's objective to remove default values. However, ensure that all parts of the codebase that rely on these default values are updated to handle the absence of defaults.
Consider adding checks or initializing these parameters where necessary to avoid runtime errors.
mn13
pushed a commit
that referenced
this pull request
Sep 3, 2024
mn13
added a commit
that referenced
this pull request
Sep 5, 2024
…dd-genesis-space (#723) * validate bench32 in cmd add-genesis-keychain add-genesis-space * in executeAnalyzers return error on bad contract address (#716) * in executeAnalyzers return error on bad contract address * update changelog * validate bench32 in cmd add-genesis-keychain add-genesis-space * fix spelling and duplicates * * Added KeybaseId, Url and Name fields to keychain * Added changelog * Review comments * Code review fixes * Regenerated model --------- Co-authored-by: Archie <artur.abliazimov@gmail.com> * Fix of localnet.just (#726) * Fixed localnet.just * Fixed localnet.just * Remove the GMP default params from genesis (#727) * feat(faucet): new faucet (#479) * feat(faucet): new faucet This version of the Faucet does not have any limitations per user, only limitations are daily token supply * fix: minor css changes for mobile * fix(faucet): add daily refresh and token refresh job * fix(faucet): fix bugs and ci * fix(faucet): remove unnessary go.mod files, replace dockerfile faucet-v2 * chore(faucet): remove unnessary variables and modify types this will modify the types of DailyLimit and Batchinterval to remove unnessary conversions and remove Cooldown and BatchInterval * fix(faucet): acquire a lock when setting the new supply * fix(faucet): removed locking where it's not needed other enhancements as well, the loading and setting context to correct place * fix(faucet): converted background to much smaller image * feat(faucet): make the run out of daily nicer * feat(faucet): add new style * feat(faucet): add check that same address cannot be multiple time in one batch * feat(faucet): add metrics * fix(faucet): Dockerfile libwasmvm was missing access rights * feat(faucet): add pre input option to address form --------- Co-authored-by: Jon Heywood <94459819+jjheywood@users.noreply.github.com> * chore(wardend): bump ibc-go to v8.4.0 * docs: Update Keychain SDK (#706) * Upgrade keychain docs Update go version Update json for keychain-fees Update chain-id * Add categorization to Keychain SDK * Categorization * Add Keychain SDK + Description * Create a new tutorial Add basic info for keychain Implement main func with - handleKeyRequest and handleSignRequest Write test function * fixed a link in the Tools menu * quick fixes * quick fixes * quick fixes * remove ) --------- Co-authored-by: Margarita Skomorokh <ijonele@gmail.com> * fixed links to the keychain sdk (#733) * Made KeychainFees = nil invalid * ci: add check to ensure go.mod is tidy * chore: go mod tidy * fix(wardend): ICAHostKeeper initialization requires QueryRouter to be set Breaking change introduced in ibc-go v8.3 * feat(spaceward): key creation in sidebar (#728) * fix key icon in sidebar cleanup * fix getProider call * bump ethers version; fix erc20 send * build: use new justfile commands * fix(x/warden): make KeychainFees non-nullable * fix(x/warden): allow empty KeychainFees * cosmoshubtestnet hotfix; do not fail on wrong message type (#746) * feat(faucet): configurable chain name (#747) this change adds configurable chain/faucet name to the site * fix(faucet): chain variable missing from struct (#749) This PR adds the missing Chain variable that missed in #747 * #561 Keychain fees should not be paid in advance (#662) * Added keychain max wanted fees handling * Implemented new testcase & snapshot * Added annotations * Added keychain fee max value specification * Code review fixes * Changed validation messages * fix(x/act): add support for sdk.Coins in CLI flags * Changed type to sdk.Coin * Updated CHANGELOG.md * Implemented escrow accounts for keychain fees * Added CHANGELOG.md line * Regenerated keychain snapshot * Updated keychain deduction fee code * Added test case for fee deduction with --max-keychain-fee * Fixed actions auto parsing for cli * Fixed bug with not forking fees deduction and context missing values * Docker versions update * Update cmd/wardend/cmd/gen-keychains.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Proto rebuild * Merged changes from #560 * Code-review fixes * Implemented integration test for escrow account * Code review fixes * Made KeychainFees = nil invalid * Fixed error message * Keychain fee validations * Code review fixes * Merge conflict fixes * Lint fixes --------- Co-authored-by: Antonio Pitasi <antonio@pitasi.dev> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(spaceward): update for rebrand (#748) * feat(spaceward): New theme added * fix(spaceward): Some theme fixes * fix(spaceward): Fixed keychains * fix(spaceward): Some theme fixes and metamask * fix(spaceward): Some more * fix(spaceward): Space Mono * fix(spaceward): fix for space mono font issue (#750) * fix(spaceward): updated welcome message (#751) * fix(faucet): fix daily limit i64 (#755) This fixes issue where user puts high uward amount to daily limit and go cannot parse it due to int restrictions * Add chain upgrade instructions for v0.4.2 (#757) * Add chain upgrade instructions for v0.4.2 * Update v0.4.2.md * update v0.4.2 Co-authored-by: Joonas Lehtimäki <joonas.lehtimaki@gmail.com> * remove reference to side-car --------- Co-authored-by: Joonas Lehtimäki <joonas.lehtimaki@gmail.com> * fixed spelling (#758) * fixed spelling * deleted the tip, fixed spelling * update slinky * fix(x/act): simplify prepareHandlerContext to always use sdk.Context * updated gov proto; fix governance types (#761) * Fix returned field (#765) * fix(spaceward): Fix logos (#767) * fix(spaceward): Fix logos * fix(spaceward): updated logo for WC connection and updated hover fill colour --------- Co-authored-by: Jon Heywood <94459819+jjheywood@users.noreply.github.com> * fix(faucet): tx page loading (#768) * feat(faucet): add new metric dailySupply * fix(faucet): page keeps loading even if tx was finished --------- Co-authored-by: backsapc <backsapce@hotmail.com> Co-authored-by: Archie <artur.abliazimov@gmail.com> Co-authored-by: Joonas Lehtimäki <joonas.lehtimaki@gmail.com> Co-authored-by: Jon Heywood <94459819+jjheywood@users.noreply.github.com> Co-authored-by: Antonio Pitasi <antonio@pitasi.dev> Co-authored-by: Aliasgar Merchant <44069404+alijnmerchant21@users.noreply.github.com> Co-authored-by: Margarita Skomorokh <ijonele@gmail.com> Co-authored-by: Margarita Skomorokh <margaret.skomorokh@anychart.com> Co-authored-by: Alex D <alex.d.nax@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Alice <37526212+laniakea42@users.noreply.github.com> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Refactor
NewGenesisStatefunction, streamlining genesis state initialization to rely solely on default values.NewMsgSetParamsfunction, altering how instances ofMsgSetParamsare created.Paramsstruct, which may affect functionality relying on these defaults.