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

(Bug?) ResponseQuery Data changed from Value to Key #156

Open
dferendo opened this issue Jan 2, 2024 · 4 comments
Open

(Bug?) ResponseQuery Data changed from Value to Key #156

dferendo opened this issue Jan 2, 2024 · 4 comments
Assignees

Comments

@dferendo
Copy link

dferendo commented Jan 2, 2024

We noticed from testing that the data encoded in the ResponseQuery on the host side was changed to be stored into Key instead of Value when changing versions from v7.0.0 to v7.1.1. The tests were ran using both osmosis and juno, unfortunately cannot provide the e2e test since it is in a private repo currently. We can spin up a test on this repo for further proof if required.

There was no mention of this change in the CHANGELOG and we were wondering whether this change was intentional. Thanks!

@Reecepbcups Reecepbcups self-assigned this Jan 2, 2024
@Reecepbcups
Copy link
Member

Reecepbcups commented Jan 2, 2024

@dferendo Thanks for the report. This change was not intentional as I can see.
The only file that was modified with any reference to abci.ResponseQuery is:

git diff modules/async-icq/v7.0.0 modules/async-icq/v7.1.1 -- modules/async-icq/keeper/relay.go > ~/Desktop/relay.diff

image

A new applyFuncIfNoError was added but nothing in the function indicates to me why it would swap key / value pairings here. executeQuery remains untouched... hmm

Did the function previously panic on v7.0.0?

@dferendo
Copy link
Author

dferendo commented Jan 3, 2024

No, the function was a normal balance query and the data is still correct, just stored in the wrong variable.

@dferendo
Copy link
Author

dferendo commented Jan 3, 2024

@Reecepbcups

Looking more into it, it seems that SerializeCosmosResponse is returning different bytes between the two versions. We checked the data in this unit test.

v7.0.0 returns
eyJkYXRhIjoiQ2dRNkFoSUEifQ==

v7.1.1 returns
eyJkYXRhIjoiQ2dReUFoSUEifQ==

We used base64 encoding to obtain the above base64.StdEncoding.EncodeToString(bz).

@migueldingli1997
Copy link

Turns out this particular unit test is mostly likely returning different bytes because of the address which is different for each test run. That being said we're still experiencing this issue but haven't narrowed it down enough.

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

No branches or pull requests

3 participants