-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: rename UMEE token to UX token part1 #2328
Conversation
WalkthroughThe recent changes streamline and enhance the handling of token metadata and denominations within the application. By centralizing the creation of Changes
Poem
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (3)
- app/modules.go (2 hunks)
- app/params/app_settings.go (1 hunks)
- app/upgrades.go (2 hunks)
Additional comments: 5
app/modules.go (3)
31-40: The renaming of the token from UMEE to UX is reflected here. Ensure that the
appparams.DisplayDenom
is correctly set to "UX" in theapp/params/app_settings.go
file. Also, verify that theBase
field, which must not change, is still correctly set to the base denomination (e.g., "uumee" or "uux" if that has been changed as well).43-51: The update includes handling for both the new and legacy denominations. Ensure that the
appparams.BaseExtraDenom
is set to the new base denomination (e.g., "uux") and thatappparams.LegacyDisplayDenom
is set to "UMEE" for backward compatibility. This is important for a smooth transition and to avoid breaking changes for existing users and integrations.57-62: The
DefaultGenesis
function has been updated to include the new token metadata in the genesis state. This is a critical part of the renaming operation, as it ensures that the new token metadata is included from the start of the blockchain. Ensure that theumeeTokenMetadata
function is called correctly and that theDenomMetadata
is properly appended to thegenState
.app/upgrades.go (2)
115-125: The migration logic for the UMEE to UX token metadata is critical and should be carefully reviewed to ensure that it correctly updates the metadata for the new UX token. It's important to verify that the
umeeTokenMetadata()
function generates the correctbanktypes.Metadata
structure for the UX token. Additionally, the new parameterMinInitialDepositRatio
is being set for the governance module, which should be checked for correctness and compatibility with the Cosmos SDK 0.47 governance parameters.131-135: The migration logic for the uibc module includes a call to
MigrateTotalOutflowSum()
andSetParams(uibc.DefaultParams())
. It's important to ensure that these methods are correctly implemented and that they handle the migration of any state or parameters necessary for the uibc module to function correctly with the new UX token. Additionally, error handling is present, but it's crucial to verify that all potential errors are being handled appropriately and that the migration will not leave the system in an inconsistent state if an error occurs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- app/params/app_settings.go (1 hunks)
Additional comments: 1
app/params/app_settings.go (1)
- 11-26: The changes reflect the renaming of the UMEE token to UX token. The addition of
BaseExtraDenom
andLegacyDisplayDenom
constants, as well as the update toDisplayDenom
, are consistent with the pull request's goal to update the token's metadata while maintaining the base stake denomination (uumee
). These changes should be cross-verified with other parts of the codebase to ensure that the new constants are used correctly and that the renaming does not affect the functionality of the blockchain, especially in areas where the denomination is used for calculations or transactions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (3)
- app/params/app_settings.go (2 hunks)
- app/params/app_settings_test.go (1 hunks)
- app/params/prefix.go (1 hunks)
Files skipped from review due to trivial changes (1)
- app/params/prefix.go
Additional comments: 1
app/params/app_settings.go (1)
- 11-26: The introduction of
BaseExtraDenom
,DisplayDenom
, andLegacyDisplayDenom
constants is a clear way to manage the transition from "UMEE" to "UX". It's important to ensure that all parts of the application that use these constants are updated accordingly to reflect the new naming convention. The comment on line 14 is especially important as it highlights the need to maintainBondDenom
for IBC compatibility, which is a good practice to avoid breaking changes in inter-blockchain communication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (10)
- app/modules.go (1 hunks)
- app/params/app_settings.go (2 hunks)
- app/test_helpers.go (3 hunks)
- app/upgrades.go (3 hunks)
- tests/cli/suite.go (1 hunks)
- tests/e2e/setup/setup.go (1 hunks)
- x/leverage/client/tests/tests.go (3 hunks)
- x/leverage/types/token.go (1 hunks)
- x/oracle/abci_test.go (1 hunks)
- x/oracle/client/tests/suite.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/upgrades.go
Additional comments: 15
app/params/app_settings.go (4)
- 15-17: The comment on
BondDenom
correctly emphasizes its importance for IBC operations, ensuring awareness of the implications of changing this value.- 20-22: Renaming
DisplayDenom
to "UX" and introducingLegacyDisplayDenom
as "UMEE" aligns with the rebranding effort. Ensure all references to these denominations across the codebase are updated accordingly.- 45-48: Modifying the regex in
sdk.SetCoinDenomRegex
to allow 2-character denominations is a necessary change given the new "UX" denomination. This adjustment maintains the flexibility of denomination naming while adhering to the new requirements.- 51-72: The
UmeeTokenMetadata
function correctly sets up bank metadata for the UX token, ensuring consistency with the new denomination and symbol. The use of constants forBase
,Name
,Display
, andSymbol
fields ensures maintainability and clarity.tests/cli/suite.go (1)
- 66-67: Switching to
assert.DeepEqual
for comparingExpectedResponse
andResponse
is appropriate for ensuring a more thorough comparison in tests. The added logging statement enhances test diagnostics by providing context for the test name.app/modules.go (1)
- 36-36: Using
appparams.UmeeTokenMetadata()
to initializeumeeMetadata
in theDefaultGenesis
method centralizes token metadata definition, improving maintainability and consistency across the application.x/oracle/client/tests/suite.go (1)
- 180-180: Updating the
Denom
field comparison toappparams.LegacyDisplayDenom
in theTestQueryExchangeRates
function aligns with the rebranding changes and ensures the test reflects the current state of the application.x/leverage/types/token.go (1)
- 162-165: Retrieving token metadata from
appparams.UmeeTokenMetadata()
to setBaseDenom
andSymbolDenom
in thedefaultUmeeToken
function ensures consistency with the application's token configuration and centralizes metadata definition.x/oracle/abci_test.go (1)
- 27-27: Updating the
displayDenom
constant toappparams.LegacyDisplayDenom
in the test file aligns with the rebranding changes and ensures the test reflects the current state of the application.app/test_helpers.go (2)
- 253-255: The usage of
appparams.UmeeTokenMetadata()
is correct and aligns with the PR objectives to update token metadata as part of the rebranding effort.- 274-279: Correct use of
params.LegacyDisplayDenom
in setting mock exchange rates and historic medians for oracle operations. This change is consistent with the PR's goal to update token denominations without affecting IBC and Oracle operations.x/leverage/client/tests/tests.go (3)
- 94-94: Correct update from
DisplayDenom
toLegacyDisplayDenom
in test setup, aligning with the PR's rebranding objectives.- 116-116: The update to use
LegacyDisplayDenom
for registered token info in tests is appropriate and consistent with the rebranding effort.- 129-134: Adjustment of
SymbolDenom
toLegacyDisplayDenom
and update ofSupply_APY
value in market summary query test are correctly implemented, reflecting the rebranding changes.tests/e2e/setup/setup.go (1)
- 217-219: Correct implementation of using
appparams.UmeeTokenMetadata()
to set token metadata for a token fixture in e2e test setup. This change aligns with the PR's rebranding objectives.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2328 +/- ##
==========================================
- Coverage 75.38% 69.15% -6.24%
==========================================
Files 100 184 +84
Lines 8025 10776 +2751
==========================================
+ Hits 6050 7452 +1402
- Misses 1589 2714 +1125
- Partials 386 610 +224
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- .revive.toml (1 hunks)
- app/test_helpers.go (2 hunks)
- app/upgrades.go (5 hunks)
- app/wasm/query/whitelist.go (1 hunks)
- x/leverage/simulation/operations.go (7 hunks)
- x/oracle/keeper/keeper_test.go (1 hunks)
- x/oracle/simulations/operations.go (3 hunks)
Files skipped from review due to trivial changes (1)
- .revive.toml
Files skipped from review as they are similar to previous changes (2)
- app/test_helpers.go
- app/upgrades.go
Additional comments: 13
x/oracle/simulations/operations.go (3)
- 114-114: Ensure all calls to
SimulateMsgAggregateExchangeRatePrevote
have been updated to match the new function signature withoutchainID
.- 158-158: Ensure all calls to
SimulateMsgAggregateExchangeRateVote
have been updated to match the new function signature withoutchainID
.- 200-200: Ensure all calls to
SimulateMsgDelegateFeedConsent
have been updated to match the new function signature withoutchainID
.app/wasm/query/whitelist.go (1)
- 171-171: Ensure the removal of the
value
variable in theRange
function call does not impact the intended functionality of collecting whitelisted query paths.x/oracle/keeper/keeper_test.go (2)
- 31-31: Ensure the update to
displayDenom
usingappparams.LegacyDisplayDenom
is consistent with the intended test scenarios and does not affect the validity of the tests.- 33-33: Verify that the direct assignment of
initialPower
toint64(10000000000)
is appropriate for the test scenarios and does not introduce any unintended side effects.x/leverage/simulation/operations.go (7)
- 126-126: Ensure all calls to
SimulateMsgSupply
have been updated to match the new function signature withoutchainID
.- 144-144: Ensure all calls to
SimulateMsgWithdraw
have been updated to match the new function signature withoutchainID
.- 162-162: Ensure all calls to
SimulateMsgBorrow
have been updated to match the new function signature withoutchainID
.- 183-183: Ensure all calls to
SimulateMsgCollateralize
have been updated to match the new function signature withoutchainID
.- 205-205: Ensure all calls to
SimulateMsgDecollateralize
have been updated to match the new function signature withoutchainID
.- 223-223: Ensure all calls to
SimulateMsgRepay
have been updated to match the new function signature withoutchainID
.- 241-241: Ensure all calls to
SimulateMsgLiquidate
have been updated to match the new function signature withoutchainID
.
044a0da
to
4dc18fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (11)
- .golangci.yml (1 hunks)
- .revive.toml (1 hunks)
- app/test_helpers.go (2 hunks)
- app/upgrades.go (5 hunks)
- app/wasm/query/whitelist.go (1 hunks)
- x/leverage/simulation/operations.go (7 hunks)
- x/oracle/keeper/grpc_query_test.go (1 hunks)
- x/oracle/keeper/hooks_test.go (1 hunks)
- x/oracle/keeper/keeper_test.go (1 hunks)
- x/oracle/simulations/operations.go (3 hunks)
- x/uibc/quota/intest/suite_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (7)
- .revive.toml
- app/test_helpers.go
- app/upgrades.go
- app/wasm/query/whitelist.go
- x/leverage/simulation/operations.go
- x/oracle/keeper/keeper_test.go
- x/oracle/simulations/operations.go
Additional comments: 3
x/oracle/keeper/hooks_test.go (1)
- 15-15: Change from
appparams.DisplayDenom
toappparams.LegacyDisplayDenom
aligns with the renaming objective and maintains backward compatibility in tests.x/uibc/quota/intest/suite_test.go (1)
- 28-33: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
Verify that the removal of
displayDenom
constant doesn't break tests or references within this file or related test files.Verification successful
The search for references to
displayDenom
within thex/uibc/quota/intest/
directory did not find any occurrences. This suggests that within this specific directory, the removal of thedisplayDenom
constant does not directly impact any tests or references. However, this verification is limited to thex/uibc/quota/intest/
directory, and it's recommended to review other parts of the codebase for potential impacts of this change.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to `displayDenom` in test files to ensure they are updated or removed. rg --type go 'displayDenom' x/uibc/quota/intest/Length of output: 48
x/oracle/keeper/grpc_query_test.go (1)
- 121-121: Change from
appparams.DisplayDenom
toappparams.LegacyDisplayDenom
is consistent with the renaming initiative and maintains backward compatibility in tests.
Description
Changes:
Name
and Displayto
UX`uumee
(this will cause problems with IBC and transfers)UMEE
(this will cause errors with Oracle)ux
anduux
.This won't allow to query nor send tokens by alias, but will allow to show it in the metadata.
Changing base name creates many problems, which I will document in Notion.
Summary by CodeRabbit
uux
denom aliases for UMEE.BondDenom
.chainID
parameters in various modules.umeeTokenMetadata
function for improved code organization.TestDenoms
to validate several denominations.CHANGELOG.md
with recent changes.