Skip to content

Add Ledger signer provider #1946

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

Open
wants to merge 1 commit into
base: feature/ledger_signer
Choose a base branch
from
Open

Add Ledger signer provider #1946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions node-gui/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ license.workspace = true
version.workspace = true
edition.workspace = true
rust-version.workspace = true
authors = ["Samer Afach <samer.afach@mintlayer.org>", "Ben Marsh <benjamin.marsh@mintlayer.org>", "Enrico Rubboli <enrico.rubboli@mintlayer.org>"]
authors = [
"Samer Afach <samer.afach@mintlayer.org>",
"Ben Marsh <benjamin.marsh@mintlayer.org>",
"Enrico Rubboli <enrico.rubboli@mintlayer.org>",
]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

Expand All @@ -23,7 +27,7 @@ utils = { path = "../utils" }
wallet = { path = "../wallet" }
wallet-controller = { path = "../wallet/wallet-controller" }
wallet-types = { path = "../wallet/types" }
wallet-cli-commands = { path = "../wallet/wallet-cli-commands"}
wallet-cli-commands = { path = "../wallet/wallet-cli-commands" }
wallet-storage = { path = "../wallet/storage" }

anyhow.workspace = true
Expand All @@ -42,5 +46,16 @@ tokio.workspace = true
winres = "0.1"

[features]
trezor = ["wallet-controller/trezor", "wallet-types/trezor", "wallet-cli-commands/trezor", "node-gui-backend/trezor"]
default = ["trezor"]
trezor = [
"wallet-controller/trezor",
"wallet-types/trezor",
"wallet-cli-commands/trezor",
"node-gui-backend/trezor",
]
ledger = [
"wallet-controller/ledger",
"wallet-types/ledger",
"wallet-cli-commands/ledger",
"node-gui-backend/ledger",
]
default = ["trezor", "ledger"]
29 changes: 24 additions & 5 deletions node-gui/backend/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ license.workspace = true
version.workspace = true
edition.workspace = true
rust-version.workspace = true
authors = ["Samer Afach <samer.afach@mintlayer.org>", "Ben Marsh <benjamin.marsh@mintlayer.org>", "Enrico Rubboli <enrico.rubboli@mintlayer.org>"]
authors = [
"Samer Afach <samer.afach@mintlayer.org>",
"Ben Marsh <benjamin.marsh@mintlayer.org>",
"Enrico Rubboli <enrico.rubboli@mintlayer.org>",
]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

Expand All @@ -24,9 +28,9 @@ utils = { path = "../../utils" }
wallet = { path = "../../wallet" }
wallet-controller = { path = "../../wallet/wallet-controller" }
wallet-types = { path = "../../wallet/types" }
wallet-rpc-lib = { path = "../../wallet/wallet-rpc-lib"}
wallet-rpc-client = { path = "../../wallet/wallet-rpc-client"}
wallet-cli-commands = { path = "../../wallet/wallet-cli-commands"}
wallet-rpc-lib = { path = "../../wallet/wallet-rpc-lib" }
wallet-rpc-client = { path = "../../wallet/wallet-rpc-client" }
wallet-cli-commands = { path = "../../wallet/wallet-cli-commands" }

anyhow.workspace = true
chrono.workspace = true
Expand All @@ -45,4 +49,19 @@ test-utils = { path = "../../test-utils" }
rstest.workspace = true

[features]
trezor = ["wallet/trezor", "wallet-controller/trezor", "wallet-types/trezor", "wallet-rpc-lib/trezor", "wallet-rpc-client/trezor", "wallet-cli-commands/trezor"]
trezor = [
"wallet/trezor",
"wallet-controller/trezor",
"wallet-types/trezor",
"wallet-rpc-lib/trezor",
"wallet-rpc-client/trezor",
"wallet-cli-commands/trezor",
]
ledger = [
"wallet/ledger",
"wallet-controller/ledger",
"wallet-types/ledger",
"wallet-rpc-lib/ledger",
"wallet-rpc-client/ledger",
"wallet-cli-commands/ledger",
]
1 change: 1 addition & 0 deletions node-gui/src/main_window/main_widget/tabs/wallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ impl Tab for WalletTab {
Some(wallet_info) => match wallet_info.extra_info {
wallet_controller::types::WalletExtraInfo::SoftwareWallet => "Software wallet",
wallet_controller::types::WalletExtraInfo::TrezorWallet { .. } => "Trezor wallet",
wallet_controller::types::WalletExtraInfo::LedgerWallet { .. } => "Ledger wallet",
},
None => "No wallet",
};
Expand Down
15 changes: 15 additions & 0 deletions node-gui/src/main_window/main_widget/tabs/wallet/status_bar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ pub fn estimate_status_bar_height(wallet_info: &WalletExtraInfo) -> f32 {
// For some reason, the status bar gets a bit of additional height.
+ 4.
}
WalletExtraInfo::LedgerWallet { .. } => {
TEXT_SIZE + 2. * VERTICAL_PADDING
// Same as Trezor
+ 4.
Comment on lines +40 to +42
Copy link
Contributor

Choose a reason for hiding this comment

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

Plz don't duplicate the value.

}
}
}

Expand Down Expand Up @@ -67,6 +72,16 @@ pub fn view_status_bar(wallet_info: &WalletExtraInfo) -> Option<Element<'static,
.size(TEXT_SIZE),
]
}
#[cfg(feature = "ledger")]
WalletExtraInfo::LedgerWallet { firmware_version } => {
use iced::widget::{rich_text, span};
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this out of the function? (for the trezor case too).


row![rich_text([
span("Firmware version: ").font(bold_font),
span(firmware_version.clone())
])
.size(TEXT_SIZE),]
Comment on lines +79 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO we should be able to obtain the device name for the ledger too (which will be the model name, not the label set in Ledger Live, but it's still better than nothing).

As I've mentioned in the ledger app PR, it seems like the APDU 0xE0/0x01 is "get device info" (need to check if it works when an app is opened though). One of the returned values is targetId, from which the model name can be deduced.

Ledger Live sometimes calls this endpoint - https://manager.api.live.ledger.com/api/devices - to get the device model name (by comparing obtained the device's targetId with target_id inside device_versions of the returned json.)
(I'm not suggesting to use their endpoint, but we might just take its current values and hardcode them).

And it also has a hardcoded array of device infos and this function obtains them by targetId - https://github.com/LedgerHQ/ledger-live/blob/2dddf3a308ec6bd9fa436c7bc5bc02bcb33593e6/libs/ledgerjs/packages/devices/src/index.ts#L176-L182

}
};

let status_bar = Container::new(
Expand Down
4 changes: 2 additions & 2 deletions wallet/src/key_chain/master_key_chain/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crypto::vrf::ExtendedVRFPrivateKey;
use std::sync::Arc;
use wallet_storage::{
StoreTxRwUnlocked, WalletStorageReadLocked, WalletStorageReadUnlocked,
WalletStorageWriteUnlocked,
WalletStorageWriteLocked, WalletStorageWriteUnlocked,
};
use wallet_types::seed_phrase::{SerializableSeedPhrase, StoreSeedPhrase};

Expand Down Expand Up @@ -131,7 +131,7 @@ impl MasterKeyChain {

pub fn create_account_key_chain(
&self,
db_tx: &mut impl WalletStorageWriteUnlocked,
db_tx: &mut (impl WalletStorageWriteLocked + WalletStorageReadUnlocked),
account_index: U31,
lookahead_size: u32,
) -> KeyChainResult<AccountKeyChainImplSoftware> {
Expand Down
23 changes: 21 additions & 2 deletions wallet/src/signer/ledger_signer/ledger_messages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use crypto::key::{
};
use serialization::{Decode, DecodeAll, Encode};
use utils::ensure;
use wallet_types::hw_data::LedgerDataFullInfo;

use ledger_lib::Exchange;

Expand Down Expand Up @@ -57,6 +58,7 @@ impl From<LedgerAddrType> for u8 {
struct Ins {}

impl Ins {
const APP_VER: u8 = 0x03;
const APP_NAME: u8 = 0x04;
const PUB_KEY: u8 = 0x05;
const SIGN_TX: u8 = 0x06;
Expand Down Expand Up @@ -199,8 +201,13 @@ pub async fn get_app_name<L: Exchange>(ledger: &mut L) -> Result<Vec<u8>, ledger
ledger.exchange(&msg_buf, Duration::from_millis(100)).await
}

async fn get_app_version<L: Exchange>(ledger: &mut L) -> Result<Vec<u8>, ledger_lib::Error> {
let msg_buf = [CLA, Ins::APP_VER, 0, P2::DONE];
ledger.exchange(&msg_buf, Duration::from_millis(100)).await
}

#[allow(dead_code)]
pub async fn check_current_app<L: Exchange>(ledger: &mut L) -> SignerResult<()> {
pub async fn check_current_app<L: Exchange>(ledger: &mut L) -> SignerResult<LedgerDataFullInfo> {
let resp = get_app_name(ledger)
.await
.map_err(|err| LedgerError::DeviceError(err.to_string()))?;
Expand All @@ -212,7 +219,19 @@ pub async fn check_current_app<L: Exchange>(ledger: &mut L) -> SignerResult<()>
LedgerError::DifferentActiveApp(name)
);

Ok(())
let ver = get_app_version(ledger)
.await
.map_err(|err| LedgerError::DeviceError(err.to_string()))?;
let firmware_version = match ver.as_slice() {
[major, minor, patch] => common::primitives::semver::SemVer {
major: *major,
minor: *minor,
patch: *patch as u16,
},
_ => return Err(SignerError::LedgerError(LedgerError::InvalidResponse)),
};

Ok(LedgerDataFullInfo { firmware_version })
Comment on lines +225 to +234
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a mintlayer app version, not firmware version

}

pub async fn get_extended_public_key<L: Exchange>(
Expand Down
Loading