-
Notifications
You must be signed in to change notification settings - Fork 20.3k
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
Do not use uint
in consensus datastructures.
#16750
Comments
It's actually |
Hmm, however, all consensus data is encoded into RLP, which truncates any integer to it's minimal size. So |
Ah, if the serialization scheme encodes the same number in both uint32 and uint64 to the same encoded byte array then this isn't a big problem. I still recommend being explicit rather than relying on this fact as a style/problem-avoidance scheme, but that is much less critical than I originally thought. |
I concur that it's generally a good idea to keep protocol fields as strict as possible. |
@MicahZoltu Actually there is a convert function to fill But for more strict field definition purpose, i will submit a PR to eliminate misleading. |
https://github.com/ethereum/go-ethereum/blob/master/core/types/receipt.go#L49
https://golang.org/pkg/builtin/#uint
uint
in golang isn't guaranteed to be exactly 32-bits, so it should not be used in a datastructure that needs to be serialized or hashed since future compilers may use uint64 under the hood.The text was updated successfully, but these errors were encountered: