fix: issue with 'v' byte when signing messages (#29)

.github/ISSUE_TEMPLATE/bug.md

@@ -16,7 +16,7 @@ $ ape plugins list
 ```
 
 * Python Version: x.x.x
-* OS: osx/linux/win
+* OS: macOS/linux/win
 
 ### What went wrong?
 

.github/workflows/commitlint.yaml

@@ -21,7 +21,7 @@ jobs:
         - name: Install Dependencies
           run: |
             python -m pip install --upgrade pip
-            pip install .[lint]
+            pip install "commitizen>=2.19,<2.20"
 
         - name: Check commit history
           run: cz check --rev-range $(git rev-list --all --reverse | head -1)..HEAD

.pre-commit-config.yaml

@@ -24,7 +24,6 @@ repos:
     rev: v0.971
     hooks:
     -   id: mypy
-        additional_dependencies: [types-PyYAML, types-requests]
 
 
 default_language_version:

CONTRIBUTING.md

@@ -11,7 +11,7 @@ cd ape-ledger
 python3 -m venv venv
 source venv/bin/activate
 
-# Install ape into the virtual environment
+# Install ape-ledger into the virtual environment
 python setup.py install
 
 # Install the developer dependencies (-e is interactive mode)

ape_ledger/_cli.py

@@ -120,6 +120,10 @@ def sign_message(cli_ctx, alias, message):
     eip191message = encode_defunct(text=message)
     account = accounts.load(alias)
     signature = account.sign_message(eip191message)
+
+    if not signature:
+        cli_ctx.abort("Failed to sign message.")
+
     signature_bytes = signature.encode_rsv()
 
     # Verify signature
@@ -128,11 +132,12 @@ def sign_message(cli_ctx, alias, message):
         cli_ctx.abort(f"Signer resolves incorrectly, got {signer}, expected {account.address}.")
 
     # Message signed successfully, return signature
-    click.echo(signature.encode_vrs().hex())
+    click.echo(signature_bytes.hex())
 
 
 @cli.command(short_help="Verify a message with your Trezor device")
 @click.argument("message")
+@click.argument("signature")
 def verify_message(message, signature):
     eip191message = encode_defunct(text=message)
 
@@ -143,5 +148,4 @@ def verify_message(message, signature):
         raise LedgerSigningError(message) from exc
 
     alias = accounts[signer_address].alias if signer_address in accounts else ""
-
     click.echo(f"Signer: {signer_address}  {alias}")

ape_ledger/accounts.py

@@ -5,7 +5,6 @@
 import click
 import rlp  # type: ignore
 from ape.api import AccountAPI, AccountContainerAPI, TransactionAPI
-from ape.logging import logger
 from ape.types import AddressType, MessageSignature, TransactionSignature
 from ape_ethereum.transactions import TransactionType
 from eth_account.messages import SignableMessage
@@ -104,24 +103,6 @@ def sign_message(self, msg: SignableMessage) -> Optional[MessageSignature]:
             )
 
         v, r, s = signed_msg
-
-        if self.network_manager.active_provider:
-            chain_id = self.provider.network.chain_id
-        else:
-            chain_id = 0
-            logger.warning(
-                f"The chain ID is not known. "
-                f"Using default value '{chain_id}' for determining parity bit."
-            )
-
-        # Compute parity
-        if (chain_id * 2 + 35) + 1 > 255:
-            ecc_parity = v - ((chain_id * 2 + 35) % 256)
-        else:
-            ecc_parity = (v + 1) % 2
-
-        v = int("%02X" % ecc_parity, 16)
-
         return MessageSignature(v, r, s)  # type: ignore
 
     def sign_transaction(self, txn: TransactionAPI) -> Optional[TransactionSignature]:

ape_ledger/client.py

@@ -402,8 +402,7 @@ def connect_to_ethereum_account(
     Create an account client using an active device connection.
     """
 
-    device = connect_to_device()
-    return LedgerEthereumAccountClient(device, address, hd_account_path)
+    return LedgerEthereumAccountClient(device_manager.device, address, hd_account_path)
 
 
 def connect_to_ethereum_app(hd_path: HDBasePath) -> LedgerEthereumAppClient:
@@ -412,27 +411,38 @@ def connect_to_ethereum_app(hd_path: HDBasePath) -> LedgerEthereumAppClient:
     using an active device connection.
     """
 
-    device = connect_to_device()
-    return LedgerEthereumAppClient(device, hd_path)
+    return LedgerEthereumAppClient(device_manager.device, hd_path)
 
 
-def connect_to_device() -> LedgerUsbDeviceClient:
-    """
-    Create a Ledger device client and connect to it.
-    """
+class DeviceManager:
+    _device: Optional[LedgerUsbDeviceClient] = None
+    _client_cls = LedgerUsbDeviceClient
+
+    @property
+    def device(self) -> LedgerUsbDeviceClient:
+        """
+        Create a Ledger device client and connect to it.
+        """
+
+        if self._device:
+            return self._device
+
+        hid_path = _get_hid_device_path()
+        if hid_path is None:
+            raise LedgerUsbError("No Ledger USB device found.")
+
+        device = _get_device(hid_path)
+        self._device = self._client_cls(device)
+        return self._device
 
-    hid_path = _get_hid_device_path()
-    if hid_path is None:
-        raise LedgerUsbError("No Ledger USB device found.")
 
-    device = _get_device(hid_path)
-    return LedgerUsbDeviceClient(device)
+device_manager = DeviceManager()
 
 
 __all__ = [
-    "connect_to_device",
     "connect_to_ethereum_account",
     "connect_to_ethereum_app",
+    "device_manager",
     "LedgerEthereumAccountClient",
     "LedgerEthereumAppClient",
     "LedgerUsbDeviceClient",

setup.py

@@ -55,15 +55,16 @@
     url="https://github.com/ApeWorX/ape-ledger",
     include_package_data=True,
     install_requires=[
-        "click>=8.0.0",
+        "click",  # Use same version as eth-ape
+        "eth-ape>=0.4.5,<0.5.0",
         "hidapi==0.10.1",
-        "eth-ape>=0.4.0,<0.5.0",
-        "eth-account>=0.5.6,<0.6.0",
-        "eth-typing>=2.2.2",
-        "eth-utils>=1.10.0",
-        "hexbytes==0.2.2",
         "importlib-metadata",
         "rlp>=2.0.1",
+        # EF Dependencies
+        "eth-account",  # Use same version as eth-ape
+        "eth-typing",  # Influenced by eth-ape
+        "eth-utils",  # Use same version as eth-ape
+        "hexbytes",  # Use same version as eth-ape
     ],
     entry_points={
         "ape_cli_subcommands": [
@@ -89,5 +90,6 @@
         "Programming Language :: Python :: 3.7",
         "Programming Language :: Python :: 3.8",
         "Programming Language :: Python :: 3.9",
+        "Programming Language :: Python :: 3.10",
     ],
 )

tests/test_accounts.py

@@ -123,14 +123,14 @@ def test_hdpath_returns_address_from_file(self, account):
 
     def test_sign_message_personal(self, mocker, account, account_connection, capsys):
         spy = mocker.spy(LedgerAccount, "_client")
-        spy.sign_personal_message.return_value = (0, b"r", b"s")
+        spy.sign_personal_message.return_value = (27, b"r", b"s")
 
         message = SignableMessage(
             version=b"E", header=b"thereum Signed Message:\n6", body=b"I\xe2\x99\xa5SF"
         )
         actual_v, actual_r, actual_s = account.sign_message(message)
 
-        assert actual_v == 1
+        assert actual_v == 27
         assert actual_r == b"r"
         assert actual_s == b"s"
         spy.sign_personal_message.assert_called_once_with(message.body)
@@ -141,12 +141,12 @@ def test_sign_message_personal(self, mocker, account, account_connection, capsys
 
     def test_sign_message_typed(self, mocker, account, account_connection, capsys):
         spy = mocker.spy(LedgerAccount, "_client")
-        spy.sign_typed_data.return_value = (0, b"r", b"s")
+        spy.sign_typed_data.return_value = (27, b"r", b"s")
 
         message = TEST_TYPED_MESSAGE.signable_message
         actual_v, actual_r, actual_s = account.sign_message(message)
 
-        assert actual_v == 1
+        assert actual_v == 27
         assert actual_r == b"r"
         assert actual_s == b"s"
         spy.sign_typed_data.assert_called_once_with(message.header, message.body)

tests/test_client.py

@@ -0,0 +1,6 @@
+import ape_ledger.client as client_module
+
+
+def test_device_connects_once(mock_device):
+    client_module.device_manager._device = mock_device
+    assert client_module.device_manager.device == mock_device