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

Use buffer reading functions #80

Merged
merged 9 commits into from
Mar 22, 2024

Conversation

spalmer25
Copy link
Collaborator

This PR use function on buffer.
This makes the code clearer and closer to what is done in LedgerHQ/app-boilerplate

It prepares issue #74

Depends on #79

@spalmer25 spalmer25 added this to the Baking app upgrade milestone Mar 19, 2024
@spalmer25 spalmer25 self-assigned this Mar 19, 2024
@spalmer25 spalmer25 marked this pull request as ready for review March 19, 2024 16:33
@spalmer25 spalmer25 force-pushed the palmer@functori@io-standard-process branch 3 times, most recently from d896d17 to 72490ac Compare March 21, 2024 14:56
The parse_next_type function is used to parse a wire structure.

It fills in nexttype_subparser_state.body using
nexttype_subparser_state.body.raw, then casts the data with the
expected type.

To be able to parse a wire structure using this function, the
structure must be in the body union. Otherwise, the parser may not
have enough space to contain the entire structure.

The largest structure in nexttype_subparser_state.body is
operation_group_header (32 bytes), the nexttype_subparser_state.body
can therefore contain a maximum of 32 bytes.

As the size of the public key depends on the key curve, no fixed size
structure was added to nexttype_subparser_state.body.

But since a public key can contain 33 bytes, a byte was missed in
nexttype_subparser_state.body.

This commit adds a new structure to be able to parse the public key
@spalmer25 spalmer25 force-pushed the palmer@functori@use-buffer-reading-functions branch from 2c7b7ff to fa77829 Compare March 22, 2024 10:13
@ajinkyaraj-23
Copy link
Collaborator

Can you rebase on main? it will be easier to review.

@ajinkyaraj-23
Copy link
Collaborator

Also if its not draft stage, please change the target branch to main

@spalmer25 spalmer25 changed the base branch from palmer@functori@io-standard-process to main March 22, 2024 12:03
@spalmer25 spalmer25 requested a review from ajinkyaraj-23 March 22, 2024 12:07
Copy link
Collaborator

@ajinkyaraj-23 ajinkyaraj-23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ajinkyaraj-23 ajinkyaraj-23 merged commit f9f17dd into main Mar 22, 2024
29 checks passed
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.

2 participants