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

Add support for Ledger HW wallet #424

Open
wants to merge 129 commits into
base: master
Choose a base branch
from

Conversation

gabrielKerekes
Copy link

@gabrielKerekes gabrielKerekes commented Apr 19, 2023

This PR adds Ledger support into the Neblio wallet app. Here's a couple of important notes:

  • see doc/ledger-hw-wallet.md for a a bit more information about the Ledger integration
  • CI should build binaries for Windows, Linux and MacOS
  • GitHub actions runners were updated to Ubuntu 22.04 since the 18.04 runners have been deprecated (unavailable)
  • The build_scripts/CompileBoost-Linux.py seemed to be broken - the simlink to boost_build dir was not being created correctly - so we fixed the boost compile script.
  • CMake build fails because of the action runner being updated to Ubuntu 22.04. Ubuntu 22.04 uses openssl3 while the wallet requires openssl 1.1 and for some reason the compiled openssl is not used by the linker rather it uses the system’s openssl3 - we were not able to resolve this issue
  • CI docker containers have not been updated - we only extended the neblioteam/nebliod-build-ccache container to install hidapi dependencies and upload the image to our S3 bucket. The Dockerfile is here ./docker/build-cache.Dockerfile - it should be simple enough to modify the official containers in a similar manner and then the Dockerfile can be removed.
  • the mxe build used when building for Windows had to be rebuilt to include hidapi - we included the mxe build file in the repo.
  • test_win_x86_64-gui_wallet.py has not been modified since CI is not setup to run it, although the same changes done to test_win_x86-gui_wallet.py would probably work here as well.
  • we added a custom artifact upload step to the CI jobs - each job uploads the binary to GitHub (this may not be desired)
  • among the makefiles only the makefile.unix has been updated since the other ./wallet/makefiles* seem to be out of date anyways (and not used)
  • osx build had to be hacked around a little to work - there was an issue with linking libicu. The workaround is to include both libicu 67 and 71 binaries in the .dmg file.

We're not aware of any issues regarding the Ledger integration so please review the code and test thoroughly. We have of course tested the integration ourselves but we might have accidentally broken some unrelated features and we don't have the full context to test the whole app thoroughly.

The CI pipeline is still running for the mentioned platforms in our fork of the repo - https://github.com/vacuumlabs/neblio/actions/runs/4741911630 - the binaries will be uploaded once it finishes. (Here's a CI run from without the last commit which has the binaries already uploaded)

gabrielKerekes and others added 30 commits February 24, 2023 11:00
- fix change outputs
- fix transactions with multiple inputs (inputs could have different order than UTxOs)
- properly gather signature paths for all inputs (although there can only be one signature path)
- change/signature paths are not hardcoded anymore
- transactions should work properly
davidmisiak and others added 30 commits April 17, 2023 17:13
- add hidapi packages to README
- revert `macdeployqtplus` `-verbose 3` to `-verbose 1`
- remove unused `CPubKey.SetRaw()`
- remove TODO comment remained
- remove unused Bip32Path string constructor
- add empty line to `wallet.pro` to reduce unnecessary changes
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