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.

Sign up for GitHub

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

Jump to bottom

Removed the data field from canonical serialization and deserialization of receipts #505

Merged
merged 2 commits into from
Jul 12, 2023
Merged

Removed the data field from canonical serialization and deserialization of receipts #505

merged 2 commits into from
Jul 12, 2023

Conversation

xgreenx
Copy link
Collaborator

@xgreenx xgreenx commented Jul 11, 2023 •
edited
Loading

  • The data field is optional now and is not recovered during canonical deserialization.
  • The len field is not padded anymore.

ref FuelLabs/fuel-core#1240

@xgreenx
Removed the data field from canonical serialization and deserializa…
cf6a6b9 
…tion of receipts

The `data` field is optional now and is not recovered during the canonical deserialization.
The `len` field is not padded anymore.
@xgreenx xgreenx added the bug Something isn't working label Jul 11, 2023
@xgreenx xgreenx requested a review from a team July 11, 2023 19:47
@xgreenx xgreenx self-assigned this Jul 11, 2023
@xgreenx xgreenx enabled auto-merge July 11, 2023 20:20
@xgreenx xgreenx disabled auto-merge July 11, 2023 20:20
@MitchTurner
Copy link
Member

LGTM! Can I ask what was wrong with the padded len?

@xgreenx
Copy link
Collaborator Author

xgreenx commented Jul 11, 2023 •
edited
Loading

Padding only len without padding data means that the digest also is incorrect because digest doesn't match the len. Also, the specification doesn't say that we need to pad it.

Plus, padding to 8 alignment is needed only for canonical serialization/deserialization.

@xgreenx xgreenx added this pull request to the merge queue Jul 12, 2023
Merged via the queue into master with commit 18a2e4a Jul 12, 2023
@xgreenx xgreenx deleted the bugfix/receipt-data-field-canonical branch July 12, 2023 11:02
@xgreenx xgreenx mentioned this pull request Jul 13, 2023
Merged
@Torres-ssf Torres-ssf mentioned this pull request Jul 21, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Voxelot Voxelot Voxelot approved these changes

Assignees

@xgreenx xgreenx

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xgreenx @MitchTurner @Voxelot