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

fix: not appending to buffer on multiple ops #31

Closed
wants to merge 1 commit into from

Conversation

dj8yfo
Copy link
Contributor

@dj8yfo dj8yfo commented Nov 6, 2023

This affects the behaviour when signing multiple tx-s WITHOUT quitting near app between operations:
Below are signatures of the very same message:

  • ledger prior to fix

    • first after entering app: 8f38d3db42410b03a63110f58a0c84eaf39a22dbe4e9ca866b406347342e897fa181fd0f9dbfcd0b1d4bbec1369f0f30bce3fa393a034670c8ad2c2cee24af02
    • second after enter: 71a89d556194d61d49691dfdc0f3921dcdf2039c0995999b8502e43b96ea830dc00d6f53870dfc0d0d382f9a9d3908f4b8525cedc9e05408a37bff009b42fa0d
    • third after enter: throws error SW_BUFFER_OVERFLOW 0x6990
  • ledger after fix

    • first after entering app: 8f38d3db42410b03a63110f58a0c84eaf39a22dbe4e9ca866b406347342e897fa181fd0f9dbfcd0b1d4bbec1369f0f30bce3fa393a034670c8ad2c2cee24af02
    • second after enter: 8f38d3db42410b03a63110f58a0c84eaf39a22dbe4e9ca866b406347342e897fa181fd0f9dbfcd0b1d4bbec1369f0f30bce3fa393a034670c8ad2c2cee24af02
    • third after enter: 8f38d3db42410b03a63110f58a0c84eaf39a22dbe4e9ca866b406347342e897fa181fd0f9dbfcd0b1d4bbec1369f0f30bce3fa393a034670c8ad2c2cee24af02

@dj8yfo
Copy link
Contributor Author

dj8yfo commented Nov 6, 2023

@tdejoigny-ledger please take a look

@tdejoigny-ledger
Copy link
Contributor

hello @dj8yfo , do you have a discord account ?

@@ -360,6 +360,7 @@ static void add_chunk_data(const uint8_t *input_data, size_t input_length)
// PRINTF("data_size: %d\n", input_length);
if (tmp_ctx.signing_context.buffer_used + input_length > MAX_DATA_SIZE)
{
tmp_ctx.signing_context.buffer_used = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#32 is fast forward extension of this pr, as it depends on signing context reset on overflow in this line,
when first attempting to sign a too long message normally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the same effect as of this pr, is achieved in #32 by commit

@dj8yfo dj8yfo mentioned this pull request Nov 10, 2023
9 tasks
@tdejoigny-ledger
Copy link
Contributor

hello @dj8yfo, I noticed that you were making some contributions to the Near app. It could be good if you can join the dedicated Near channel on our discord, it will be easier to discuss about your PRs and the app process.

@dj8yfo
Copy link
Contributor Author

dj8yfo commented Nov 13, 2023

@tdejoigny-ledger , sure.
Can you post a link here?
I'll create a pr to attach it to repo's README, as a nice place for cache instead of a local one on my machine.

upd: probably not relevant here, as the community appears not to be public by default

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