Skip to content
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

refactor: use address.Codec to replace deprecated function #7797

Closed
wants to merge 1 commit into from

Conversation

1ch0
Copy link
Contributor

@1ch0 1ch0 commented Dec 24, 2024

Description

This PR replaces the deprecated address-related functions with the new address.Codec implementations:

  • Replace sdk.AccAddressFromBech32() with address.Codec StringToBytes()
  • Replace sdk.AccAddress.String() with address.Codec BytesToString()`

These changes align with the latest SDK address handling practices and remove usage of deprecated functions.

Copy link
Contributor

@gjermundgaraba gjermundgaraba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for the PR! I think there is a simpler way of doing this with the SDK. If you look at the bank module cli as an example: https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/bank/client/cli/tx.go

I am not sure if this has been set up in our simapps yet, but it might also require some upgrade docs to make sure the client context always have this set up (especially since we don't support depinject).

@1ch0
Copy link
Contributor Author

1ch0 commented Dec 27, 2024

Hi! Thanks for the PR! I think there is a simpler way of doing this with the SDK. If you look at the bank module cli as an example: https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/bank/client/cli/tx.go

I am not sure if this has been set up in our simapps yet, but it might also require some upgrade docs to make sure the client context always have this set up (especially since we don't support depinject).

Thank you so much for taking the time to review my PR and for pointing me to the relevant example! I’ve only recently started learning about Cosmos, so I really appreciate your feedback and guidance. I’m excited to keep contributing to this project and to learn more from the community. Looking forward to collaborating more in the future!

@1ch0 1ch0 closed this Dec 27, 2024
@1ch0
Copy link
Contributor Author

1ch0 commented Dec 27, 2024

Hi! Thanks for the PR! I think there is a simpler way of doing this with the SDK. If you look at the bank module cli as an example: https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/bank/client/cli/tx.go

I am not sure if this has been set up in our simapps yet, but it might also require some upgrade docs to make sure the client context always have this set up (especially since we don't support depinject).

#7802 Changed by reference example: https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/bank/client/cli/tx.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants