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

AccAddress caching causes incorrect bech32 prefixes #15317

Closed
KyleMoser opened this issue Mar 9, 2023 · 1 comment · Fixed by #15433
Closed

AccAddress caching causes incorrect bech32 prefixes #15317

KyleMoser opened this issue Mar 9, 2023 · 1 comment · Fixed by #15433
Assignees
Labels

Comments

@KyleMoser
Copy link
Contributor

KyleMoser commented Mar 9, 2023

Summary of Bug

AccAddress caching (the caches in types/address.go, including accAddrCache) cause unexpected behavior when printing bech32 account addresses on clients. For example, it may print an address prefixed with "cosmos" when you expected "osmo". An example client that experiences this buggy behavior is the go relayer.

Furthermore, you cannot always work around this behavior since many internal SDK functions call .String() on the AccAddress. For example, this is the case during Feegranting.

Version

Every version I could find

Steps to Reproduce

It's important to note that the bug requires using the same mnemonic on multiple chains.

  1. Set the SDK account prefixes to "cosmos"
  2. Print out an AccAddress.String() and it will start with "cosmos"
  3. Set the SDK account prefixes to "osmo"
  4. Print out the same AccAddress.String() and it will still start with "cosmos"

There are two plausible ways I can think of to fix this bug.

  1. Add a function to the SDK that allows disabling the account caches, e.g. SetAccountCacheEnabled(bool).
  2. Any time the bech32 prefixes are reset (e.g. SetBech32PrefixForAccount is called) clear the caches.

I think fix 1 is a much better solution as it gives clients more control, I am working on a PR to address this bug.

@KyleMoser KyleMoser self-assigned this Mar 9, 2023
@github-project-automation github-project-automation bot moved this to 📝 Todo in Cosmos-SDK Mar 9, 2023
@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Mar 9, 2023
@KyleMoser KyleMoser mentioned this issue Mar 9, 2023
19 tasks
@tac0turtle
Copy link
Member

thanks for opening this issue, as part of the q1 work we will be removing this global in favour of interface to decode and encode that sets the desired prefix.

This will most likely land in 0.48, so in 0.47 we can think of accepting a pr to fix the issue for the go relayer

@tac0turtle tac0turtle added C:x/auth and removed needs-triage Issue that needs to be triaged labels Mar 9, 2023
@KyleMoser KyleMoser mentioned this issue Mar 16, 2023
19 tasks
@github-project-automation github-project-automation bot moved this from 📝 Todo to 👏 Done in Cosmos-SDK Mar 31, 2023
@tac0turtle tac0turtle moved this to 🥳 Done in Cosmos-SDK Nov 16, 2023
@tac0turtle tac0turtle removed this from Cosmos-SDK Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants