Skip to content

Commit

Permalink
fix: issue where KeyfileAccount would not stay unlocked [APE-1586] (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
antazoey authored Dec 6, 2023
1 parent f797f5b commit f292af3
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 19 deletions.
3 changes: 2 additions & 1 deletion src/ape/managers/project/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
10 changes: 8 additions & 2 deletions src/ape_accounts/accounts.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import json
from os import environ
from pathlib import Path
from typing import Iterator, Optional
from typing import Dict, Iterator, Optional

import click
from eth_account import Account as EthAccount
Expand All @@ -25,6 +25,8 @@ def __init__(self):


class AccountContainer(AccountContainerAPI):
loaded_accounts: Dict[str, "KeyfileAccount"] = {}

@property
def _keyfiles(self) -> Iterator[Path]:
return self.data_folder.glob("*.json")
Expand All @@ -37,7 +39,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])
Expand Down
5 changes: 4 additions & 1 deletion src/ape_pm/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
2 changes: 2 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 2 additions & 0 deletions tests/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
42 changes: 27 additions & 15 deletions tests/functional/test_accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,22 @@ 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_recover_signer(signer):
message = encode_defunct(text="Hello Apes!")
def test_recover_signer(signer, message):
signature = signer.sign_message(message)
assert recover_signer(message, signature) == signer

Expand All @@ -62,11 +65,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)

Expand Down Expand Up @@ -270,9 +272,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)

Expand Down Expand Up @@ -329,21 +330,19 @@ 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"):
signature = keyfile_account.sign_message(message)
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"):
Expand All @@ -370,8 +369,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

Expand All @@ -381,8 +381,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)
Expand All @@ -403,6 +401,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 = {
Expand Down

0 comments on commit f292af3

Please sign in to comment.