diff --git a/src/ape/api/accounts.py b/src/ape/api/accounts.py index fefe7d8278..3b96f912ad 100644 --- a/src/ape/api/accounts.py +++ b/src/ape/api/accounts.py @@ -1,9 +1,12 @@ from pathlib import Path -from typing import TYPE_CHECKING, Iterator, List, Optional, Type, Union +from typing import TYPE_CHECKING, Any, Iterator, List, Optional, Type, Union import click +from eip712.messages import EIP712Message from eip712.messages import SignableMessage as EIP712SignableMessage from eth_account import Account +from eth_account.messages import encode_defunct +from hexbytes import HexBytes from ape.api.address import BaseAddress from ape.api.transactions import ReceiptAPI, TransactionAPI @@ -54,18 +57,21 @@ def alias(self) -> Optional[str]: return None @abstractmethod - def sign_message(self, msg: SignableMessage) -> Optional[MessageSignature]: + def sign_message(self, msg: Any, **signer_options) -> Optional[MessageSignature]: """ Sign a message. Args: - msg (:class:`~ape.types.signatures.SignableMessage`): The message to sign. + msg (Any): The message to sign. Account plugins can handle various types of messages. + For example, :class:`~ape_accounts.accouns.KeyfileAccount` can handle + :class:`~ape.types.signatures.SignableMessage`, str, int, and bytes. See these `docs `__ # noqa: E501 - for more type information on this type. + for more type information on the ``SignableMessage`` type. + **signer_options: Additional kwargs given to the signer to modify the signing operation. Returns: - :class:`~ape.types.signatures.MessageSignature` (optional): The signed message. + :class:`~ape.types.signatures.MessageSignature` (optional): The signature corresponding to the message. """ @abstractmethod @@ -258,7 +264,7 @@ def declare(self, contract: "ContractContainer", *args, **kwargs) -> ReceiptAPI: def check_signature( self, - data: Union[SignableMessage, TransactionAPI], + data: Union[SignableMessage, TransactionAPI, str, EIP712Message, int], signature: Optional[MessageSignature] = None, # TransactionAPI doesn't need it ) -> bool: """ @@ -273,6 +279,12 @@ def check_signature( Returns: bool: ``True`` if the data was signed by this account. ``False`` otherwise. """ + if isinstance(data, str): + data = encode_defunct(text=data) + elif isinstance(data, int): + data = encode_defunct(hexstr=HexBytes(data).hex()) + elif isinstance(data, EIP712Message): + data = data.signable_message if isinstance(data, (SignableMessage, EIP712SignableMessage)): if signature: return self.address == Account.recover_message(data, vrs=signature) @@ -494,7 +506,7 @@ class ImpersonatedAccount(AccountAPI): def address(self) -> AddressType: return self.raw_address - def sign_message(self, msg: SignableMessage) -> Optional[MessageSignature]: + def sign_message(self, msg: Any, **signer_options) -> Optional[MessageSignature]: raise NotImplementedError("This account cannot sign messages") def sign_transaction(self, txn: TransactionAPI, **kwargs) -> Optional[TransactionAPI]: diff --git a/src/ape/api/providers.py b/src/ape/api/providers.py index bd90bcd9d0..ab78d0b69f 100644 --- a/src/ape/api/providers.py +++ b/src/ape/api/providers.py @@ -824,6 +824,10 @@ def _sanitize_web3_url(msg: str) -> str: parts = msg.split("URI: ") prefix = parts[0].strip() rest = parts[1].split(" ") + + # * To remove the `,` from the url http://127.0.0.1:8545, + if "," in rest[0]: + rest[0] = rest[0].rstrip(",") sanitized_url = sanitize_url(rest[0]) return f"{prefix} URI: {sanitized_url} {' '.join(rest[1:])}" diff --git a/src/ape/managers/project/manager.py b/src/ape/managers/project/manager.py index b521398c34..749615494c 100644 --- a/src/ape/managers/project/manager.py +++ b/src/ape/managers/project/manager.py @@ -752,7 +752,8 @@ def track_deployment(self, contract: ContractInstance): if destination.is_file(): logger.debug("Deployment already tracked. Re-tracking.") - destination.unlink() + # NOTE: missing_ok=True to handle race condition. + destination.unlink(missing_ok=True) destination.write_text(artifact.json()) diff --git a/src/ape/types/signatures.py b/src/ape/types/signatures.py index 0e8ec3611b..81ee65828f 100644 --- a/src/ape/types/signatures.py +++ b/src/ape/types/signatures.py @@ -1,8 +1,9 @@ -from typing import Iterator, Union +from typing import Iterator, Optional, Union from eth_account import Account from eth_account.messages import SignableMessage from eth_utils import to_bytes, to_hex +from hexbytes import HexBytes from pydantic.dataclasses import dataclass from ape.types import AddressType @@ -13,6 +14,41 @@ ) +# Improve repr to force hexstr for body instead of raw bytes. +def signable_message_repr(msg) -> str: + name = getattr(SignableMessage, "__name__", "SignableMessage") + default_value = "" # Shouldn't happen + version_str = _bytes_to_human_str(msg.version) or default_value + header_str = _bytes_to_human_str(msg.header) or default_value + body_str = _bytes_to_human_str(msg.body) or default_value + return f"{name}(" f'version="{version_str}", header="{header_str}", body="{body_str}")' + + +SignableMessage.__repr__ = signable_message_repr # type: ignore[method-assign] + + +def _bytes_to_human_str(bytes_value: bytes) -> Optional[str]: + try: + # Try as text + return bytes_value.decode("utf8") + except Exception: + pass + + try: + # Try as hex + return HexBytes(bytes_value).hex() + except Exception: + pass + + try: + # Try normal str + return str(bytes_value) + except Exception: + pass + + return None + + def _left_pad_bytes(val: bytes, num_bytes: int) -> bytes: return b"\x00" * (num_bytes - len(val)) + val if len(val) < num_bytes else val diff --git a/src/ape_accounts/accounts.py b/src/ape_accounts/accounts.py index 588e8038a5..b5fa2b5e5b 100644 --- a/src/ape_accounts/accounts.py +++ b/src/ape_accounts/accounts.py @@ -1,10 +1,12 @@ import json from os import environ from pathlib import Path -from typing import Iterator, Optional +from typing import Any, Dict, Iterator, Optional import click +from eip712.messages import EIP712Message from eth_account import Account as EthAccount +from eth_account.messages import encode_defunct from eth_keys import keys # type: ignore from eth_utils import to_bytes from ethpm_types import HexBytes @@ -25,6 +27,8 @@ def __init__(self): class AccountContainer(AccountContainerAPI): + loaded_accounts: Dict[str, "KeyfileAccount"] = {} + @property def _keyfiles(self) -> Iterator[Path]: return self.data_folder.glob("*.json") @@ -37,7 +41,11 @@ def aliases(self) -> Iterator[str]: @property def accounts(self) -> Iterator[AccountAPI]: for keyfile in self._keyfiles: - yield KeyfileAccount(keyfile_path=keyfile) + if keyfile.stem not in self.loaded_accounts: + keyfile_account = KeyfileAccount(keyfile_path=keyfile) + self.loaded_accounts[keyfile.stem] = keyfile_account + + yield self.loaded_accounts[keyfile.stem] def __len__(self) -> int: return len([*self._keyfiles]) @@ -149,8 +157,50 @@ def delete(self): self.__decrypt_keyfile(passphrase) self.keyfile_path.unlink() - def sign_message(self, msg: SignableMessage) -> Optional[MessageSignature]: - user_approves = self.__autosign or click.confirm(f"{msg}\n\nSign: ") + def sign_message(self, msg: Any, **signer_options) -> Optional[MessageSignature]: + user_approves = False + + if isinstance(msg, str): + user_approves = self.__autosign or click.confirm(f"Message: {msg}\n\nSign: ") + msg = encode_defunct(text=msg) + elif isinstance(msg, int): + user_approves = self.__autosign or click.confirm(f"Message: {msg}\n\nSign: ") + msg = encode_defunct(hexstr=HexBytes(msg).hex()) + elif isinstance(msg, bytes): + user_approves = self.__autosign or click.confirm(f"Message: {msg.hex()}\n\nSign: ") + msg = encode_defunct(primitive=msg) + elif isinstance(msg, EIP712Message): + # Display message data to user + display_msg = "Signing EIP712 Message\n" + + # Domain Data + display_msg += "Domain\n" + if msg._name_: + display_msg += f"\tName: {msg._name_}\n" + if msg._version_: + display_msg += f"\tVersion: {msg._version_}\n" + if msg._chainId_: + display_msg += f"\tChain ID: {msg._chainId_}\n" + if msg._verifyingContract_: + display_msg += f"\tContract: {msg._verifyingContract_}\n" + if msg._salt_: + display_msg += f"\tSalt: 0x{msg._salt_.hex()}\n" + + # Message Data + display_msg += "Message\n" + for field, value in msg._body_["message"].items(): + display_msg += f"\t{field}: {value}\n" + + user_approves = self.__autosign or click.confirm(f"{display_msg}\nSign: ") + + # Convert EIP712Message to SignableMessage for handling below + msg = msg.signable_message + elif isinstance(msg, SignableMessage): + user_approves = self.__autosign or click.confirm(f"{msg}\n\nSign: ") + else: + logger.warning("Unsupported message type, (type=%r, msg=%r)", type(msg), msg) + return None + if not user_approves: return None diff --git a/src/ape_ethereum/ecosystem.py b/src/ape_ethereum/ecosystem.py index 07c70c2250..81766a8e99 100644 --- a/src/ape_ethereum/ecosystem.py +++ b/src/ape_ethereum/ecosystem.py @@ -639,7 +639,9 @@ def create_transaction(self, **kwargs) -> TransactionAPI: if isinstance(kwargs.get("chainId"), str): kwargs["chainId"] = int(kwargs["chainId"], 16) - elif "chainId" not in kwargs and self.network_manager.active_provider is not None: + elif ( + "chainId" not in kwargs or kwargs["chainId"] is None + ) and self.network_manager.active_provider is not None: kwargs["chainId"] = self.provider.chain_id if "input" in kwargs: @@ -660,7 +662,16 @@ def create_transaction(self, **kwargs) -> TransactionAPI: kwargs["gas"] = kwargs.pop("gas_limit", kwargs.get("gas")) if "value" in kwargs and not isinstance(kwargs["value"], int): - kwargs["value"] = self.conversion_manager.convert(kwargs["value"], int) + value = kwargs["value"] or 0 # Convert None to 0. + kwargs["value"] = self.conversion_manager.convert(value, int) + + # This causes problems in pydantic for some reason. + if "gas_price" in kwargs and kwargs["gas_price"] is None: + del kwargs["gas_price"] + + # None is not allowed, the user likely means `b""`. + if "data" in kwargs and kwargs["data"] is None: + kwargs["data"] = b"" return txn_class(**kwargs) diff --git a/src/ape_pm/compiler.py b/src/ape_pm/compiler.py index d4e1d9770b..f6b56bd442 100644 --- a/src/ape_pm/compiler.py +++ b/src/ape_pm/compiler.py @@ -31,12 +31,15 @@ def compile( get_relative_path(path, base_path) if base_path and path.is_absolute() else path ) source_id = str(source_path) + code = path.read_text() + if not code: + continue try: # NOTE: Always set the source ID to the source of the JSON file # to avoid manifest corruptions later on. contract_type = self.compile_code( - path.read_text(), + code, base_path=base_path, sourceId=source_id, ) diff --git a/src/ape_test/accounts.py b/src/ape_test/accounts.py index f0f18e8219..9742359c99 100644 --- a/src/ape_test/accounts.py +++ b/src/ape_test/accounts.py @@ -1,8 +1,9 @@ -from typing import Iterator, List, Optional +from typing import Any, Iterator, List, Optional from eth_account import Account as EthAccount -from eth_account.messages import SignableMessage +from eth_account.messages import SignableMessage, encode_defunct from eth_utils import to_bytes +from hexbytes import HexBytes from ape.api import TestAccountAPI, TestAccountContainerAPI, TransactionAPI from ape.types import AddressType, MessageSignature, TransactionSignature @@ -101,13 +102,23 @@ def alias(self) -> str: def address(self) -> AddressType: return self.network_manager.ethereum.decode_address(self.address_str) - def sign_message(self, msg: SignableMessage) -> Optional[MessageSignature]: - signed_msg = EthAccount.sign_message(msg, self.private_key) - return MessageSignature( - v=signed_msg.v, - r=to_bytes(signed_msg.r), - s=to_bytes(signed_msg.s), - ) + def sign_message(self, msg: Any, **signer_options) -> Optional[MessageSignature]: + # Convert str and int to SignableMessage if needed + if isinstance(msg, str): + msg = encode_defunct(text=msg) + elif isinstance(msg, int): + msg = HexBytes(msg).hex() + msg = encode_defunct(hexstr=msg) + + # Process SignableMessage + if isinstance(msg, SignableMessage): + signed_msg = EthAccount.sign_message(msg, self.private_key) + return MessageSignature( + v=signed_msg.v, + r=to_bytes(signed_msg.r), + s=to_bytes(signed_msg.s), + ) + return None def sign_transaction(self, txn: TransactionAPI, **kwargs) -> Optional[TransactionAPI]: # Signs anything that's given to it diff --git a/tests/conftest.py b/tests/conftest.py index 66590fbce9..932cf5dff8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -309,6 +309,8 @@ def empty_data_folder(): @pytest.fixture def keyfile_account(owner, keyparams, temp_accounts_path, temp_keyfile_account_ctx): with temp_keyfile_account_ctx(temp_accounts_path, ALIAS, keyparams, owner) as account: + # Ensure starts off locked. + account.lock() yield account diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index b103d19159..4e905f950e 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -173,6 +173,8 @@ def address(): @pytest.fixture def second_keyfile_account(sender, keyparams, temp_accounts_path, temp_keyfile_account_ctx): with temp_keyfile_account_ctx(temp_accounts_path, ALIAS_2, keyparams, sender) as account: + # Ensure starts off locked. + account.lock() yield account diff --git a/tests/functional/test_accounts.py b/tests/functional/test_accounts.py index fd7690e3f3..b4d24e182b 100644 --- a/tests/functional/test_accounts.py +++ b/tests/functional/test_accounts.py @@ -38,19 +38,40 @@ def core_account(request, owner, keyfile_account): yield keyfile_account # from ape_accounts plugin +@pytest.fixture +def message(): + return encode_defunct(text="Hello Apes!") + + class Foo(EIP712Message): _name_: "string" = "Foo" # type: ignore # noqa: F821 bar: "address" # type: ignore # noqa: F821 -def test_sign_message(signer): - message = encode_defunct(text="Hello Apes!") +def test_sign_message(signer, message): + signature = signer.sign_message(message) + assert signer.check_signature(message, signature) + + +def test_sign_string(signer): + message = "Hello Apes!" signature = signer.sign_message(message) assert signer.check_signature(message, signature) -def test_recover_signer(signer): - message = encode_defunct(text="Hello Apes!") +def test_sign_int(signer): + message = 4 + signature = signer.sign_message(message) + assert signer.check_signature(message, signature) + + +def test_sign_message_unsupported_type_returns_none(signer): + message = 1234.123 + signature = signer.sign_message(message) + assert signature is None + + +def test_recover_signer(signer, message): signature = signer.sign_message(message) assert recover_signer(message, signature) == signer @@ -62,11 +83,10 @@ def test_sign_eip712_message(signer): assert signer.check_signature(message, signature) -def test_sign_message_with_prompts(runner, keyfile_account): +def test_sign_message_with_prompts(runner, keyfile_account, message): # "y\na\ny": yes sign, password, yes keep unlocked start_nonce = keyfile_account.nonce with runner.isolation(input="y\na\ny"): - message = encode_defunct(text="Hello Apes!") signature = keyfile_account.sign_message(message) assert keyfile_account.check_signature(message, signature) @@ -270,9 +290,8 @@ def test_accounts_contains(accounts, owner): assert owner.address in accounts -def test_autosign_messages(runner, keyfile_account): +def test_autosign_messages(runner, keyfile_account, message): keyfile_account.set_autosign(True, passphrase="a") - message = encode_defunct(text="Hello Apes!") signature = keyfile_account.sign_message(message) assert keyfile_account.check_signature(message, signature) @@ -329,9 +348,8 @@ def test_contract_as_sender_non_fork_network(contract_instance): contract_instance.setNumber(5, sender=contract_instance) -def test_unlock_with_passphrase_and_sign_message(runner, keyfile_account): +def test_unlock_with_passphrase_and_sign_message(runner, keyfile_account, message): keyfile_account.unlock(passphrase="a") - message = encode_defunct(text="Hello Apes!") # y: yes, sign (note: unlocking makes the key available but is not the same as autosign). with runner.isolation(input="y\n"): @@ -339,11 +357,10 @@ def test_unlock_with_passphrase_and_sign_message(runner, keyfile_account): assert keyfile_account.check_signature(message, signature) -def test_unlock_from_prompt_and_sign_message(runner, keyfile_account): +def test_unlock_from_prompt_and_sign_message(runner, keyfile_account, message): # a = password with runner.isolation(input="a\n"): keyfile_account.unlock() - message = encode_defunct(text="Hello Apes!") # yes, sign the message with runner.isolation(input="y\n"): @@ -370,8 +387,9 @@ def test_unlock_from_prompt_and_sign_transaction(runner, keyfile_account, receiv assert receipt.receiver == receiver -def test_unlock_with_passphrase_from_env_and_sign_message(runner, keyfile_account): +def test_unlock_with_passphrase_from_env_and_sign_message(runner, keyfile_account, message): ENV_VARIABLE = f"APE_ACCOUNTS_{keyfile_account.alias}_PASSPHRASE" + # Set environment variable with passphrase environ[ENV_VARIABLE] = PASSPHRASE @@ -381,8 +399,6 @@ def test_unlock_with_passphrase_from_env_and_sign_message(runner, keyfile_accoun # Account should be unlocked assert not keyfile_account.locked - message = encode_defunct(text="Hello Apes!") - # y: yes, sign (note: unlocking makes the key available but is not the same as autosign). with runner.isolation(input="y\n"): signature = keyfile_account.sign_message(message) @@ -403,6 +419,20 @@ def test_unlock_with_wrong_passphrase_from_env(keyfile_account): assert keyfile_account.locked +def test_unlock_and_reload(runner, accounts, keyfile_account, message): + """ + Tests against a condition where reloading after unlocking + would not honor unlocked state. + """ + keyfile_account.unlock(passphrase="a") + reloaded_account = accounts.load(keyfile_account.alias) + + # y: yes, sign (note: unlocking makes the key available but is not the same as autosign). + with runner.isolation(input="y\n"): + signature = reloaded_account.sign_message(message) + assert keyfile_account.check_signature(message, signature) + + def test_custom_num_of_test_accounts_config(test_accounts, temp_config): custom_number_of_test_accounts = 20 test_config = { diff --git a/tests/functional/test_ecosystem.py b/tests/functional/test_ecosystem.py index 3c165eef8e..ad3221c259 100644 --- a/tests/functional/test_ecosystem.py +++ b/tests/functional/test_ecosystem.py @@ -10,7 +10,7 @@ from ape.types import AddressType from ape.utils import DEFAULT_LOCAL_TRANSACTION_ACCEPTANCE_TIMEOUT from ape_ethereum.ecosystem import BLUEPRINT_HEADER, Block -from ape_ethereum.transactions import TransactionType +from ape_ethereum.transactions import DynamicFeeTransaction, StaticFeeTransaction, TransactionType LOG = { "removed": False, @@ -378,6 +378,38 @@ def test_create_transaction_uses_network_gas_limit(tx_type, ethereum, eth_tester assert tx.gas_limit == eth_tester_provider.max_gas +def test_create_transaction_with_none_values(ethereum, eth_tester_provider): + """ + Tests against None being in place of values in kwargs, + causing their actual defaults not to get used and ValidationErrors + to occur. + """ + static = ethereum.create_transaction( + value=None, data=None, chain_id=None, gas_price=None, nonce=None, receiver=None + ) + dynamic = ethereum.create_transaction( + value=None, + data=None, + chain_id=None, + max_fee=None, + max_prioriy_fee=None, + nonce=None, + receiver=None, + ) + assert isinstance(static, StaticFeeTransaction) # Because used gas_price + assert isinstance(dynamic, DynamicFeeTransaction) # Because used max_fee + + for tx in (static, dynamic): + assert tx.data == b"" # None is not allowed. + assert tx.value == 0 # None is same as 0. + assert tx.chain_id == eth_tester_provider.chain_id + assert tx.nonce is None + + assert static.gas_price is None + assert dynamic.max_fee is None + assert dynamic.max_priority_fee is None + + @pytest.mark.parametrize("tx_type", TransactionType) def test_encode_transaction(tx_type, ethereum, vyper_contract_instance, owner, eth_tester_provider): abi = vyper_contract_instance.contract_type.methods[0] diff --git a/tests/functional/test_provider.py b/tests/functional/test_provider.py index 9df5d27dad..45b04f7e8b 100644 --- a/tests/functional/test_provider.py +++ b/tests/functional/test_provider.py @@ -6,6 +6,7 @@ from eth_utils import ValidationError from web3.exceptions import ContractPanicError +from ape.api.providers import _sanitize_web3_url from ape.exceptions import BlockNotFoundError, ContractLogicError, TransactionNotFoundError from ape.types import LogFilter from ape.utils import DEFAULT_TEST_CHAIN_ID @@ -262,3 +263,10 @@ def test_prepare_tx_with_max_gas(tx_type, eth_tester_provider, ethereum, owner): actual = eth_tester_provider.prepare_transaction(tx) assert actual.gas_limit == eth_tester_provider.max_gas + + +def test_no_comma_in_rpc_url(): + test_url = "URI: http://127.0.0.1:8545," + sanitised_url = _sanitize_web3_url(test_url) + + assert "," not in sanitised_url diff --git a/tests/functional/test_types.py b/tests/functional/test_types.py index 2c0ec7d620..eb4a06d979 100644 --- a/tests/functional/test_types.py +++ b/tests/functional/test_types.py @@ -4,7 +4,7 @@ from eth_utils import to_hex from ethpm_types.abi import EventABI -from ape.types import ContractLog, LogFilter, TransactionSignature +from ape.types import ContractLog, LogFilter, SignableMessage, TransactionSignature from ape.utils import ZERO_ADDRESS TXN_HASH = "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa222222222222222222222222" @@ -110,3 +110,24 @@ def test_topic_filter_encoding(): def test_signature_repr(): signature = TransactionSignature(v=0, r=b"123", s=b"456") assert repr(signature) == "" + + +def test_signable_message_repr(): + version = b"E" + header = b"thereum Signed Message:\n32" + body = ( + b"\x86\x05\x99\xc6\xfa\x0f\x05|po(\x1f\xe3\x84\xc0\x0f" + b"\x13\xb2\xa6\x91\xa3\xb8\x90\x01\xc0z\xa8\x17\xbe'\xf3\x13" + ) + message = SignableMessage(version=version, header=header, body=body) + + actual = repr(message) + expected_version = "E" + expected_header = "thereum Signed Message:\n32" + expected_body = "0x860599c6fa0f057c706f281fe384c00f13b2a691a3b89001c07aa817be27f313" + expected = ( + f'SignableMessage(version="{expected_version}", header="{expected_header}", ' + f'body="{expected_body}")' + ) + + assert actual == expected