-
Notifications
You must be signed in to change notification settings - Fork 17
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
release: ARK Ledger App v.2.0.0 #65
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Lock application to its own path * Prepare for Nano S 1.4 SDK * add autodeploy stuff * quickfix COMPLIANCE_141 * Move to SDK U2F framework * Add Nano X support
* chore: cleanup and refactor implementation The current implementation had lots of warnings and next-to-zero organization. This PR is primarily concerned with resolving those warnings and improve the overall implementation. Other ledger-app repos were used as a general guideline of what can be done. Specifically, this PR does the following: - splits all Platform-specific code into named directories. - bundles all relative data and methods into individual instances. - separates interfaces from their implementations. - debrands the filenames. - adds macros to define Token parameters. - adds live example bash scripts. - adds a short doc in examples * fix nanox compilation * fix: amount printing * chore: final legacy reimplementation
* feat: implement v2 Transaction Types * Update README.md * fixes and updates - adds more documentation to all unimplemented examples - updates all unimplemented examples. - fixes a few formatting issue. - adds Expiration deserialization and Display to Transfer/Type 0. - corrects IPFS DAG length checks. - improves Tx Type objects. - no need for `TransactionTypes. - No need for hex in Tx Type constants. - improves readability. - removes redundant '\0' on displayCtx title and vars. - fixes a bug in adjustDecimals to allow printing number values where no Token name or decimal is needed. * remove more redundant null terminators * chore: update HTLC-related code and docs - updates code, docs, and fixtures for HTLC-types. - adds expirationType and expirationValue to HTLC Lock. - adds display of HTLC Lock expiration. - adds another (5th) UX Step. - drops unused U8BE. - minor formatting improvements. * chore: final touchups - mostly formatting, naming, and unused header removal.
- supports v2, v1, and Legacy Transactions types. - truncates vf > 64 bytes. - updates fixtures. - updates documentation. - updates changelog. - bumps app version to v01.1.0. - also includes a few minor formatting updates.
- adjust char width.
Vote Asset len should be 35.
* feat: add message signing This PR adds Message Signing features. Max Message length is 255 Bytes. Large messages split across multiple display steps. Multiple display steps prepended/appended by ellipses. `INS` flag is `0x08`. ``` #define INS_GET_PUBLIC_KEY 0x02 #define INS_SIGN 0x04 #define INS_SIGN_MESSAGE 0x08 ``` This PR also does the following: - adds message signing operations. - moves `StreamStatus` to `./operations` dir. - adds message examples script. - adds message signing feature to example Py script. - updates message-related documentation. - updates the changelog. * chore: improve message handling - loop vs reuse. - adds more comments. - updates `message.sh` example script text. * chore: improve example py script & handleMessage comments * chore: improve message screen handling
* chore: update nanos-secure-sdk to v1.60 In the `provision/setup.sh` script: - remove `python-pil` dep. - remove `python-pip` dep. - add `pip3` install dep. - add `Pillow` install dep. - use `pip3` vs `pip` - checkout `nanos-secure-sdk` `1553` -> `160`. In Vagrantfile: - copy Icons to env. in Makefile: - version `1.1.0` -> `1.2.0`. - change `python` python3`. * chore: update examples to use python3 The nanos-secure-sdk-160 uses python3 vs python2 previously. - update the `example_helper.py` script to use python3. - update payloads doc to reflect python3 changes. - update example scripts to reflect python3 changes. * chore: resolve new warnings - add extern declarations of `setDisplaySteps`. - update `bagl_element_t` initialization to match nanos-sdk-160. - interpret `os_sched_exit` as a `ux_menu_callback_t`. * docs: update changelog * ci: update build workflow
Add UX headers per LedgerHQ, "...(ux.h) will be needed when building an app with the next nano S SDK (release). Basically you need to include this file as soon as you include os_io_seproxyhal.h somewhere. Our Boilerplate sample application has been modified accordingly...: `https://github.com/LedgerHQ/ledger-app-boilerplate/commit/4a73823576c81fe0d24a0f7c3426307539937b62`"
- remove all Ledger SDK headers from the Operations source. - add and use a cross-platform bytecpy macro for memcpy lib-wide. - implement uint packing macros for non-Ledger SDK instances. - rename dir assets -> types. - rename asset source files (e.g. type_0 -> transfer). - move transactions/assets/xxxx_display -> transactions/ux/xxxx_ux. - split assets/types.h -> assets/types(enum) & assets/assets. - drop StreamStatus in favor of bool. - add network variable to Transaction Data. - add global and local constants to avoid magic values. - refactor sha256 call to further extract Ledger SDK-code. - add a doubleHash256 method. - update "unimplemented" docs. - chore: update changelog.
This commit removes the `doubleHash256` method, and updates `base58.c` to reflect the change. Double hashing is not going to be needed very much, especially after current and future refactoring. It's used in deriving a wallet address from a publicKey or RecipientId bytes, and is only called from the `encodeBase58PublicKey` method. This change is non-breaking.
* refactor: unify ledger display flow Ledger Nano SDK v.160 allows the NanoS and NanoX to share the same UX flow pattern. This PR unifies the code which handles the UX display for NanoS and NanoX. Specifically, this PR proposes the following: - replace `src/ux` with a `src/display` directory. - move and rename `src/ux/display_context.h/c` -> `src/display/context.h/c`. - create a preprocessor header(`display/display.h`) to detect the nano platform. - unify `src/ux/nanos` and `src/ux/nanox` to `src/display/nano.h/c`. - remove legacy display code from `src/io.c`. - update headers in all relevant files. - update the changelog. * chore: resize display-context - resize `DisplayContext` to 472, 64-aligned. - rename `DisplayContext.var` -> `DisplayContext.text` * refactor(message_op): improve using new ux flow Splitting a message and using ellipses is no longer needed with the newest Ledger Nano SDK and unified ux flow.
The only way to get large fields to display properly would be to use patterns that invoke undefined behavior. This PR creates a separate variable for these large fields and 64-byte aligns the DisplayContext object (704-bytes). - add an extended-sized text variable to DisplayCtx. - change large fields to last display step (ipfs, tranfer, etc). - rename the UX flow steps to be more concise. - create secondary UX flow steps for extended text fields. - change `transaction_t` `vendorFieldLength` to `size_t`. - add bool (isExtended) to the `setDisplaySteps` method. - update examples and docs to demonstrate v2 Transfers with vendorFields up to 255 characters. - update Message example with extended field up to 255 characters. - update the changelog.
- add separate extended title to DisplayContext for extended texts (maintains 64-byte-alignment). - update relevant files to use `extended_title`. - add 6-screen extended flow var support in `display/nano.c`. - move vendorfield ux to separate file. - update `transfer_ux`, `htlc_lock_ux`, and legacy transfer to use new vendorfield file. - update `display_ux` to detect Htlc Lock vendorfield for display. - add Htlc Lock w/VendorField Json. - add Htlc Lock w/VendorField to payload examples.
* chore: cleanup and formatting - updated all license info. - team-made code: ARK MIT license headers. - Ledger-like code: ARK MIT + Apache 2.0 license headers. - add Ledger Apache 2.0 info the main LICENSE file. - remove whitespace/empty lines. - updated all “unimplemented” tx type examples. - split resources and installation into separate doc files. - update PAYLOADS doc. - update main README. - rename `./provision` -> `./scripts`. - move remaining scripts to `./scripts`. - move icons to `./icons` folder. * chore: update `unsupported` comments and docs - rename `unimplemented` docs -> `unsupported` - update main README with a link the the `unsupported` docs. - add more information about what types aren't supported and why in code comments and in `unsupported` docs.
The implementation has always appended a UTF8-encoded address to the `getPublicKey` calls' return value. This is not needed because: 1) ARK addresses are always passed as a 21-byte serialized address internally. 2) Wallets create Ledger addresses from the returned publicKey. 3) There was never a means to specify the network byte when creating an address. This PR proposes returning only the publicKey len and publicKey from the `getPublicKey` method. (e.g. (0x21(33) + publicKey)) Specifically, the following is suggested: - drop `cx_ecfp_public_key_t` from PublicKeyContext in favor of a raw byte buffer. - create a constant for the Uncompressed PublicKey len. - remove the `setPublicKeyContext` method. - simplify the `compressPublicKey` method by removing Ledger SDK code and operating on the raw byte buffer. - add more checks to the `compressPublicKey` method. - factor out `setPublicKeyContext` from the approval flow. - add calls to clear sensitive values from the approval flow. - factor out `setPublicKeyContext` from `handlePublicKeyContext`. - add minor formatting updates to the touched files. - update the default path in the `example_helper.py` file ("44'/111'/0'/0/0" -> "44'/1'/0'/0/0"). These changes are non-breaking and further help to decouple Ledger SDK-reliant code.
VirtualBox 6.1.2 is currently incompatible with the latest Vagrant (2.2.6). Homebrew also unexpectedly does not have a clean or quick way to force an older version of VirtualBox without modifying system files or the homebrew cask. While I expect this to eventually be resolved, I have no idea when that will happen. This PR recommends manual installation of version-specific packages to replace the homebrew and apt install steps for macOS and Linux. * VirtualBox 6.0.14 * VirtualBox Extensions Pack 6.0.14 * Vagrant 2.2.6
- add note about Vagrant on Windows requiring use of PowerShell with a recommendation and link to VSCode.
While there are no breaking changes from a users perspective, this release includes a massive rewrite of the project. Backwards compatibility is maintained in this release. Specific objectives accomplished within this release: - Support ARK Core v2 Transactions. - Support Message Signing. - Merge changes from `LedgerHQ/ledger-app-ark`. - Update the ARK Ledger App to use Ledger Nano FW v.1.6.0. - Improve overall project layout. - Add files to support development environment setup using Vagrant. - Update documentation. Significant changes can also be found in the CHANGELOG, as well as the following pull requests: - added message signing features [#40](#40) - implement v2 Transactions [#27](#27) - added VendorField display support [#29](#29) - added build options and documentation [#32](#32) - merged updates from LedgerHQ/ledger-app-ark [#23](#23) - cleaned up warnings and refactored implementation [#25](#25) - upgraded `nanos-secure-sdk` version `1553` -> `160` [#46](#46) - refactor transactions classes to decouple Ledger SDK code [#51](#51) - unify Ledger NanoS and NanoX ux display flow [#53](#53) - implement support for large text fields [#55](#55)
ghost
added
Complexity: Undetermined
Needs specialized, in-depth review.
Type: Release
The issue or pull request is related to an upcoming release.
labels
Feb 5, 2020
sleepdefic1t
requested review from
faustbrian,
fix,
alexbarnsley,
boldninja,
kristjank,
adrian69 and
ItsANameToo
and removed request for
fix
February 5, 2020 23:08
sleepdefic1t
added
Complexity: High
More than 256 lines changed.
and removed
Complexity: Undetermined
Needs specialized, in-depth review.
labels
Feb 6, 2020
faustbrian
approved these changes
Feb 7, 2020
sleepdefic1t
added a commit
that referenced
this pull request
Feb 8, 2020
## Summary While there are no breaking changes from a users perspective, this release includes a massive rewrite of the project. Specific objectives accomplished within this release: - Support ARK Core v2 Transactions. - Support Message Signing. - Merge changes from `LedgerHQ/ledger-app-ark`. - Update the ARK Ledger App to use Ledger Nano FW v.1.6.0. - Implement optimized Base58 from Ledger BTC App. - Improve overall project layout. - Add files to support development environment setup using Vagrant. - Update documentation. Significant changes can be found in the CHANGELOG, as well as the following pull requests: - added message signing features [#40](#40) - implement v2 Transactions [#27](#27) - added VendorField display support [#29](#29) - added build options and documentation [#32](#32) - merged updates from LedgerHQ/ledger-app-ark [#23](#23) - cleaned up warnings and refactored implementation [#25](#25) - upgraded `nanos-secure-sdk` version `1553` -> `160` [#46](#46) - refactor transactions classes to decouple Ledger SDK code [#51](#51) - unify Ledger NanoS and NanoX ux display flow [#53](#53) - implement support for large text fields [#55](#55) - use optimized Base58 implementation from Ledger BTC app [#66](#66) ## Supported in v.2.0.0 ### Transaction Versions: - v0 - v1 (_added_) - v2 (_added_) ### Signing Algorithms: - Ecdsa ### Operation Types: - Get PublicKey - Get App Configuration - Sign Message (_added_) - Sign Transaction ### Transaction Types: - v0 - Transfer - Vote - v1 (_added_) - Transfer - Vote - v2 (_added_) - Transfer - Second Signature Registration - Vote - IPFS - Htlc Lock - Htlc Claim - Htlc Refund ## Checklist - [x] Documentation _(if necessary)_ - [x] Ready to be merged ## Additional Discussion This patch release adds #66 to #65
ghost
mentioned this pull request
Feb 9, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Complexity: High
More than 256 lines changed.
Type: Release
The issue or pull request is related to an upcoming release.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
While there are no breaking changes from a users perspective, this release includes a massive rewrite of the project.
Specific objectives accomplished within this release:
LedgerHQ/ledger-app-ark
.Significant changes can be found in the CHANGELOG, as well as the following pull requests:
nanos-secure-sdk
version1553
->160
#46Supported in v.2.0.0
Transaction Versions:
Signing Algorithms:
Operation Types:
Transaction Types:
Checklist