Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

hardware_wallet/Ledger Sign messages + some refactoring #8868

Merged
merged 7 commits into from
Jun 13, 2018

Conversation

niklasad1
Copy link
Collaborator

@niklasad1 niklasad1 commented Jun 11, 2018

Attempt to close #6083

Also, I refactored the code a bit because it took me a while to understand why some our existing tests with big payload failed for Ledger failed but now all tests for Ledger works on my laptop

OS: Linux archlinux 4.16.13-2
Ledger Firmware version: 1.0.24

Also, I refactored update_devices because I found it annoying that we turn Ok() even if the no devices were detected!

@niklasad1 niklasad1 added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jun 11, 2018
@5chdn 5chdn added this to the 1.12 milestone Jun 12, 2018
Copy link
Collaborator

@debris debris left a comment

Choose a reason for hiding this comment

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

haven't run it on the device, but code looks good

use std::str::FromStr;
use std::sync::{atomic, atomic::AtomicBool, Arc, Weak};
use std::time::{Duration, Instant};
use std::{fmt, thread};
Copy link
Collaborator

Choose a reason for hiding this comment

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

we usually keep std imports before the others

@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 13, 2018
@5chdn 5chdn merged commit 6201532 into master Jun 13, 2018
@5chdn 5chdn deleted the na-ledger-sign-personal-messages branch June 13, 2018 09:02
tavakyan referenced this pull request in C4Coin/c4coin-parity Jun 14, 2018
* getting started

* getting started

* signing personal messages works and refactoring

* Refactor `Ledger`

* Make `Ledger Manager` only visible inside the hardwallet-crate
* Refactor `send_apdu` with separate functions for read and write

* Add support for signing messages through `ethcore`

* Trezor modify update_devices and some error msgs

* nits
dvdplm added a commit that referenced this pull request Jun 14, 2018
* master:
  Handle removed logs in filter changes and add geth compatibility field (#8796)
  fixed ipc leak, closes #8774 (#8876)
  scripts: remove md5 checksums (#8884)
  hardware_wallet/Ledger `Sign messages` + some refactoring (#8868)
ordian added a commit to ordian/parity that referenced this pull request Jun 20, 2018
…rp_sync_on_light_client

* 'master' of https://github.com/paritytech/parity: (29 commits)
  Block 0 is valid in queries (openethereum#8891)
  fixed osx permissions (openethereum#8901)
  Atomic create new files with permissions to owner in ethstore (openethereum#8896)
  Add ETC Cooperative-run load balanced parity node (openethereum#8892)
  Add support for --chain tobalaba (openethereum#8870)
  fix some warns on nightly (openethereum#8889)
  Add new ovh bootnodes and fix port for foundation bootnode 3.2 (openethereum#8886)
  SecretStore: service pack 1 (openethereum#8435)
  Handle removed logs in filter changes and add geth compatibility field (openethereum#8796)
  fixed ipc leak, closes openethereum#8774 (openethereum#8876)
  scripts: remove md5 checksums (openethereum#8884)
  hardware_wallet/Ledger `Sign messages` + some refactoring (openethereum#8868)
  Check whether we need resealing in miner and unwrap has_account in account_provider (openethereum#8853)
  docker: Fix alpine build (openethereum#8878)
  Remove mac os installers etc (openethereum#8875)
  README.md: update the list of dependencies (openethereum#8864)
  Fix concurrent access to signer queue (openethereum#8854)
  Tx permission contract improvement (openethereum#8400)
  Limit the number of transactions in pending set (openethereum#8777)
  Use sealing.enabled to emit eth_mining information (openethereum#8844)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hardware wallets don't support signing arbitrary messages: "Unknown binary data"
4 participants