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

Stax Support #4

Merged
merged 106 commits into from
Jul 17, 2024
Merged

Stax Support #4

merged 106 commits into from
Jul 17, 2024

Conversation

chris124567
Copy link

@chris124567 chris124567 commented Feb 22, 2024 •
edited
Loading

Checklist

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

This PR:

  • Adds Stax support
  • Removes transaction streaming and instead loads transaction data up front in the case of signing a transaction
  • Rewrites things according to the new guidelines

These changes have been reviewed here previously: SiaFoundation#3

@chris124567
Revert "Bump to 0.4.5"
3def5f2 
This reverts commit 0e1a577.
@chris124567
Revert "fix: build for lns+"
cf6c1f5 
This reverts commit 2e57ec7.
@chris124567
basic changes
a5e11ba
@chris124567
basic stax app with about menu
3265551
@chris124567
format
ec63a4d
@chris124567
fix icon and get address/public key display working
a94d1f7
@chris124567
add sign hash
ed9f21a
@chris124567
add transaction signing
08cdaa0
@chris124567
add non UI tests
ac93724
@chris124567
add sign hash and pubkey tests
42ecc64
@chris124567
organize existing tests better and add transaction signing
46d9ce3
@chris124567
format tests
d7ebf92
@chris124567
add go.mod
064f47d
@chris124567
add CI workflow
43e5ca0
@chris124567
fixes
bcc795f
@chris124567
fix fallthrough warning
3a9a353
@chris124567
remove alpha channel from glyph
e22f23b
@chris124567
fix unused function warning
dc92270
@chris124567
fixes
9554868
@chris124567
add stax to sialedger and fix compiler warnings
173f34c
@chris124567
add nanosp support
ea2964f
@chris124567
add address verification confirmation screen
b9623ee
@chris124567
add developer to about page
6a9557d
@chris124567
add nanosp icon and fix nanox load parameters
770ffe3
@chris124567
fix icon in makefile
d92ca19
@chris124567
add status screens for all actions and fix address verification status
0ae9762
@chris124567
change transaction signing UI flow to Ledger recommendations
7ea99bc
@chris124567
fix tests
afe25be
@chris124567
use nbgl_useCaseRegularReview instead of nbgl_useCaseForwardOnlyRevie…
541a36f 
…w, where transaction hash confirmation page would get stuck if cancelled
@chris124567
store transaction elements data in fixed size array
5967098
@chris124567
Copy link
Author

chris124567 commented Jun 25, 2024 •
edited
Loading

I did the SIA-02 (potential out of bounds write) and SIA-03 (blind signing enabled by default) changes requested in the audit. I made there be an option to enable hash signing which is disabled by default. I also fixed the errors in the APDU documentation, and replaced the tutorial with a link to our newer one which is much more detailed.

n8maninger
n8maninger reviewed Jun 26, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
tdejoigny-ledger
tdejoigny-ledger reviewed Jul 11, 2024
View reviewed changes
src/getPublicKey.c Outdated Show resolved Hide resolved
src/signHash.c Outdated Show resolved Hide resolved
src/calcTxnHash_nbgl.c Outdated Show resolved Hide resolved
src/calcTxnHash_nbgl.c Outdated Show resolved Hide resolved
tdejoigny-ledger
tdejoigny-ledger reviewed Jul 15, 2024
View reviewed changes
Copy link

@tdejoigny-ledger tdejoigny-ledger left a comment

Choose a reason for hiding this comment

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

Last comments (and we should be able to deploy the app):

@chris124567
Copy link
Author

Fixed the linter issue and the icons look much better now!

n8maninger
n8maninger reviewed Jul 16, 2024
View reviewed changes
stax_app_sia_big.gif Outdated Show resolved Hide resolved
@tdejoigny-ledger tdejoigny-ledger merged commit 7b1b096 into LedgerHQ:develop Jul 17, 2024
28 of 31 checks passed
@tdejoigny-ledger
Copy link

@chris124567
I merged too quickly, I didn't realize that the toml file wasn't updated with Stax, so the CI didn't pass, and there are still things to fix.
Please refer to #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@n8maninger n8maninger n8maninger left review comments

@tdejoigny-ledger tdejoigny-ledger tdejoigny-ledger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@chris124567 @tdejoigny-ledger @n8maninger