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

Account-id type used as int64 instead of uint64 #13410

Closed
atheeshp opened this issue Sep 28, 2022 · 2 comments · Fixed by #13411 or #13780
Closed

Account-id type used as int64 instead of uint64 #13410

atheeshp opened this issue Sep 28, 2022 · 2 comments · Fixed by #13411 or #13780
Assignees
Labels
T: Proto Breaking Protobuf breaking changes: never don't do this! T: UX

Comments

@atheeshp
Copy link
Contributor

atheeshp commented Sep 28, 2022

Summary of Bug

int64 range : -2^63 to 2^63-1
uint64 range: 0 to 2^64-1

https://github.com/cosmos/cosmos-sdk/blob/main/x/auth/keeper/grpc_query.go#L22-L38
Here we are converting (ingnoring -ve values) the int64 values to uint64, that limits the range to 0 to 2^63-1

Version

main

Steps to Reproduce

@atheeshp
Copy link
Contributor Author

atheeshp commented Oct 6, 2022

re-opening this, since the changes reverted here #13460

@atheeshp atheeshp reopened this Oct 6, 2022
@amaury1093
Copy link
Contributor

amaury1093 commented Oct 6, 2022

What we could do is deprecate the int64 field, and create a new uint64 field in QueryAccountAddressByIDRequest. If the user queries the address using either one of those, then return the corresponding address. We only return an error if the user populates both fields with different values.

However, it's also low priority IMO, as we should still have some margin before acc nums overflow 2^63-1.

@amaury1093 amaury1093 added T: Proto Breaking Protobuf breaking changes: never don't do this! T: UX labels Oct 6, 2022
@atheeshp atheeshp changed the title Account-id type used as int64 instead of uint64 chore: Account-id type used as int64 instead of uint64 Nov 7, 2022
@atheeshp atheeshp changed the title chore: Account-id type used as int64 instead of uint64 Account-id type used as int64 instead of uint64 Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: Proto Breaking Protobuf breaking changes: never don't do this! T: UX
Projects
None yet
2 participants