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

Trezor Safe 3 implementation #12687

Merged
merged 13 commits into from
Apr 2, 2024
Merged

Trezor Safe 3 implementation #12687

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b8955c1
fix typestring before parse
Whem Mar 18, 2024
09a1f8b
added ts3 to models
Whem Mar 18, 2024
856b070
added tz3 to hwiEnumerateEntry
Whem Mar 18, 2024
f7cb0de
added tz3 to hwikatas
Whem Mar 18, 2024
7016185
Update HwiProcessBridgeMock.cs
Whem Mar 18, 2024
7674842
Add ts3 mocktest
Whem Mar 18, 2024
281cd01
Merge branch 'master' into trezor_safe_3_implementation
Whem Mar 22, 2024
572f23d
fix format
Whem Mar 22, 2024
ebda714
fix format
Whem Mar 22, 2024
9c363a6
Update WasabiCompatibility.md
Whem Mar 22, 2024
d75c757
Update WalletDirTests.cs
Whem Mar 22, 2024
d1dd918
Fix nits
yahiheb Mar 23, 2024
5a5b26b
add ValidatePathString
Whem Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions WalletWasabi.Documentation/WasabiCompatibility.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ This document lists all the officially supported software and devices by Wasabi
- Ledger Nano S Plus
- Ledger Nano X
- Trezor Model T
- Trezor Safe 3

<sup><sup>1*</sup> The device by default asks for a "Pairing code", currently, there is no such function in Wasabi. Therefore, either disable the feature or unlock the device with BitBoxApp or hwi-qt before using it with Wasabi.</sup>

Expand Down
72 changes: 72 additions & 0 deletions WalletWasabi.Tests/AcceptanceTests/HwiKatas.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using NBitcoin;

Check notice on line 1 in WalletWasabi.Tests/AcceptanceTests/HwiKatas.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

ℹ Getting worse: Code Duplication 

introduced similar code in: TrezorS3KataAsync. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

Check warning on line 1 in WalletWasabi.Tests/AcceptanceTests/HwiKatas.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ Getting worse: Duplicated Assertion Blocks 

The number of functions with duplicated assertion blocks increases from 7 to 8 (introduced in TrezorS3KataAsync), threshold = 2. This test file has several blocks of duplicated assertion statements. Avoid adding more.
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -105,6 +105,78 @@
Assert.Equal(TransactionCheckResult.Success, checkResult);
}

[Fact]
public async Task TrezorS3KataAsync()
yahiheb marked this conversation as resolved.
Show resolved Hide resolved
{
// --- USER INTERACTIONS ---
//
// Connect and initialize your Trezor Safe 3 with the following seed phrase:
// more maid moon upgrade layer alter marine screen benefit way cover alcohol
// NEVER STORE REAL MONEY ON THIS WALLET. IT IS NOT SAFE.
// Run this test.
// displayaddress request: confirm 1 time
// displayaddress request: confirm 1 time
// signtx request: refuse 1 time
// signtx request: confirm 1 time
//
// --- USER INTERACTIONS ---

var network = Network.Main;
var client = new HwiClient(network);
using var cts = new CancellationTokenSource(ReasonableRequestTimeout);
var enumerate = await client.EnumerateAsync(cts.Token);
Assert.Single(enumerate);
HwiEnumerateEntry entry = enumerate.Single();
Assert.NotNull(entry.Path);
Assert.Equal(HardwareWalletModels.Trezor_Safe_3, entry.Model);
Assert.NotNull(entry.Fingerprint);

string devicePath = entry.Path;
HardwareWalletModels deviceType = entry.Model;
HDFingerprint fingerprint = entry.Fingerprint!.Value;

await Assert.ThrowsAsync<HwiException>(async () => await client.SetupAsync(deviceType, devicePath, false, cts.Token));

await Assert.ThrowsAsync<HwiException>(async () => await client.RestoreAsync(deviceType, devicePath, false, cts.Token));

yahiheb marked this conversation as resolved.
Show resolved Hide resolved

await Assert.ThrowsAsync<HwiException>(async () => await client.PromptPinAsync(deviceType, devicePath, cts.Token));
await Assert.ThrowsAsync<HwiException>(async () => await client.SendPinAsync(deviceType, devicePath, 1111, cts.Token));

KeyPath keyPath1 = new("m/84h/0h/0h/0/0");
KeyPath keyPath2 = new("m/84h/0h/0h/0/1");
ExtPubKey xpub1 = await client.GetXpubAsync(deviceType, devicePath, keyPath1, cts.Token);
ExtPubKey xpub2 = await client.GetXpubAsync(deviceType, devicePath, keyPath2, cts.Token);
Assert.NotNull(xpub1);
Assert.NotNull(xpub2);
Assert.NotEqual(xpub1, xpub2);

// USER: CONFIRM
BitcoinWitPubKeyAddress address1 = await client.DisplayAddressAsync(deviceType, devicePath, keyPath1, cts.Token);
// USER: CONFIRM
BitcoinWitPubKeyAddress address2 = await client.DisplayAddressAsync(fingerprint, keyPath2, cts.Token);
Assert.NotNull(address1);
Assert.NotNull(address2);
Assert.NotEqual(address1, address2);
var expectedAddress1 = xpub1.PubKey.GetAddress(ScriptPubKeyType.Segwit, network);
var expectedAddress2 = xpub2.PubKey.GetAddress(ScriptPubKeyType.Segwit, network);
Assert.Equal(expectedAddress1, address1);
Assert.Equal(expectedAddress2, address2);

// USER SHOULD REFUSE ACTION
var result = await Assert.ThrowsAsync<HwiException>(async () => await client.SignTxAsync(deviceType, devicePath, Psbt, cts.Token));
Assert.Equal(HwiErrorCode.ActionCanceled, result.ErrorCode);

// USER: Hold to confirm
PSBT signedPsbt = await client.SignTxAsync(deviceType, devicePath, Psbt, cts.Token);

Transaction signedTx = signedPsbt.GetOriginalTransaction();
Assert.Equal(Psbt.GetOriginalTransaction().GetHash(), signedTx.GetHash());

var checkResult = signedTx.Check();
Assert.Equal(TransactionCheckResult.Success, checkResult);
}

[Fact]
public async Task TrezorOneKataAsync()
{
Expand Down
20 changes: 14 additions & 6 deletions WalletWasabi.Tests/UnitTests/Hwi/HwiProcessBridgeMock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,196 +32,204 @@
{
HardwareWalletModels.Trezor_T => "trezor_t",
HardwareWalletModels.Trezor_1 => "trezor_1",
HardwareWalletModels.Trezor_Safe_3 => "trezor_safe_3",
HardwareWalletModels.Coldcard => "coldcard",
HardwareWalletModels.Ledger_Nano_S => "ledger_nano_s",
HardwareWalletModels.Ledger_Nano_X => "ledger_nano_x",
HardwareWalletModels.Jade => "jade",
HardwareWalletModels.BitBox02_BTCOnly => "bitbox02_btconly",
_ => throw new NotImplementedException("Mock missing.")
};

// This come from hwi.exe enumerate (path).
rawPath = Model switch
{
HardwareWalletModels.Trezor_T => "webusb: 001:4",
HardwareWalletModels.Trezor_Safe_3 => "webusb: 001:9",
HardwareWalletModels.Trezor_1 => "hid:\\\\\\\\?\\\\hid#vid_534c&pid_0001&mi_00#7&6f0b727&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}",
HardwareWalletModels.Coldcard => @"\\\\?\\hid#vid_d13e&pid_cc10&mi_00#7&1b239988&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}",
HardwareWalletModels.Ledger_Nano_S => "\\\\\\\\?\\\\hid#vid_2c97&pid_0001&mi_00#7&e45ae20&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}",
HardwareWalletModels.Ledger_Nano_X => "\\\\\\\\?\\\\hid#vid_2c97&pid_0001&mi_00#7&e45ae20&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}",
HardwareWalletModels.Jade => "COM3",
HardwareWalletModels.BitBox02_BTCOnly => "\\\\\\\\?\\\\hid#vid_03eb&pid_2403#6&229ae20&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}",
_ => throw new NotImplementedException("Mock missing.")
};

string path = HwiParser.NormalizeRawDevicePath(rawPath);
string devicePathAndTypeArgumentString = $"--device-path \"{path}\" --device-type \"{model}\"";

const string SuccessTrueResponse = "{\"success\": true}\r\n";

string? response = null;
int code = 0;

if (CompareArguments(arguments, "enumerate"))
{
response = Model switch
{
HardwareWalletModels.Trezor_T => $"[{{\"model\": \"{model}\", \"path\": \"{rawPath}\", \"needs_pin_sent\": false, \"needs_passphrase_sent\": false, \"error\": \"Not initialized\"}}]",
HardwareWalletModels.Trezor_Safe_3 => $"[{{\"model\": \"{model}\", \"label\": \"Test trezor\", \"type\":\"trezor\", \"path\": \"{rawPath}\", \"needs_pin_sent\": false, \"needs_passphrase_sent\": false, \"fingerprint\": \"e5dbc9cb\"}}]",
HardwareWalletModels.Trezor_1 => $"[{{\"model\": \"{model}\", \"path\": \"{rawPath}\", \"needs_pin_sent\": true, \"needs_passphrase_sent\": false, \"error\": \"Could not open client or get fingerprint information: Trezor is locked. Unlock by using 'promptpin' and then 'sendpin'.\", \"code\": -12}}]\r\n",
HardwareWalletModels.Coldcard => $"[{{\"model\": \"{model}\", \"path\": \"{rawPath}\", \"needs_passphrase\": false, \"fingerprint\": \"a3d0d797\"}}]\r\n",
HardwareWalletModels.Ledger_Nano_S => $"[{{\"model\": \"{model}\", \"path\": \"{rawPath}\", \"fingerprint\": \"4054d6f6\", \"needs_pin_sent\": false, \"needs_passphrase_sent\": false}}]\r\n",
HardwareWalletModels.Ledger_Nano_X => $"[{{\"model\": \"{model}\", \"path\": \"{rawPath}\", \"fingerprint\": \"4054d6f6\", \"needs_pin_sent\": false, \"needs_passphrase_sent\": false}}]\r\n",
HardwareWalletModels.Jade => $"[{{\"type\": \"{model}\", \"model\": \"{model}\", \"path\": \"{rawPath}\", \"needs_pin_sent\": false, \"needs_passphrase_sent\": false, \"fingerprint\": \"9bdca818\"}}]",
HardwareWalletModels.BitBox02_BTCOnly => $"[{{\"type\": \"{model}\", \"model\": \"{model}\", \"path\": \"{rawPath}\", \"needs_pin_sent\": false, \"needs_passphrase_sent\": false, \"fingerprint\": \"2ebf60e1\"}}]",
_ => throw new NotImplementedException($"Mock missing for {model}")
};
}
else if (CompareArguments(arguments, $"{devicePathAndTypeArgumentString} wipe"))
{
response = Model switch
{
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 => SuccessTrueResponse,
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 or HardwareWalletModels.Trezor_Safe_3 => SuccessTrueResponse,
HardwareWalletModels.Coldcard => "{\"error\": \"The Coldcard does not support wiping via software\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_S => "{\"error\": \"The Ledger Nano S does not support wiping via software\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_X => "{\"error\": \"The Ledger Nano X does not support wiping via software\", \"code\": -9}\r\n",
HardwareWalletModels.Jade => "{\"error\": \"Blockstream Jade does not support wiping via software\", \"code\": -9}",
HardwareWalletModels.BitBox02_BTCOnly => SuccessTrueResponse,
_ => throw new NotImplementedException("Mock missing.")
};
}
else if (CompareArguments(arguments, $"{devicePathAndTypeArgumentString} setup"))
{
response = Model switch
{
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 => "{\"error\": \"setup requires interactive mode\", \"code\": -9}",
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 or HardwareWalletModels.Trezor_Safe_3 => "{\"error\": \"setup requires interactive mode\", \"code\": -9}",
HardwareWalletModels.Coldcard => "{\"error\": \"The Coldcard does not support software setup\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_S => "{\"error\": \"The Ledger Nano S does not support software setup\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_X => "{\"error\": \"The Ledger Nano X does not support software setup\", \"code\": -9}\r\n",
HardwareWalletModels.Jade => "{\"error\": \"setup requires interactive mode\", \"code\": -9}",
HardwareWalletModels.BitBox02_BTCOnly => "{\"error\": \"setup requires interactive mode\", \"code\": -9}",
_ => throw new NotImplementedException("Mock missing.")
};
}
else if (CompareArguments(arguments, $"{devicePathAndTypeArgumentString} --interactive setup"))
{
response = Model switch
{
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 => SuccessTrueResponse,
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 or HardwareWalletModels.Trezor_Safe_3 => SuccessTrueResponse,
HardwareWalletModels.Coldcard => "{\"error\": \"The Coldcard does not support software setup\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_S => "{\"error\": \"The Ledger Nano S does not support software setup\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_X => "{\"error\": \"The Ledger Nano X does not support software setup\", \"code\": -9}\r\n",
HardwareWalletModels.Jade => "{\"error\": \"Blockstream Jade does not support software setup\", \"code\": -9}",
HardwareWalletModels.BitBox02_BTCOnly => SuccessTrueResponse,
_ => throw new NotImplementedException("Mock missing.")
};
}
else if (CompareArguments(arguments, $"{devicePathAndTypeArgumentString} --interactive restore"))
{
response = Model switch
{
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 => SuccessTrueResponse,
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 or HardwareWalletModels.Trezor_Safe_3 => SuccessTrueResponse,
HardwareWalletModels.Coldcard => "{\"error\": \"The Coldcard does not support restoring via software\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_S => "{\"error\": \"The Ledger Nano S does not support restoring via software\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_X => "{\"error\": \"The Ledger Nano X does not support restoring via software\", \"code\": -9}\r\n",
HardwareWalletModels.Jade => "{\"error\": \"Blockstream Jade does not support restoring via software\", \"code\": -9}",
HardwareWalletModels.BitBox02_BTCOnly => SuccessTrueResponse,
_ => throw new NotImplementedException("Mock missing.")
};
}
else if (CompareArguments(arguments, $"{devicePathAndTypeArgumentString} promptpin"))
{
response = Model switch
{
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 => "{\"error\": \"The PIN has already been sent to this device\", \"code\": -11}",
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 or HardwareWalletModels.Trezor_Safe_3 => "{\"error\": \"The PIN has already been sent to this device\", \"code\": -11}",
HardwareWalletModels.Coldcard => "{\"error\": \"The Coldcard does not need a PIN sent from the host\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_S => "{\"error\": \"The Ledger Nano S does not need a PIN sent from the host\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_X => "{\"error\": \"The Ledger Nano X does not need a PIN sent from the host\", \"code\": -9}\r\n",
HardwareWalletModels.Jade => "{\"error\": \"Blockstream Jade does not need a PIN sent from the host\", \"code\": -9}",
HardwareWalletModels.BitBox02_BTCOnly => "{\"error\": \"The BitBox02 does not need a PIN sent from the host\", \"code\": -9}",
_ => throw new NotImplementedException("Mock missing.")
};
}
else if (CompareArguments(arguments, $"{devicePathAndTypeArgumentString} sendpin", true))
{
response = Model switch
{
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 => "{\"error\": \"The PIN has already been sent to this device\", \"code\": -11}",
HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_1 or HardwareWalletModels.Trezor_Safe_3 => "{\"error\": \"The PIN has already been sent to this device\", \"code\": -11}",
HardwareWalletModels.Coldcard => "{\"error\": \"The Coldcard does not need a PIN sent from the host\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_S => "{\"error\": \"The Ledger Nano S does not need a PIN sent from the host\", \"code\": -9}\r\n",
HardwareWalletModels.Ledger_Nano_X => "{\"error\": \"The Ledger Nano X does not need a PIN sent from the host\", \"code\": -9}\r\n",
HardwareWalletModels.Jade => "{\"error\": \"Blockstream Jade does not need a PIN sent from the host\", \"code\": -9}",
HardwareWalletModels.BitBox02_BTCOnly => "{\"error\": \"The BitBox02 does not need a PIN sent from the host\", \"code\": -9}",
_ => throw new NotImplementedException("Mock missing.")
};
}
else if (CompareGetXbpubArguments(arguments, out string? xpub))
{
switch (Model)
{
case HardwareWalletModels.Trezor_T:
case HardwareWalletModels.Trezor_Safe_3:
case HardwareWalletModels.Trezor_1:
case HardwareWalletModels.Coldcard:
case HardwareWalletModels.Ledger_Nano_S:
case HardwareWalletModels.Ledger_Nano_X:
case HardwareWalletModels.Jade:
case HardwareWalletModels.BitBox02_BTCOnly:
response = $"{{\"xpub\": \"{xpub}\"}}\r\n";
break;
}
}
else if (CompareArguments(out bool t1, arguments, $"{devicePathAndTypeArgumentString} displayaddress --path m/84h/0h/0h --addr-type wit", false))
{
switch (Model)
{
case HardwareWalletModels.Trezor_T:
case HardwareWalletModels.Trezor_Safe_3:
case HardwareWalletModels.Trezor_1:
case HardwareWalletModels.Coldcard:
case HardwareWalletModels.Ledger_Nano_S:
case HardwareWalletModels.Ledger_Nano_X:
case HardwareWalletModels.Jade:
case HardwareWalletModels.BitBox02_BTCOnly:
response = t1
? "{\"address\": \"tb1q7zqqsmqx5ymhd7qn73lm96w5yqdkrmx7rtzlxy\"}\r\n"
: "{\"address\": \"bc1q7zqqsmqx5ymhd7qn73lm96w5yqdkrmx7fdevah\"}\r\n";
break;
}
}
else if (CompareArguments(out bool t2, arguments, $"{devicePathAndTypeArgumentString} displayaddress --path m/84h/0h/0h/1 --addr-type wit", false))
{
switch (Model)
{
case HardwareWalletModels.Trezor_T:
case HardwareWalletModels.Trezor_Safe_3:
case HardwareWalletModels.Trezor_1:
case HardwareWalletModels.Coldcard:
case HardwareWalletModels.Ledger_Nano_S:
case HardwareWalletModels.Ledger_Nano_X:
case HardwareWalletModels.Jade:
case HardwareWalletModels.BitBox02_BTCOnly:
response = t2
? "{\"address\": \"tb1qmaveee425a5xjkjcv7m6d4gth45jvtnjqhj3l6\"}\r\n"
: "{\"address\": \"bc1qmaveee425a5xjkjcv7m6d4gth45jvtnj23fzyf\"}\r\n";
break;
}
}
else if (CompareArguments(out bool _, arguments, $"{devicePathAndTypeArgumentString} displayaddress --path m/84h/1h/0h --addr-type wit", false))
{
switch (Model)
{
case HardwareWalletModels.Trezor_T:
case HardwareWalletModels.Trezor_Safe_3:
case HardwareWalletModels.Trezor_1:
case HardwareWalletModels.Coldcard:
case HardwareWalletModels.Ledger_Nano_S:
case HardwareWalletModels.Ledger_Nano_X:
case HardwareWalletModels.Jade:
case HardwareWalletModels.BitBox02_BTCOnly:
response = "{\"address\": \"tb1q7zqqsmqx5ymhd7qn73lm96w5yqdkrmx7rtzlxy\"}\r\n";
break;
}
}
else if (CompareArguments(out bool _, arguments, $"{devicePathAndTypeArgumentString} displayaddress --path m/84h/1h/0h/1 --addr-type wit", false))
{
switch (Model)
{
case HardwareWalletModels.Trezor_T:
case HardwareWalletModels.Trezor_Safe_3:

Check warning on line 232 in WalletWasabi.Tests/UnitTests/Hwi/HwiProcessBridgeMock.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ Getting worse: Complex Method 

SendCommandAsync increases in cyclomatic complexity from 127 to 135, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
case HardwareWalletModels.Trezor_1:
case HardwareWalletModels.Coldcard:
case HardwareWalletModels.Ledger_Nano_S:
Expand Down
83 changes: 83 additions & 0 deletions WalletWasabi.Tests/UnitTests/Hwi/MockedDeviceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using NBitcoin;

Check warning on line 1 in WalletWasabi.Tests/UnitTests/Hwi/MockedDeviceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ Getting worse: Code Duplication 

introduced similar code in: TrezorS3MockTestsAsync. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

Check notice on line 1 in WalletWasabi.Tests/UnitTests/Hwi/MockedDeviceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

✅ Getting better: Overall Function Size 

The median function size decrease from 75.0 to 73.0 LOC, threshold = 50.0. This file contains overly long functions, measured by their lines of code.

Check notice on line 1 in WalletWasabi.Tests/UnitTests/Hwi/MockedDeviceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

ℹ Getting worse: Overall Code Complexity 

The mean cyclomatic complexity increases from 4.63 to 4.67, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
using System.Collections.Generic;
using System.Linq;
using System.Threading;
Expand Down Expand Up @@ -54,61 +54,144 @@

// Trezor T doesn't support it.
var promptpin = await Assert.ThrowsAsync<HwiException>(async () => await client.PromptPinAsync(deviceType, devicePath, cts.Token));
Assert.Equal("The PIN has already been sent to this device", promptpin.Message);
Assert.Equal(HwiErrorCode.DeviceAlreadyUnlocked, promptpin.ErrorCode);

var sendpin = await Assert.ThrowsAsync<HwiException>(async () => await client.SendPinAsync(deviceType, devicePath, 1111, cts.Token));
Assert.Equal("The PIN has already been sent to this device", sendpin.Message);
Assert.Equal(HwiErrorCode.DeviceAlreadyUnlocked, sendpin.ErrorCode);

KeyPath keyPath1 = KeyManager.GetAccountKeyPath(network, ScriptPubKeyType.Segwit);
KeyPath keyPath2 = KeyManager.GetAccountKeyPath(network, ScriptPubKeyType.Segwit).Derive(1);
ExtPubKey xpub1 = await client.GetXpubAsync(deviceType, devicePath, keyPath1, cts.Token);
ExtPubKey xpub2 = await client.GetXpubAsync(deviceType, devicePath, keyPath2, cts.Token);
ExtPubKey expectedXpub1;
ExtPubKey expectedXpub2;
if (network == Network.TestNet)
{
expectedXpub1 = NBitcoinHelpers.BetterParseExtPubKey("xpub6CaGC5LjEw1YWw8br7AURnB6ioJY2bEVApXh8NMsPQ9mdDbzN51iwVrnmGSof3MfjjRrntnE8mbYeTW5ywgvCXdjqF8meQEwnhPDQV2TW7c");
expectedXpub2 = NBitcoinHelpers.BetterParseExtPubKey("xpub6E7pup6CRRS5jM1r3HVYQhHwQHpddJALjRDbsVDtsnQJozHrfE8Pua2X5JhtkWCxdcmGhPXWxV7DoJtSgZSUvUy6cvDchVQt2RGEd4mD4FA");
}
else
{
expectedXpub1 = NBitcoinHelpers.BetterParseExtPubKey("xpub6DHjDx4gzLV37gJWMxYJAqyKRGN46MT61RHVizdU62cbVUYu9L95cXKzX62yJ2hPbN11EeprS8sSn8kj47skQBrmycCMzFEYBQSntVKFQ5M");
expectedXpub2 = NBitcoinHelpers.BetterParseExtPubKey("xpub6FJS1ne3STcKdQ9JLXNzZXidmCNZ9dxLiy7WVvsRkcmxjJsrDKJKEAXq4MGyEBM3vHEw2buqXezfNK5SNBrkwK7Fxjz1TW6xzRr2pUyMWFu");
}
Assert.Equal(expectedXpub1, xpub1);
Assert.Equal(expectedXpub2, xpub2);

BitcoinWitPubKeyAddress address1 = await client.DisplayAddressAsync(deviceType, devicePath, keyPath1, cts.Token);
BitcoinWitPubKeyAddress address2 = await client.DisplayAddressAsync(deviceType, devicePath, keyPath2, cts.Token);

BitcoinAddress expectedAddress1;
BitcoinAddress expectedAddress2;
if (network == Network.Main)
{
expectedAddress1 = BitcoinAddress.Create("bc1q7zqqsmqx5ymhd7qn73lm96w5yqdkrmx7fdevah", Network.Main);
expectedAddress2 = BitcoinAddress.Create("bc1qmaveee425a5xjkjcv7m6d4gth45jvtnj23fzyf", Network.Main);
}
else if (network == Network.TestNet)
{
expectedAddress1 = BitcoinAddress.Create("tb1q7zqqsmqx5ymhd7qn73lm96w5yqdkrmx7rtzlxy", Network.TestNet);
expectedAddress2 = BitcoinAddress.Create("tb1qmaveee425a5xjkjcv7m6d4gth45jvtnjqhj3l6", Network.TestNet);
}
else if (network == Network.RegTest)
{
expectedAddress1 = BitcoinAddress.Create("bcrt1q7zqqsmqx5ymhd7qn73lm96w5yqdkrmx7pzmj3d", Network.RegTest);
expectedAddress2 = BitcoinAddress.Create("bcrt1qmaveee425a5xjkjcv7m6d4gth45jvtnjz7tugn", Network.RegTest);
}
else
{
throw new NotSupportedNetworkException(network);
}

Assert.Equal(expectedAddress1, address1);
Assert.Equal(expectedAddress2, address2);
}

Check notice on line 110 in WalletWasabi.Tests/UnitTests/Hwi/MockedDeviceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

ℹ Getting worse: Large Assertion Blocks 

The number of large assertion blocks increases from 7 to 8, threshold = 4. This test file has several blocks of large, consecutive assert statements. Avoid adding more.

Check warning on line 110 in WalletWasabi.Tests/UnitTests/Hwi/MockedDeviceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ Getting worse: Duplicated Assertion Blocks 

The number of functions with duplicated assertion blocks increases from 5 to 6 (introduced in TrezorS3MockTestsAsync), threshold = 2. This test file has several blocks of duplicated assertion statements. Avoid adding more.

[Theory]
[MemberData(nameof(GetDifferentNetworkValues))]
public async Task TrezorS3MockTestsAsync(Network network)
yahiheb marked this conversation as resolved.
Show resolved Hide resolved
{
var client = new HwiClient(network, new HwiProcessBridgeMock(HardwareWalletModels.Trezor_Safe_3));

using var cts = new CancellationTokenSource(ReasonableRequestTimeout);
IEnumerable<HwiEnumerateEntry> enumerate = await client.EnumerateAsync(cts.Token);
Assert.Single(enumerate);
HwiEnumerateEntry entry = enumerate.Single();
Assert.Equal(HardwareWalletModels.Trezor_Safe_3, entry.Model);
Assert.Equal("webusb: 001:9", entry.Path);
Assert.False(entry.NeedsPassphraseSent);
Assert.False(entry.NeedsPinSent);
Assert.Null(entry.Error);
Assert.Null(entry.Error);
Assert.True(entry.IsInitialized());
Assert.NotNull(entry.Fingerprint);

var deviceType = entry.Model;
var devicePath = entry.Path;

await client.WipeAsync(deviceType, devicePath, cts.Token);
await client.SetupAsync(deviceType, devicePath, false, cts.Token);
await client.RestoreAsync(deviceType, devicePath, false, cts.Token);

// Trezor T doesn't support it.
yahiheb marked this conversation as resolved.
Show resolved Hide resolved
var promptpin = await Assert.ThrowsAsync<HwiException>(async () => await client.PromptPinAsync(deviceType, devicePath, cts.Token));

Check warning on line 139 in WalletWasabi.Tests/UnitTests/Hwi/MockedDeviceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ New issue: Large Method 

TrezorS3MockTestsAsync has 71 lines, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.
Assert.Equal("The PIN has already been sent to this device", promptpin.Message);
Assert.Equal(HwiErrorCode.DeviceAlreadyUnlocked, promptpin.ErrorCode);

var sendpin = await Assert.ThrowsAsync<HwiException>(async () => await client.SendPinAsync(deviceType, devicePath, 1111, cts.Token));
Assert.Equal("The PIN has already been sent to this device", sendpin.Message);
Assert.Equal(HwiErrorCode.DeviceAlreadyUnlocked, sendpin.ErrorCode);

KeyPath keyPath1 = KeyManager.GetAccountKeyPath(network, ScriptPubKeyType.Segwit);
KeyPath keyPath2 = KeyManager.GetAccountKeyPath(network, ScriptPubKeyType.Segwit).Derive(1);
ExtPubKey xpub1 = await client.GetXpubAsync(deviceType, devicePath, keyPath1, cts.Token);
ExtPubKey xpub2 = await client.GetXpubAsync(deviceType, devicePath, keyPath2, cts.Token);
ExtPubKey expectedXpub1;
ExtPubKey expectedXpub2;
if (network == Network.TestNet)
{
expectedXpub1 = NBitcoinHelpers.BetterParseExtPubKey("xpub6CaGC5LjEw1YWw8br7AURnB6ioJY2bEVApXh8NMsPQ9mdDbzN51iwVrnmGSof3MfjjRrntnE8mbYeTW5ywgvCXdjqF8meQEwnhPDQV2TW7c");
expectedXpub2 = NBitcoinHelpers.BetterParseExtPubKey("xpub6E7pup6CRRS5jM1r3HVYQhHwQHpddJALjRDbsVDtsnQJozHrfE8Pua2X5JhtkWCxdcmGhPXWxV7DoJtSgZSUvUy6cvDchVQt2RGEd4mD4FA");
}
else
{
expectedXpub1 = NBitcoinHelpers.BetterParseExtPubKey("xpub6DHjDx4gzLV37gJWMxYJAqyKRGN46MT61RHVizdU62cbVUYu9L95cXKzX62yJ2hPbN11EeprS8sSn8kj47skQBrmycCMzFEYBQSntVKFQ5M");
expectedXpub2 = NBitcoinHelpers.BetterParseExtPubKey("xpub6FJS1ne3STcKdQ9JLXNzZXidmCNZ9dxLiy7WVvsRkcmxjJsrDKJKEAXq4MGyEBM3vHEw2buqXezfNK5SNBrkwK7Fxjz1TW6xzRr2pUyMWFu");
}
Assert.Equal(expectedXpub1, xpub1);
Assert.Equal(expectedXpub2, xpub2);

BitcoinWitPubKeyAddress address1 = await client.DisplayAddressAsync(deviceType, devicePath, keyPath1, cts.Token);
BitcoinWitPubKeyAddress address2 = await client.DisplayAddressAsync(deviceType, devicePath, keyPath2, cts.Token);

BitcoinAddress expectedAddress1;
BitcoinAddress expectedAddress2;
if (network == Network.Main)
{
expectedAddress1 = BitcoinAddress.Create("bc1q7zqqsmqx5ymhd7qn73lm96w5yqdkrmx7fdevah", Network.Main);
expectedAddress2 = BitcoinAddress.Create("bc1qmaveee425a5xjkjcv7m6d4gth45jvtnj23fzyf", Network.Main);
}
else if (network == Network.TestNet)
{
expectedAddress1 = BitcoinAddress.Create("tb1q7zqqsmqx5ymhd7qn73lm96w5yqdkrmx7rtzlxy", Network.TestNet);
expectedAddress2 = BitcoinAddress.Create("tb1qmaveee425a5xjkjcv7m6d4gth45jvtnjqhj3l6", Network.TestNet);
}
else if (network == Network.RegTest)
{
expectedAddress1 = BitcoinAddress.Create("bcrt1q7zqqsmqx5ymhd7qn73lm96w5yqdkrmx7pzmj3d", Network.RegTest);
expectedAddress2 = BitcoinAddress.Create("bcrt1qmaveee425a5xjkjcv7m6d4gth45jvtnjz7tugn", Network.RegTest);
}
else
{
throw new NotSupportedNetworkException(network);
}

Assert.Equal(expectedAddress1, address1);
Assert.Equal(expectedAddress2, address2);
}

[Theory]
[MemberData(nameof(GetDifferentNetworkValues))]
public async Task TrezorOneMockTestsAsync(Network network)
Expand Down
1 change: 1 addition & 0 deletions WalletWasabi.Tests/UnitTests/WalletDirTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ public void GetFriendlyNameTest()
Assert.Equal("Trezor One Simulator", HardwareWalletModels.Trezor_1_Simulator.FriendlyName());
Assert.Equal("Trezor T", HardwareWalletModels.Trezor_T.FriendlyName());
Assert.Equal("Trezor T Simulator", HardwareWalletModels.Trezor_T_Simulator.FriendlyName());
Assert.Equal("Trezor Safe 3", HardwareWalletModels.Trezor_Safe_3.FriendlyName());
Assert.Equal("BitBox", HardwareWalletModels.BitBox02_BTCOnly.FriendlyName());
Assert.Equal("BitBox", HardwareWalletModels.BitBox02_Multi.FriendlyName());
Assert.Equal("Jade", HardwareWalletModels.Jade.FriendlyName());
Expand Down
3 changes: 3 additions & 0 deletions WalletWasabi/Hwi/Models/HardwareWalletModels.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ public enum HardwareWalletModels
[FriendlyName("Trezor T Simulator")]
Trezor_T_Simulator,

[FriendlyName("Trezor Safe 3")]
Trezor_Safe_3,

[FriendlyName("BitBox")]
BitBox02_BTCOnly,

Expand Down
6 changes: 4 additions & 2 deletions WalletWasabi/Hwi/Models/HwiEnumerateEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
{
HardwareWalletModels.Coldcard or HardwareWalletModels.Coldcard_Simulator => WalletType.Coldcard,
HardwareWalletModels.Ledger_Nano_S or HardwareWalletModels.Ledger_Nano_X or HardwareWalletModels.Ledger_Nano_S_Plus => WalletType.Ledger,
HardwareWalletModels.Trezor_1 or HardwareWalletModels.Trezor_1_Simulator or HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_T_Simulator => WalletType.Trezor,
HardwareWalletModels.Trezor_1 or HardwareWalletModels.Trezor_1_Simulator or HardwareWalletModels.Trezor_T or HardwareWalletModels.Trezor_T_Simulator or HardwareWalletModels.Trezor_Safe_3 => WalletType.Trezor,
HardwareWalletModels.Jade => WalletType.Jade,
HardwareWalletModels.BitBox02_BTCOnly => WalletType.BitBox,
_ => WalletType.Hardware
Expand All @@ -62,10 +62,12 @@
HardwareWalletModels.Coldcard => true,
HardwareWalletModels.Ledger_Nano_S or HardwareWalletModels.Ledger_Nano_X or HardwareWalletModels.Ledger_Nano_S_Plus => false,
HardwareWalletModels.Trezor_1 => false,
HardwareWalletModels.Trezor_T => true,
HardwareWalletModels.Trezor_T => false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this value wrong and it got corrected?

Do we still need IsOfflinePsbtWorkflowCompatible?

Copy link
Collaborator Author

@Whem Whem Mar 25, 2024 •
edited

Choose a reason for hiding this comment

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

The first time when I read the Trezor T specs, I thought it could support native PSBT workflow

Yes, I need it in upcoming features

HardwareWalletModels.Trezor_Safe_3 => false,
HardwareWalletModels.Trezor_1_Simulator or HardwareWalletModels.Trezor_T_Simulator or HardwareWalletModels.Coldcard_Simulator => false,
HardwareWalletModels.Jade => false,
HardwareWalletModels.BitBox02_BTCOnly => false,

Check notice on line 70 in WalletWasabi/Hwi/Models/HwiEnumerateEntry.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

ℹ Getting worse: Complex Method 

IsOfflinePsbtWorkflowCompatible increases in cyclomatic complexity from 10 to 11, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
yahiheb marked this conversation as resolved.
Show resolved Hide resolved
_ => false
};
}
Expand Down