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 from Big Endian to Small Endian #1338

Closed
terencechain opened this issue Jan 17, 2019 · 4 comments
Closed

Switch from Big Endian to Small Endian #1338

terencechain opened this issue Jan 17, 2019 · 4 comments
Assignees
Labels
Discussion Simply a thread for talking about stuff Priority: Low Low priority item

Comments

@terencechain
Copy link
Member

terencechain commented Jan 17, 2019

Looks the community has favored using small endian over big endian. Let's update that in our code base: ethereum/consensus-specs#139

This applies to SSZ and another potential location is: https://github.com/prysmaticlabs/prysm/blob/master/shared/bytes/bytes.go

@terencechain terencechain added Good First Issue Good for newcomers Priority: Low Low priority item labels Jan 17, 2019
@nisdas
Copy link
Member

nisdas commented Jan 17, 2019

This is only for SSZ though, unless you want to apply this across the whole repo ?

@terencechain terencechain added Discussion Simply a thread for talking about stuff and removed Good First Issue Good for newcomers labels Jan 17, 2019
@terencechain
Copy link
Member Author

terencechain commented Jan 17, 2019

Definitely for SSZ, but we should discuss whether we want to switch to small endian for the whole repo. Not sure which way I'm leaning yet

@nisdas
Copy link
Member

nisdas commented Jan 17, 2019

I dont know if its the best to apply this across the whole repo.
Ex: Log data from the POW chain stores its integers in big-endian. It can be confusing keeping in mind all these places where it is big-endian. I am leaning towards defaulting to big-endian across our repo excluding SSZ. Afaik there is no where in our repo where we use little endian

@houjieth
Copy link
Contributor

Let me fix the ssz part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Simply a thread for talking about stuff Priority: Low Low priority item
Projects
None yet
Development

No branches or pull requests

5 participants