-
Notifications
You must be signed in to change notification settings - Fork 636
[ABCI]: replace int64 with uint64 where possible #2059
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
Comments
Hey @tac0turtle we discussed this briefly in the team. There's hesitation to move to uint, on the same grounds as the prior decision documented here: https://blog.cosmos.network/choosing-a-type-for-blockchain-height-beware-of-unsigned-integers-714804dddf1d |
i guess tendermint and comet will be the only consensus protocol in blockchain written with ints. When this came up in the past with the previous comet team even they were shocked int64 is used. 🤷 |
sorry wrong button. this shouldnt bother us in the Cosmos sdk pre soon |
Closing as not planned. We might reconsider later. |
Feature Request
Summary
Throughout the years there have been a few attempts to migrate tendermint (now cometbft) to uint64. Some of these attempts fell through due to the amount of changes. This issue is not about the repo but about ABCI. It would be nice to have all the values that can not be negative be transitioned to uint64. We in the Cosmos SDK, do overflow and underflow checks in order to work with ABCI. This isnt a problem today but a quality of life improvement would be to change those values to uint64
Problem Definition
ABCI uses int64 instead of uint64, this forces applications to handle negative cases or the application opts to use int64. In the Cosmos SDK we are aiming to use uint64 every where we can, for this to happen we need to be defensive when writing translations between int64 and uint64.
Proposal
Migrate all values in ABCI which use int64 to use uint64 if they do not need negative numbers.
The text was updated successfully, but these errors were encountered: