8000 [ABCI]: replace int64 with uint64 where possible · Issue #2059 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[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

Closed
Tracked by #9
tac0turtle opened this issue Jan 17, 2024 · 4 comments
Closed
Tracked by #9

[ABCI]: replace int64 with uint64 where possible #2059

tac0turtle opened this issue Jan 17, 2024 · 4 comments
Labels
abci Application blockchain interface enhancement New feature or request
Milestone

Comments

@tac0turtle
Copy link
Contributor

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.

@tac0turtle tac0turtle added enhancement New feature or request needs-triage This issue/PR has not yet been triaged by the team. labels Jan 17, 2024
@sergio-mena sergio-mena removed the needs-triage This issue/PR has not yet been triaged by the team. label Jan 17, 2024
@melekes melekes added the abci Application blockchain interface label Jan 31, 2024
@melekes melekes added this to CometBFT Jan 31, 2024
@github-project-automation github-project-automation bot moved this to Todo in CometBFT Jan 31, 2024
@adizere adizere added this to the 2024-Q2 milestone Jan 31, 2024
@adizere adizere assigned adizere and glnro and unassigned adizere Mar 5, 2024
@adizere
Copy link
Member
adizere commented Mar 11, 2024

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

@tac0turtle
Copy link
Contributor Author

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. 🤷

@github-project-automation github-project-automation bot moved this from Todo to Done in CometBFT Mar 11, 2024
@tac0turtle tac0turtle reopened this Mar 11, 2024
@tac0turtle
Copy link
Contributor Author
tac0turtle commented Mar 11, 2024

sorry wrong button. this shouldnt bother us in the Cosmos sdk pre soon

@adizere adizere moved this from Done to Blocked in CometBFT Mar 27, 2024
@adizere
Copy link
Member
adizere commented Apr 1, 2024

Closing as not planned. We might reconsider later.

@adizere adizere closed this as not planned Won't fix, can't repro, duplicate, stale Apr 1, 2024
@github-project-automation github-project-automation bot moved this from Blocked to Done in CometBFT Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abci Application blockchain interface enhancement New feature or request
Projects
No open projects
Status: Done
Development

No branches or pull requests

5 participants
0