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

Add LIP defining a generic serialization for transactions #49

Merged
merged 9 commits into from Mar 18, 2020

Conversation

@IkerAlus
Copy link
Member

IkerAlus commented Mar 9, 2020

Add a new LIP defining a generic serialization method for transactions.

@IkerAlus IkerAlus added the new lip label Mar 9, 2020
@IkerAlus IkerAlus requested review from shuse2, ricott1 and AndreasKendziorra Mar 9, 2020
@IkerAlus IkerAlus self-assigned this Mar 9, 2020
IkerAlus added 3 commits Mar 9, 2020
Copy link
Contributor

ricott1 left a comment

Looks all good to me, maybe the only thing is to remove the leading 0x from the signatures and in general bytes values in the bottom example. I think we had some discussion with @AndreasKendziorra and @MaximeGagnebin for the generic serialization LIP, and we decided to use 0x only for numbers. Maybe @AndreasKendziorra can confirm this?

@AndreasKendziorra

This comment has been minimized.

Copy link
Contributor

AndreasKendziorra commented Mar 10, 2020

Looks all good to me, maybe the only thing is to remove the leading 0x from the signatures and in general bytes values in the bottom example. I think we had some discussion with @AndreasKendziorra and @MaximeGagnebin for the generic serialization LIP, and we decided to use 0x only for numbers. Maybe @AndreasKendziorra can confirm this?

We don't have to be strict about it. I think in this case there is no confusion. The case where I found the usage of "0x" confusing was the little endian varint encoding of an integer which could be false misinterpreted as the regular hex representation of the integer.

@IkerAlus IkerAlus requested a review from karmacoma Mar 11, 2020
@shuse2
shuse2 approved these changes Mar 11, 2020
karmacoma added 5 commits Mar 18, 2020
@karmacoma karmacoma merged commit a714ae7 into master Mar 18, 2020
@karmacoma karmacoma deleted the IkerAlus-transaction_serialization branch Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.