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

1.7.4 final #10

Merged
merged 3 commits into from
Jun 24, 2024
Merged

1.7.4 final #10

merged 3 commits into from
Jun 24, 2024

Conversation

aido
Copy link

@aido aido commented Jun 19, 2024

Checklist

  • App update process has been followed
  • Target branch is develop
  • Application version has been bumped

Description

This PR bumps the version of app-seed-tool to v1.7.4

v1.7.4 fixes issues aido#32 and aido#36.

While working on v1.7.4 the following was observed:

  1. The documentation for the cx_bn_gf2_n_mul() syscall should probably be updated to state that the operands and result should not overlap. Maybe something similar to what is already documented for the BN bit manipulation syscalls:
    https://github.com/LedgerHQ/ledger-secure-sdk/blob/c6d481025144ab2a57b904e98f796f3d5c1d83fc/include/ox_bn.h#L356

  2. Perhaps this rule should even be enforced by changing:
    https://github.com/LedgerHQ/ledger-secure-sdk/blob/c6d481025144ab2a57b904e98f796f3d5c1d83fc/src/syscalls.c#L638
    to something like the following:

return (r == a || r == b)
            ? CX_INVALID_PARAMETER
            : SVC_cx_call(SYSCALL_cx_bn_gf2_n_mul_ID, parameters);
  1. The documentation for the cx_bn_cnt_bits() syscall is misleading. The documentation suggests that the syscall returns the number of bits set in a big number:
    https://github.com/LedgerHQ/ledger-secure-sdk/blob/c6d481025144ab2a57b904e98f796f3d5c1d83fc/include/ox_bn.h#L495
    This instead should probably state that the syscall returns the number of significant bits in a big number.
    See Incorrect value returned from cx_bn_cnt_bits speculos#487 (comment)

I am not sure if an issue needs to be raised for each of these points

version 2 sskr uses IANA registered CBOR tag #6.40309 updated from deprecared version 1 crypto-sskr tag #309
@aido aido force-pushed the 1.7.4-final branch 2 times, most recently from ad32da9 to 57acc91 Compare June 23, 2024 23:48
@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@aido
Copy link
Author

aido commented Jun 24, 2024

Hi @lpascal-ledger,

A chat with @vforgeoux-ledger on Discord informs me that this app is almost ready for a general release. If this PR is approved then Donjon can have a look and if all is OK we will be good to go.

@lpascal-ledger lpascal-ledger merged commit aa58ae5 into LedgerHQ:develop Jun 24, 2024
64 of 65 checks passed
@lpascal-ledger
Copy link

@aido merged, and a deployment on provider 4 (NanoZ) and 83 (Stax) is on its way.

Indeed Victor is the right person for stable release (everything going to provider 1) decision and more generally everything regarding application release process 👍

@aido aido deleted the 1.7.4-final branch June 24, 2024 15:57
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.

3 participants