Skip to content
This repository has been archived by the owner on Dec 1, 2023. It is now read-only.

Add nile-account flag to set Nile artifacts path #315

Merged
merged 27 commits into from
Dec 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
de7a04b
add nile-account flag
andrew-fleming Nov 29, 2022
4996bdc
add overriding path to get_class_hash
andrew-fleming Nov 29, 2022
d639ffa
add overriding_path to tests
andrew-fleming Nov 29, 2022
f44e22d
fix docstring
andrew-fleming Nov 29, 2022
8bbc137
add try/except to execute_call
andrew-fleming Nov 29, 2022
c57224c
fix path to locate nile artifacts
andrew-fleming Nov 30, 2022
8052418
add _get_contracts_data test
andrew-fleming Nov 30, 2022
3b55c9e
remove unused var
andrew-fleming Nov 30, 2022
4c482bd
Merge remote-tracking branch 'origin' into fix-declare-path
andrew-fleming Nov 30, 2022
1d879f4
add unregister for accounts
andrew-fleming Dec 1, 2022
72b9c25
add watch_mode to account inits
andrew-fleming Dec 1, 2022
e8e0edf
unregister account upon account deploy rejection
andrew-fleming Dec 1, 2022
8c26a0b
remove duplicate func
andrew-fleming Dec 1, 2022
7722a76
fix formatting
andrew-fleming Dec 1, 2022
3419026
add declare account test
andrew-fleming Dec 1, 2022
9965aa3
fix formatting
andrew-fleming Dec 2, 2022
cac7199
remove unused variables
andrew-fleming Dec 2, 2022
1991538
add unregister tests, add indent to accounts.json
andrew-fleming Dec 6, 2022
1086930
add unregister test with wrong address
andrew-fleming Dec 6, 2022
835d310
consolidate abi to contract path logic, fix tests
andrew-fleming Dec 7, 2022
5033b53
add assertion error message
andrew-fleming Dec 7, 2022
0e2df27
Update src/nile/core/account.py
andrew-fleming Dec 9, 2022
e96e95f
add nile root path constants
andrew-fleming Dec 9, 2022
96e3e1d
fix deploy test
andrew-fleming Dec 9, 2022
825ff94
fix formatting
andrew-fleming Dec 9, 2022
28ae116
define NILE_ARTIFACTS_PATH as constant
andrew-fleming Dec 12, 2022
424bc90
rename test_accounts.py
andrew-fleming Dec 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/nile/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,23 @@ def register(pubkey, address, index, alias, network):
"alias": alias,
}
with open(file, "w") as file:
json.dump(accounts, file)
json.dump(accounts, file, indent=2)


def unregister(address, network):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add tests for this?

"""Unregister an account."""
file = f"{network}.{ACCOUNTS_FILENAME}"

with open(file, "r") as fp:
accounts = json.load(fp)
to_delete = None
for pubkey, data in accounts.items():
if address == data["address"]:
to_delete = pubkey
accounts.pop(to_delete, None)

with open(file, "w") as file:
json.dump(accounts, file, indent=2)


def exists(pubkey, network):
Expand Down
9 changes: 6 additions & 3 deletions src/nile/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ async def deploy(
):
"""Deploy a StarkNet smart contract."""
if not ignore_account:
account = await Account(signer, network)
account = await Account(signer, network, watch_mode=watch_mode)
await account.deploy_contract(
contract_name,
salt,
Expand Down Expand Up @@ -159,6 +159,7 @@ async def deploy(
@click.option("--max_fee", nargs=1)
@click.option("--alias")
@click.option("--overriding_path")
@click.option("--nile_account", is_flag=True)
@network_option
@mainnet_token_option
@watch_option
Expand All @@ -171,16 +172,18 @@ async def declare(
alias,
overriding_path,
token,
nile_account,
):
"""Declare a StarkNet smart contract through an Account."""
account = await Account(signer, network)
account = await Account(signer, network, watch_mode=watch_mode)
await account.declare(
contract_name,
alias=alias,
max_fee=max_fee,
overriding_path=overriding_path,
mainnet_token=token,
watch_mode=watch_mode,
nile_account=nile_account,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about making it a constructor argument and object attribute? i.e. self.is_nile_account

it could be leaner if we eventually need it for other operations, and it would debloat the function's arguments. although right now it's the same since it's just here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can maybe add this in the future, but I don't think this would work here. The point of nile_account is to direct the path to the Nile artifacts. Really, it's just for declaring a Nile account, so we need a way to adjust the path for this specific type of declaration. In other words, when an account wants to declare a Nile account, use the flag to change the path. For other declarations, Nile assumes the local artifacts/ path.

IMO a better fix would be to iterate both paths checking for the target contract. That way, we don't have to worry about any of this. I already have a working example of this improvement, but for the sake of getting this shipped, I decided not to include it

)


Expand Down Expand Up @@ -228,7 +231,7 @@ async def send(
watch_mode,
):
"""Invoke a contract's method through an Account."""
account = await Account(signer, network)
account = await Account(signer, network, watch_mode=watch_mode)
print(
"Calling {} on {} with params: {}".format(
method, address_or_alias, [x for x in params]
Expand Down
10 changes: 6 additions & 4 deletions src/nile/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@

from nile.utils import normalize_number, str_to_felt

pt = os.path.dirname(os.path.realpath(__file__)).replace("/core", "")
CONTRACTS_DIRECTORY = "contracts"
BUILD_DIRECTORY = "artifacts"
TEMP_DIRECTORY = ".temp"
ABIS_DIRECTORY = f"{BUILD_DIRECTORY}/abis"
NILE_ROOT_PATH = os.path.dirname(os.path.realpath(__file__)).replace("/core", "")
NILE_BUILD_DIR = f"{NILE_ROOT_PATH}/{BUILD_DIRECTORY}"
NILE_ABIS_DIR = f"{NILE_ROOT_PATH}/{ABIS_DIRECTORY}"
NILE_ARTIFACTS_PATH = (NILE_BUILD_DIR, NILE_ABIS_DIR)
DEPLOYMENTS_FILENAME = "deployments.txt"
DECLARATIONS_FILENAME = "declarations.txt"
ACCOUNTS_FILENAME = "accounts.json"
Expand All @@ -22,7 +25,7 @@
TRANSACTION_VERSION = 1
QUERY_VERSION_BASE = 2**128
QUERY_VERSION = QUERY_VERSION_BASE + TRANSACTION_VERSION
ETH_TOKEN_ABI = f"{pt}/artifacts/abis/ERC20.json"
ETH_TOKEN_ABI = f"{NILE_ABIS_DIR}/ERC20.json"
ETH_TOKEN_ADDRESS = "0x49D36570D4E46F48E99674BD3FCC84644DDD6B96F7C741B1562B82F9E004DC7"
UNIVERSAL_DEPLOYER_ADDRESS = (
# subject to change
Expand Down Expand Up @@ -137,8 +140,7 @@ def get_class_hash(contract_name, overriding_path=None):

def get_account_class_hash(contract="Account"):
"""Return the class_hash of an Account contract."""
pt = os.path.dirname(os.path.realpath(__file__)).replace("/core", "")
overriding_path = (f"{pt}/artifacts", f"{pt}/artifacts/abis")
overriding_path = (NILE_BUILD_DIR, NILE_ABIS_DIR)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

return get_class_hash(contract, overriding_path=overriding_path)


Expand Down
9 changes: 7 additions & 2 deletions src/nile/core/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from nile import accounts, deployments
from nile.common import (
NILE_ARTIFACTS_PATH,
QUERY_VERSION,
TRANSACTION_VERSION,
UNIVERSAL_DEPLOYER_ADDRESS,
Expand Down Expand Up @@ -106,8 +107,7 @@ async def __init__(
async def deploy(self, salt=None, max_fee=None, query_type=None, watch_mode=None):
"""Deploy an Account contract for the given private key."""
index = accounts.current_index(self.network)
pt = os.path.dirname(os.path.realpath(__file__)).replace("/core", "")
overriding_path = (f"{pt}/artifacts", f"{pt}/artifacts/abis")
overriding_path = NILE_ARTIFACTS_PATH

class_hash = get_account_class_hash("Account")
salt = 0 if salt is None else normalize_number(salt)
Expand Down Expand Up @@ -152,10 +152,15 @@ async def declare(
overriding_path=None,
mainnet_token=None,
watch_mode=None,
nile_account=False,
):
"""Declare a contract through an Account."""
max_fee, nonce, _ = await self._process_arguments(max_fee, nonce)

if nile_account:
assert overriding_path is None, "Cannot override path to Nile account."
overriding_path = NILE_ARTIFACTS_PATH

contract_class = get_contract_class(
contract_name=contract_name, overriding_path=overriding_path
)
Expand Down
3 changes: 2 additions & 1 deletion src/nile/core/deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
)
from starkware.starknet.services.api.gateway.transaction import DeployAccount

from nile import deployments
from nile import accounts, deployments
from nile.common import (
ABIS_DIRECTORY,
BUILD_DIRECTORY,
Expand Down Expand Up @@ -182,6 +182,7 @@ async def deploy_account(
tx_status = await status(tx_hash, network, watch_mode)
if tx_status.status.is_rejected:
deployments.unregister(address, network, alias, abi=register_abi)
accounts.unregister(address, network)
return

return address, register_abi
12 changes: 8 additions & 4 deletions src/nile/utils/debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,18 @@ async def debug_message(error_message, tx_hash, network, contracts_file=None):

def _get_contracts_data(contracts_file, network, addresses):
file = contracts_file or f"{network}.{DEPLOYMENTS_FILENAME}"
# contracts_file should already link to compiled contracts and not ABIs
to_contract = (lambda x: x) if contracts_file else _abi_to_build_path
to_contract = (lambda x: x) if contracts_file else _abi_to_path
contracts = _locate_error_lines_with_abis(file, addresses, to_contract)
return contracts


def _abi_to_build_path(filename):
return os.path.join(BUILD_DIRECTORY, os.path.basename(filename))
def _abi_to_path(filename):
build_path = os.path.join(BUILD_DIRECTORY, os.path.basename(filename))
if os.path.isfile(build_path):
return build_path
else:
pt = os.path.dirname(os.path.realpath(__file__)).replace("/utils", "/artifacts")
return os.path.join(pt, os.path.basename(filename))


def _locate_error_lines_with_abis(file, addresses, to_contract):
Expand Down
49 changes: 41 additions & 8 deletions tests/commands/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
from nile.common import (
ABIS_DIRECTORY,
BUILD_DIRECTORY,
NILE_ABIS_DIR,
NILE_ARTIFACTS_PATH,
NILE_BUILD_DIR,
QUERY_VERSION,
TRANSACTION_VERSION,
UNIVERSAL_DEPLOYER_ADDRESS,
Expand Down Expand Up @@ -60,13 +63,8 @@ async def test_account_init_bad_key(caplog):
@patch("nile.core.account.Signer.sign_deployment", return_value=SIGNATURE)
@patch("nile.core.account.os.path.dirname")
async def test_deploy(mock_path, mock_signer, mock_hash, mock_deploy):
test_path = "/overriding_path"
overriding_path = (
f"{test_path}/artifacts",
f"{test_path}/artifacts/abis",
)

mock_path.return_value.replace.return_value = test_path
# overriding_path is set to Nile's root path for Nile Account deployments.
nile_root_path = NILE_BUILD_DIR, NILE_ABIS_DIR

account = await Account(KEY, NETWORK, salt=SALT, max_fee=MAX_FEE)
calldata = [account.signer.public_key]
Expand All @@ -78,7 +76,7 @@ async def test_deploy(mock_path, mock_signer, mock_hash, mock_deploy):
signature=SIGNATURE,
max_fee=MAX_FEE,
query_type=None,
overriding_path=overriding_path,
overriding_path=nile_root_path,
watch_mode=None,
)

Expand Down Expand Up @@ -147,6 +145,41 @@ async def test_declare(mock_declare, mock_get_class, mock_hash, mock_deploy):
)


@pytest.mark.asyncio
@pytest.mark.parametrize(
"nile_account, overriding_path", [(False, None), (True, NILE_ARTIFACTS_PATH)]
)
@patch("nile.core.account.deploy_account", return_value=(MOCK_ADDRESS, MOCK_INDEX))
@patch("nile.core.account.get_account_class_hash", return_value=CLASS_HASH)
@patch("nile.core.account.get_contract_class", return_value="ContractClass")
@patch("nile.core.account.declare")
async def test_declare_account(
mock_declare, mock_get_class, mock_hash, mock_deploy, nile_account, overriding_path
):
account = await Account(KEY, NETWORK)

signature = [999, 888]
nonce = 4
contract_name = "Account"

account.signer.sign_declare = MagicMock(return_value=signature)

args = {
"contract_name": contract_name,
"nonce": nonce,
}

if nile_account:
args["nile_account"] = True

await account.declare(**args)

# Check 'get_contract_class' call
mock_get_class.assert_called_once_with(
contract_name=contract_name, overriding_path=overriding_path
)


@pytest.mark.asyncio
@pytest.mark.parametrize("deployer_address", [None, 0xDE0])
@pytest.mark.parametrize("watch_mode", [None, "debug"])
Expand Down
40 changes: 33 additions & 7 deletions tests/commands/test_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@

import pytest

from nile.common import BUILD_DIRECTORY
from nile.utils.debug import _abi_to_build_path, _locate_error_lines_with_abis
from nile.common import BUILD_DIRECTORY, DEPLOYMENTS_FILENAME
from nile.utils.debug import (
_abi_to_path,
_get_contracts_data,
_locate_error_lines_with_abis,
)
from nile.utils.status import status

MOCK_HASH = 1234
Expand All @@ -18,6 +22,7 @@
MOCK_FILE = 123
ACCEPTED_OUT = b'{"tx_status": "ACCEPTED_ON_L2"}'
REJECTED_OUT = b'{"tx_failure_reason": {"error_message": "E"}, "tx_status": "REJECTED"}'
ADDRESSES = {0x123, 0x456}


@pytest.fixture(autouse=True)
Expand All @@ -26,10 +31,11 @@ def tmp_working_dir(monkeypatch, tmp_path):
return tmp_path


def test__abi_to_build_path():
def test__abi_to_path():
Path(BUILD_DIRECTORY).mkdir()
filename = "contract"
assert f"{BUILD_DIRECTORY}/{filename}" == _abi_to_build_path(filename)
open(f"{BUILD_DIRECTORY}/contract.json", "w")

assert f"{BUILD_DIRECTORY}/contract.json" == _abi_to_path("contract.json")


@pytest.mark.parametrize(
Expand All @@ -39,7 +45,7 @@ def test__abi_to_build_path():
([f"{DEBUG_ADDRESS}:{ABI_PATH}:{ALIAS}"], [int(DEBUG_ADDRESS, 16)]),
],
)
@patch("nile.utils.debug._abi_to_build_path", return_value=ABI_PATH)
@patch("nile.utils.debug._abi_to_path", return_value=ABI_PATH)
def test__locate_error_lines_with_abis_with_and_without_alias(
mock_path, file, address_set
):
Expand All @@ -53,7 +59,7 @@ def test__locate_error_lines_with_abis_with_and_without_alias(
assert return_array == [f"{DEBUG_ADDRESS}:{ABI_PATH}"]


@patch("nile.utils.debug._abi_to_build_path", return_value=ABI_PATH)
@patch("nile.utils.debug._abi_to_path", return_value=ABI_PATH)
def test__locate_error_lines_with_abis_misformatted_line(mock_path, caplog):
logging.getLogger().setLevel(logging.INFO)

Expand Down Expand Up @@ -87,3 +93,23 @@ async def test_status_feedback_with_message(mock_output, output, expected, caplo
await status(MOCK_HASH, NETWORK, "debug")

assert expected in caplog.text


@pytest.mark.parametrize(
"contracts_file, expected",
[
(None, _abi_to_path),
("contracts_file.json", "contracts_file.json"),
],
)
@patch("nile.utils.debug._locate_error_lines_with_abis")
def test__get_contracts_data(mock_locate, contracts_file, expected):
_get_contracts_data(contracts_file, NETWORK, ADDRESSES)

if not contracts_file:
mock_locate.assert_called_once_with(
f"{NETWORK}.{DEPLOYMENTS_FILENAME}", ADDRESSES, expected
)
else:
_contracts_file = mock_locate.call_args[0][0]
assert _contracts_file == expected
Loading