Merge pull request #3486 from derekpierre/signer-tx

Fix inconsistent `sign_transaction` API return values

newsfragments/3486.bugfix.rst

@@ -0,0 +1 @@
+Fix inconsistent API for ``sign_transaction`` return values; all now return bytes.

nucypher/blockchain/eth/clients.py

@@ -6,7 +6,6 @@ from typing import Union
 from constant_sorrow.constants import UNKNOWN_DEVELOPMENT_CHAIN_ID
 from cytoolz.dicttoolz import dissoc
 from eth_account import Account
-from eth_account.datastructures import SignedTransaction
 from eth_account.messages import encode_defunct
 from eth_typing.evm import BlockNumber, ChecksumAddress
 from eth_utils import to_canonical_address, to_checksum_address
@@ -558,12 +557,14 @@ class EthereumTesterClient(EthereumClient):
             raise self.UnknownAccount(account)
         return signing_key
 
-    def sign_transaction(self, transaction_dict: dict) -> SignedTransaction:
+    def sign_transaction(self, transaction_dict: dict) -> bytes:
         # Sign using a local private key
         address = to_canonical_address(transaction_dict['from'])
         signing_key = self.__get_signing_key(account=address)
-        signed_transaction = self.w3.eth.account.sign_transaction(transaction_dict, private_key=signing_key)
-        return signed_transaction
+        raw_transaction = self.w3.eth.account.sign_transaction(
+            transaction_dict, private_key=signing_key
+        ).rawTransaction
+        return raw_transaction
 
     def sign_message(self, account: str, message: bytes) -> str:
         """Sign, EIP-191 (Geth) Style"""

nucypher/blockchain/eth/interfaces.py

@@ -617,7 +617,7 @@ class BlockchainInterface:
                 f"({max_cost} @ {max_price_gwei} gwei)",
                 color="yellow",
             )
-        signed_transaction = transacting_power.sign_transaction(transaction_dict)
+        raw_transaction = transacting_power.sign_transaction(transaction_dict)
 
         #
         # Broadcast
@@ -627,9 +627,7 @@ class BlockchainInterface:
             color="yellow",
         )
         try:
-            txhash = self.client.send_raw_transaction(
-                signed_transaction.rawTransaction
-            )  # <--- BROADCAST
+            txhash = self.client.send_raw_transaction(raw_transaction)  # <--- BROADCAST
             emitter.message(f"TXHASH {txhash.hex()}", color="yellow")
         except ValueError:
             raise  # TODO: Unify with Transaction failed handling -- Entry point for _handle_failed_transaction

nucypher/blockchain/eth/signers/base.py

@@ -2,7 +2,6 @@ from abc import ABC, abstractmethod
 from typing import List
 from urllib.parse import urlparse
 
-from eth_account.datastructures import SignedTransaction
 from eth_typing.evm import ChecksumAddress
 from hexbytes.main import HexBytes
 
@@ -78,7 +77,7 @@ class Signer(ABC):
         return NotImplemented
 
     @abstractmethod
-    def sign_transaction(self, transaction_dict: dict) -> SignedTransaction:
+    def sign_transaction(self, transaction_dict: dict) -> bytes:
         return NotImplemented
 
     @abstractmethod

nucypher/blockchain/eth/signers/software.py

@@ -6,7 +6,6 @@ from urllib.parse import urlparse
 
 from cytoolz.dicttoolz import dissoc
 from eth_account.account import Account
-from eth_account.datastructures import SignedTransaction
 from eth_account.messages import encode_defunct
 from eth_account.signers.local import LocalAccount
 from eth_utils.address import is_address, to_canonical_address, to_checksum_address
@@ -99,11 +98,11 @@ class Web3Signer(Signer):
         signature = self.__client.sign_message(account=account, message=message)
         return HexBytes(signature)
 
-    def sign_transaction(self, transaction_dict: dict) -> SignedTransaction:
-        signed_transaction = self.__client.sign_transaction(
+    def sign_transaction(self, transaction_dict: dict) -> bytes:
+        raw_transaction = self.__client.sign_transaction(
             transaction_dict=transaction_dict
         )
-        return signed_transaction
+        return raw_transaction
 
 
 class KeystoreSigner(Signer):
@@ -275,7 +274,7 @@ class KeystoreSigner(Signer):
         return account not in self.__signers
 
     @validate_checksum_address
-    def sign_transaction(self, transaction_dict: dict) -> SignedTransaction:
+    def sign_transaction(self, transaction_dict: dict) -> bytes:
         """
         Produce a raw signed ethereum transaction signed by the account specified
         in the 'from' field of the transaction dictionary.
@@ -355,15 +354,15 @@ class InMemorySigner(Signer):
                 raise self.AccountLocked(account=account)
 
     @validate_checksum_address
-    def sign_transaction(self, transaction_dict: dict) -> SignedTransaction:
+    def sign_transaction(self, transaction_dict: dict) -> bytes:
         sender = transaction_dict["from"]
         signer = self.__get_signer(account=sender)
         if not transaction_dict["to"]:
             transaction_dict = dissoc(transaction_dict, "to")
-        signed_transaction = signer.sign_transaction(
+        raw_transaction = signer.sign_transaction(
             transaction_dict=transaction_dict
         ).rawTransaction
-        return signed_transaction
+        return raw_transaction
 
     @validate_checksum_address
     def sign_message(self, account: str, message: bytes, **kwargs) -> HexBytes:

nucypher/crypto/powers.py

@@ -2,7 +2,6 @@ import inspect
 from typing import List, Optional, Tuple, Union
 
 from eth_account._utils.signing import to_standard_signature_bytes
-from eth_account.datastructures import SignedTransaction
 from eth_typing.evm import ChecksumAddress
 from nucypher_core import (
     EncryptedThresholdDecryptionRequest,
@@ -201,7 +200,7 @@ class TransactingPower(CryptoPowerUp):
         # from the recovery byte, bringing it to the standard choice of {0, 1}.
         return to_standard_signature_bytes(signature)
 
-    def sign_transaction(self, transaction_dict: dict) -> SignedTransaction:
+    def sign_transaction(self, transaction_dict: dict) -> bytes:
         """Signs the transaction with the private key of the TransactingPower."""
         return self._signer.sign_transaction(transaction_dict=transaction_dict)
 

tests/acceptance/characters/test_transacting_power.py

@@ -59,12 +59,10 @@ def test_character_transacting_power_signing(testerchain, test_registry):
         "data": b"",
     }
 
-    signed_transaction = power.sign_transaction(transaction_dict=transaction_dict)
+    raw_transaction = power.sign_transaction(transaction_dict=transaction_dict)
 
     # Demonstrate that the transaction is valid RLP encoded.
-    restored_transaction = Transaction.from_bytes(
-        serialized_bytes=signed_transaction.rawTransaction
-    )
+    restored_transaction = Transaction.from_bytes(serialized_bytes=raw_transaction)
     restored_dict = restored_transaction.as_dict()
     assert to_checksum_address(restored_dict["to"]) == transaction_dict["to"]
 
@@ -125,9 +123,7 @@ def test_transacting_power_sign_transaction(testerchain):
     # Demonstrate that the transaction is valid RLP encoded.
     from eth_account._utils.legacy_transactions import Transaction
 
-    restored_transaction = Transaction.from_bytes(
-        serialized_bytes=signed_transaction.rawTransaction
-    )
+    restored_transaction = Transaction.from_bytes(serialized_bytes=signed_transaction)
     restored_dict = restored_transaction.as_dict()
     assert to_checksum_address(restored_dict["to"]) == transaction_dict["to"]
 
@@ -163,9 +159,7 @@ def test_transacting_power_sign_agent_transaction(testerchain, coordinator_agent
         signer=Web3Signer(testerchain.client),
         account=testerchain.etherbase_account,
     )
-    signed_raw_transaction = transacting_power.sign_transaction(
-        unsigned_transaction
-    ).rawTransaction
+    signed_raw_transaction = transacting_power.sign_transaction(unsigned_transaction)
 
     # Demonstrate that the transaction is valid RLP encoded.
     restored_transaction = Transaction.from_bytes(

tests/unit/conftest.py

@@ -119,3 +119,17 @@ def random_transcript(get_random_checksum_address):
     )
 
     return transcript
+
+
+@pytest.fixture(scope="module")
+def tx_dict():
+    _tx_dict = {
+        "chainId": 1,
+        "nonce": 2,
+        "gasPrice": 2000000000000,
+        "gas": 314159,
+        "to": "0xd3CdA913deB6f67967B99D67aCDFa1712C293601",
+        "value": 12345,
+        "data": b"in that metric, kman is above reproach",  # thank you friends
+    }
+    yield _tx_dict

tests/unit/test_keystore_signer.py

@@ -0,0 +1,77 @@
+import json
+
+import pytest
+from cytoolz import assoc
+from eth_account._utils.legacy_transactions import Transaction
+from eth_account.messages import encode_defunct
+from eth_utils import to_checksum_address
+from hexbytes import HexBytes
+
+from nucypher.blockchain.eth.constants import LENGTH_ECDSA_SIGNATURE_WITH_RECOVERY
+from nucypher.blockchain.eth.signers import KeystoreSigner, Signer
+
+PASSWORD = "so_secure"
+
+
+@pytest.fixture(scope="module")
+def keystore_file(random_account, temp_dir_path):
+    _keystore_file = temp_dir_path / "keystore_file.json"
+    with _keystore_file.open("w") as f:
+        key_data = random_account.encrypt(PASSWORD)
+        json.dump(key_data, f)
+    yield _keystore_file
+
+
+@pytest.fixture(scope="module")
+def signer(keystore_file):
+    signer = Signer.from_signer_uri(uri=f"keystore://{keystore_file.absolute()}")
+    yield signer
+
+
+def test_blank_keystore_uri():
+    with pytest.raises(
+        Signer.InvalidSignerURI, match="Blank signer URI - No keystore path provided"
+    ):
+        Signer.from_signer_uri(uri="keystore://", testnet=True)  # it's blank!
+
+
+def test_keystore_signer_from_signer_uri(random_account, signer):
+    assert isinstance(signer, KeystoreSigner)
+    assert signer.uri_scheme() == "keystore"
+
+    assert len(signer.accounts) == 1
+    assert isinstance(signer.accounts[0], str)
+    assert signer.accounts[0] == random_account.address
+    assert len(signer.accounts[0]) == 42
+
+
+def test_keystore_signer_lock_account(signer, random_account):
+    account = random_account.address
+    assert signer.is_device(account=account) is False
+    assert signer.lock_account(account=account) is True
+    assert signer.is_device(account=account) is False
+    assert signer.unlock_account(account=account, password=PASSWORD) is True
+
+
+def test_keystore_signer_message(signer, random_account):
+    message = b"Our attitude toward life determines life's attitude towards us."  # - Earl Nightingale
+    signature = signer.sign_message(account=random_account.address, message=message)
+    assert len(signature) == LENGTH_ECDSA_SIGNATURE_WITH_RECOVERY
+
+    assert (
+        signature
+        == random_account.sign_message(encode_defunct(primitive=message)).signature
+    )
+
+
+def test_keystore_signer_transaction(signer, random_account, tx_dict):
+    transaction_dict = assoc(tx_dict, "from", value=random_account.address)
+    signed_transaction = signer.sign_transaction(transaction_dict=transaction_dict)
+    assert isinstance(signed_transaction, HexBytes)
+    assert (
+        signed_transaction
+        == random_account.sign_transaction(transaction_dict).rawTransaction
+    )
+
+    transaction = Transaction.from_bytes(signed_transaction)
+    assert to_checksum_address(transaction.to) == transaction_dict["to"]

tests/unit/test_memory_signer.py

@@ -6,7 +6,6 @@ from hexbytes import HexBytes
 
 from nucypher.blockchain.eth.constants import LENGTH_ECDSA_SIGNATURE_WITH_RECOVERY
 from nucypher.blockchain.eth.signers import InMemorySigner, Signer
-from tests.unit.test_web3_signers import TRANSACTION_DICT
 
 
 @pytest.fixture(scope="function")
@@ -49,8 +48,8 @@ def test_memory_signer_message(signer, account):
     assert len(signature) == LENGTH_ECDSA_SIGNATURE_WITH_RECOVERY
 
 
-def test_memory_signer_transaction(signer, account):
-    transaction_dict = assoc(TRANSACTION_DICT, "from", value=account)
+def test_memory_signer_transaction(signer, account, tx_dict):
+    transaction_dict = assoc(tx_dict, "from", value=account)
     signed_transaction = signer.sign_transaction(transaction_dict=transaction_dict)
     assert isinstance(signed_transaction, HexBytes)
     transaction = Transaction.from_bytes(signed_transaction)

tests/unit/test_web3_signers.py

@@ -1,29 +0,0 @@
-
-
-
-import pytest
-from eth_account.account import Account
-
-from nucypher.blockchain.eth.signers import Signer
-
-TRANSACTION_DICT = {
-    'chainId': 1,
-    'nonce': 2,
-    'gasPrice': 2000000000000,
-    'gas': 314159,
-    'to': '0xd3CdA913deB6f67967B99D67aCDFa1712C293601',
-    'value': 12345,
-    'data': b'in that metric, kman is above reproach',  # thank you friends
-}
-
-
-@pytest.fixture(scope='module')
-def mock_account():
-    key = Account.create(extra_entropy='M*A*S*H* DIWOKNECNECENOE#@!')
-    account = Account.from_key(private_key=key.key)
-    return account
-
-
-def test_blank_keystore_uri():
-    with pytest.raises(Signer.InvalidSignerURI, match='Blank signer URI - No keystore path provided') as error:
-        Signer.from_signer_uri(uri='keystore://', testnet=True)  # it's blank!