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

Switch integer encoding to big-endian #22

Merged
merged 1 commit into from
Apr 22, 2020
Merged

Switch integer encoding to big-endian #22

merged 1 commit into from
Apr 22, 2020

Conversation

carver
Copy link
Contributor

@carver carver commented Apr 16, 2020

Thoughts on changing the integer encoding to big-endian?

Everywhere else that I know of uses big-endian (like all the storage, balance, nonce, etc fields). Is there a benefit to introducing a different encoding that I'm missing?

@AlexeyAkhunov
Copy link
Contributor

Yeah, it must have been a typo

@poemm
Copy link
Contributor

poemm commented Apr 16, 2020

Not a typo. Little-endian was chosen because <U32> represents a 32-bit unsigned integer, which most modern machines handle internally as little-endian. Using big-endian instead would require reversing bytes each time a <U32> is parsed or generated. Edit: Luckily, compilers can pattern-match to bswap.

There were similar endianness discussions in eth2 specs.

I consider this to be non-critical. Either way is fine. This change is OK with me. And it is OK to revert back to little-endian if people want this optimization later.

@poemm
Copy link
Contributor

poemm commented Apr 16, 2020

Another possibility is compressing <U32> with LEB128 encoding. (And avoid ambiguity in LEB128 by requiring the shortest encoding for each integer, but I am not sure whether this gets rid of all ambiguity.)

Edit: A similar variable-length encoding VLQ has a way to remove ambiguity.

@mandrigin
Copy link
Contributor

I will merge it for now and add an issue to define uint compression.

@mandrigin
Copy link
Contributor

the compression-related issue: #30

@mandrigin mandrigin merged commit ba17745 into master Apr 22, 2020
@mandrigin mandrigin deleted the int-big-endian branch April 22, 2020 12:43
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

Successfully merging this pull request may close these issues.

None yet

4 participants