Reinstate usage of some SDK functionality unique to our fork. #1930
Conversation
…the grant command, so there's a couple things that are still needed in the SDK.
…it. Remove the check-test stuff since we've also got build-tests which is easier for me to remember.
… it's actually a comment rather than a command that's printed.
…ame flow as make test, and also give it the same tags.
…terface since that interface doesn't have InputOutputCoinsProv.
…v instead of the old one.
…nce to one because there's some deep down-stream wiring stuff that assumes it's concrete.
…al test is commented out right now until the quarantine PR merges that has the needed query helper.
WalkthroughThis update harmonizes various adjustments across the codebase to enhance testing, build processes, and vital functionalities like pagination, governance proposals, and bank parameters. Notable changes involve restoring the hold module, refining CLI commands, and enhancing coin transfer mechanisms within the exchange module to align with the ongoing Cosmos SDK v0.50.x upgrade efforts. Changes
Possibly related issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Files skipped from review as they are similar to previous changes (3)
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 Configration File (
|
… so that once the quarantine PR is merged, this will be ready too.
# Conflicts: # app/app.go
![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
Actionable comments outside the diff hunks (6)
app/app.go (4)
Line range hint
994-994: Check the error return value ofapp.mm.RegisterServices.- app.mm.RegisterServices(app.configurator) + if err := app.mm.RegisterServices(app.configurator); err != nil { + panic(fmt.Errorf("failed to register services: %w", err)) + }It's important to handle errors returned by
RegisterServicesto avoid runtime panics due to unhandled issues during the service registration process.
Line range hint
1205-1205: Check the error return value ofapp.UpgradeKeeper.SetModuleVersionMap.- app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap()) + if err := app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap()); err != nil { + panic(fmt.Errorf("failed to set module version map: %w", err)) + }Proper error handling is crucial when setting the module version map to ensure that the application state is correctly initialized.
Line range hint
1428-1428: Check the error return value ofapp.UpgradeKeeper.ClearUpgradePlan.- app.UpgradeKeeper.ClearUpgradePlan(ctx) + if err := app.UpgradeKeeper.ClearUpgradePlan(ctx); err != nil { + panic(fmt.Errorf("failed to clear upgrade plan: %w", err)) + }It's important to handle errors when clearing the upgrade plan to ensure that the application does not enter an inconsistent state.
Line range hint
454-454: Correct the misuse of theappendfunction.- enabledSignModes := append(tx.DefaultSignModes, sigtypes.SignMode_SIGN_MODE_TEXTUAL) + enabledSignModes := append([]sigtypes.SignMode{}, tx.DefaultSignModes...) + enabledSignModes = append(enabledSignModes, sigtypes.SignMode_SIGN_MODE_TEXTUAL)Ensure that the
appenddoes not modify the originalDefaultSignModesslice. This change avoids potential side effects across different parts of the application whereDefaultSignModesmight be used.CHANGELOG.md (2)
Line range hint
64-300: Consistent list marker style needed.- - Bump `github.com/cosmos/ibc-go/v8` from 8.0.0 to 8.2.0 ([#1910](https://github.com/provenance-io/provenance/pull/1910)) + * Bump `github.com/cosmos/ibc-go/v8` from 8.0.0 to 8.2.0 ([#1910](https://github.com/provenance-io/provenance/pull/1910))Please replace all dashes (-) with asterisks (*) for list items to maintain a consistent style throughout the document.
Line range hint
175-596: Convert bare URLs to Markdown links.- https://github.com/provenance-io/provenance/issues/1760 + [https://github.com/provenance-io/provenance/issues/1760](https://github.com/provenance-io/provenance/issues/1760)Please update all bare URLs throughout the document to use Markdown link syntax for better readability and consistency.
# Conflicts: # CHANGELOG.md # app/upgrades.go # app/upgrades_test.go
SpicyLemon
dismissed stale reviews from nullpointer0x00 and Taztingo
via
9c30aa5
There was a problem hiding this comment.
Actionable comments posted: 0
Actionable comments outside the diff hunks (2)
CHANGELOG.md (2)
Line range hint
66-303: Consider using a consistent list marker style in markdown.- - Bump `github.com/cosmos/ibc-go/v8` from 8.0.0 to 8.2.0 ([#1910](https://github.com/provenance-io/provenance/pull/1910)) + * Bump `github.com/cosmos/ibc-go/v8` from 8.0.0 to 8.2.0 ([#1910](https://github.com/provenance-io/provenance/pull/1910))Change all list markers from dashes (-) to asterisks (*) for consistency throughout the document.
Line range hint
177-598: Ensure all URLs are properly formatted with markdown links.- https://github.com/provenance-io/provenance/issues/1772 + [https://github.com/provenance-io/provenance/issues/1772](https://github.com/provenance-io/provenance/issues/1772)Apply this change to all bare URLs throughout the document to improve readability and accessibility.
# Conflicts: # CHANGELOG.md # app/upgrades.go # app/upgrades_test.go
There was a problem hiding this comment.
Actionable comments posted: 0
Actionable comments outside the diff hunks (5)
CHANGELOG.md (5)
Line range hint
69-306: Consider using a consistent list marker style in Markdown.- - + *
Line range hint
769-769: Remove consecutive blank lines to maintain a clean and consistent format.-
Line range hint
180-180: Convert bare URLs to Markdown links for better readability and to provide clickable links in Markdown viewers.- https://github.com/provenance-io/provenance/issues/1760 + [v0.50 epic](https://github.com/provenance-io/provenance/issues/1760)
Line range hint
897-897: Remove spaces inside emphasis markers to fix Markdown formatting.- * message PR <pull-request-number> * + *message PR <pull-request-number>*
Line range hint
293-293: Remove spaces inside code span elements to ensure correct rendering.- ` MsgAddMarkerRequest ` + `MsgAddMarkerRequest`
There was a problem hiding this comment.
Actionable comments posted: 1
Actionable comments outside the diff hunks (2)
CHANGELOG.md (2)
Line range hint
146-148: Ensure consistency in list item markers, prefer using asterisks (*) over dashes (-) for unordered lists.- * Remove unsupported database types [#1760](https://github.com/provenance-io/provenance/issues/1760). - * Update ibc and migrate params [#1760](https://github.com/provenance-io/provenance/issues/1760). - * Replace ModuleBasics with BasicModuleManager [#1760](https://github.com/provenance-io/provenance/issues/1760). + * Remove unsupported database types [#1760](https://github.com/provenance-io/provenance/issues/1760). + * Update ibc and migrate params [#1760](https://github.com/provenance-io/provenance/issues/1760). + * Replace ModuleBasics with BasicModuleManager [#1760](https://github.com/provenance-io/provenance/issues/1760).
Line range hint
206-208: Ensure consistency in list item markers, prefer using asterisks (*) over dashes (-) for unordered lists.- * Remove deleted marker send deny entries [#1666](https://github.com/provenance-io/provenance/issues/1666). - * Update protos, naming, and documentation to use mills for NAVs [#1813](https://github.com/provenance-io/provenance/issues/1813). - * Update marker transfer to work with groups [#1818](https://github.com/provenance-io/provenance/issues/1818). + * Remove deleted marker send deny entries [#1666](https://github.com/provenance-io/provenance/issues/1666). + * Update protos, naming, and documentation to use mills for NAVs [#1813](https://github.com/provenance-io/provenance/issues/1813). + * Update marker transfer to work with groups [#1818](https://github.com/provenance-io/provenance/issues/1818).
Description
Note: The hold cli tests will fail to build until #1926 has been merged. I'll keep this PR in draft until then.
This PR bumps the SDK to
v0.50.5-pio-3(fromv0.50.5-pio-2).This PR also takes care of a handful of TODOs in the v0.50 epic:
TODO[1760]: cli: ReadPageRequestWithPageKeyDecodedTODO[1760]: Copy needed stuff out of simapp and remove this require.(Removes import ofcosmossdk.io/simapp.)TODO[1760]: count-authz(Reinstate several tests that use the CountAuthorization.)TODO[1760]: gov-cli(Restores several commands that submit gov props.)TODO[1760]: locked-coins(Restore thex/holdmodule functionality.)TODO[1760]: exchange(Switch back toInputOutputCoinsProvfor exchange transfers.)make test-unit-amino.Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passesSummary by CodeRabbit
New Features
Bug Fixes
addr1Esrowtoaddr1Escrow.Refactor
BaseKeeperinstead ofKeeper.InputOutputCoinsmethod toInputOutputCoinsProvin the BankKeeper interface for better exchange transfers.Documentation
Tests