Address @pipermerriam reviews

eth_account/signers/ledger.py

@@ -8,6 +8,10 @@
 
 from hexbytes import HexBytes
 
+from eth_account import (
+ Account,
+)
+
 from eth_account.datastructures import (
  AttributeDict,
 )
@@ -44,7 +48,7 @@ class LedgerAccount(BaseAccount):
 
  '''
 
- def __init__(self, account_id=0, address='', path_prefix=ETH_DERIVATION_PATH_PREFIX):
+ def __init__(self, account_id=0, address=None, path_prefix=ETH_DERIVATION_PATH_PREFIX):
  self.device = None
  self.path_prefix = path_prefix
 
@@ -141,24 +145,24 @@ def get_account_id(self, address, search_limit=20, search_account_id=0):
 
  for account_id in itertools.count(start=search_account_id):
  if account_id > search_limit:
- raise Exception(f'Address {address} not found' +
- f'(search_limit={search_limit}, ' +
- f'search_account_id={search_account_id})')
- if address == self.get_address(account_id):
+ raise ValueError(f'Address {address} not found' +
+  f'(search_limit={search_limit}, ' +
+  f'search_account_id={search_account_id})')
+ if eth_utils.is_same_address(address, self.get_address(account_id)):
  return account_id
 
- def list(self, limit=5, page=0):
+ def get_addresses(self, limit=5, page=0):
  '''
  List Ethereum HD wallet adrress of the ledger device
  '''
- return list(map(lambda account_id: self.get_address(account_id),
-  range(page * limit, (page + 1) * limit)))
+ return [self.get_address(account_id)
+ for account_id in range(page * limit, (page + 1) * limit)]
 
  def signHash(self, message_hash):
  '''
  Not available with a Ledger
  '''
- raise Exception('signHash() is not available within ledger devices')
+ raise NotImplementedError('signHash() is not available within ledger devices')
 
  def signTransaction(self, transaction_dict):
  '''
@@ -187,6 +191,11 @@ def signTransaction(self, transaction_dict):
  rlp_encoded = encode_transaction(unsigned_transaction, vrs=(v, r, s))
  transaction_hash = keccak(rlp_encoded)
 
+ # Sanity check on the signed transaction
+ expected_sender = self.address
+ recover_sender = Account.recoverTransaction(rlp_encoded)
+ assert eth_utils.is_same_address(expected_sender, recover_sender)
+
  return AttributeDict({
  'rawTransaction': HexBytes(rlp_encoded),
  'hash': HexBytes(transaction_hash),

setup.py

@@ -7,7 +7,7 @@
 
 extras_require={
  'ledger': [
- "ledgerblue==0.1.17",
+ "ledgerblue>=0.1.17,<0.2.0",
  ],
  'test': [
  "pytest==3.3.2",

tests/core/test_ledger.py

@@ -53,21 +53,21 @@ def test_address():
  assert address == ledger.get_address(account_id)
 
 
-def test_list():
- accounts_1 = ledger.list(1)
+def test_get_addresses():
+ accounts_1 = ledger.get_addresses(1)
  assert len(accounts_1) == 1
 
- accounts_5 = ledger.list(5)
+ accounts_5 = ledger.get_addresses(5)
  assert len(accounts_5) == 5
  assert accounts_1[0] == accounts_5[0]
 
- accounts_10 = ledger.list(10)
+ accounts_10 = ledger.get_addresses(10)
  assert len(accounts_10) == 10
  assert accounts_5 == accounts_10[:5]
 
- accounts_4p0 = ledger.list(limit=4, page=0)
- accounts_4p1 = ledger.list(limit=4, page=1)
- accounts_4p2 = ledger.list(limit=4, page=2)
+ accounts_4p0 = ledger.get_addresses(limit=4, page=0)
+ accounts_4p1 = ledger.get_addresses(limit=4, page=1)
+ accounts_4p2 = ledger.get_addresses(limit=4, page=2)
  assert len(accounts_4p1) == 4
  assert accounts_4p0 == accounts_10[:4]
  assert accounts_4p1 == accounts_10[4:8]