Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: circular import issues #25

Merged
merged 4 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
139 changes: 106 additions & 33 deletions ape_safe/_cli/click_ext.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from typing import NoReturn, Sequence, Union, cast
from typing import NoReturn, Optional, Sequence, Union, cast

import click
from ape import accounts, config
from ape.api import AccountAPI
from ape.cli import ApeCliContextObject, ape_cli_context
from ape.exceptions import Abort
from ape.utils import ManagerAccessMixin
from click import BadOptionUsage, MissingParameter

from ape_safe.accounts import SafeContainer
Expand All @@ -26,37 +27,6 @@ def safe_cli_ctx():
return ape_cli_context(obj_type=SafeCliContext)


def _safe_callback(ctx, param, value):
# NOTE: For some reason, the Cli CTX object is not the SafeCliCtx yet at this point.
safes = accounts.containers["safe"]
if value is None:
# First, check config for a default. If one is there,
# we must use that.
safe_config = config.get_config("safe")
if alias := safe_config.default_safe:
return accounts.load(alias)

# If there is only 1 safe, just use that.
elif len(safes) == 1:
return next(safes.accounts)

elif len(safes) == 0:
raise Abort("First, add a safe account using command:\n\t`ape safe add`")

options = ", ".join(safes.aliases)
raise MissingParameter(message=f"Must specify one of '{options}').")

elif value in safes.aliases:
return accounts.load(value)

else:
raise BadOptionUsage("--safe", f"No safe with alias '{value}'")


safe_option = click.option("--safe", callback=_safe_callback)
safe_argument = click.argument("safe", callback=_safe_callback)


def _txn_ids_callback(ctx, param, value):
value_ls = value or []
return [int(x) if x.isnumeric() else x for x in value_ls if x]
Expand All @@ -65,3 +35,106 @@ def _txn_ids_callback(ctx, param, value):
txn_ids_argument = click.argument(
"txn_ids", nargs=-1, callback=_txn_ids_callback, metavar="NONCE_OR_SAFE_TX_HASH(s)"
)


class CallbackFactory(ManagerAccessMixin):
"""
Helper class to prevent circular import and have access
to Ape objects.
"""

@classmethod
def safe_callback(cls, ctx, param, value):
# NOTE: For some reason, the Cli CTX object is not the SafeCliCtx yet at this point.
safes = cls.account_manager.containers["safe"]
if value is None:
# First, check config for a default. If one is there,
# we must use that.
safe_config = cls.config_manager.get_config("safe")
if alias := safe_config.default_safe:
return cls.account_manager.load(alias)

# If there is only 1 safe, just use that.
elif len(safes) == 1:
return next(safes.accounts)

elif len(safes) == 0:
raise Abort("First, add a safe account using command:\n\t`ape safe add`")

options = ", ".join(safes.aliases)
raise MissingParameter(message=f"Must specify one of '{options}').")

elif value in safes.aliases:
return cls.account_manager.load(value)

else:
raise BadOptionUsage("--safe", f"No safe with alias '{value}'")

@classmethod
def submitter_callback(cls, ctx, param, val):
if val is None:
return None

elif val in cls.account_manager.aliases:
return cls.account_manager.load(val)

# Account address - execute using this account.
elif val in cls.account_manager:
return cls.account_manager[val]

# Saying "yes, execute". Use first "local signer".
elif val.lower() in ("true", "t", "1"):
safe = cls.account_manager.load(ctx.params["alias"])
if not safe.local_signers:
ctx.obj.abort("Cannot use `--execute TRUE` without a local signer.")

return safe.select_signer(for_="submitter")

return None

@classmethod
def sender_callback(cls, ctx, param, val) -> Optional[Union[AccountAPI, bool]]:
"""
Either returns the account or ``False`` meaning don't execute.
NOTE: The handling of the `--execute` flag in the `pending` CLI
all happens here EXCEPT if a pending tx is executable and no
value of `--execute` was provided.
"""
return cls._get_execute_callback(ctx, param, val, name="sender")

@classmethod
def execute_callback(cls, ctx, param, val) -> Optional[Union[AccountAPI, bool]]:
"""
Either returns the account or ``False`` meaning don't execute.
"""
return cls._get_execute_callback(ctx, param, val)

@classmethod
def _get_execute_callback(cls, ctx, param, val, name: str = "execute"):
if val is None:
# Was not given any value.
# If it is determined in `pending` that a tx can execute,
# the user will get prompted.
# Avoid this by always doing `--execute false`.
return None

elif submitter := cls.submitter_callback(ctx, param, val):
return submitter

# Saying "no, do not execute", even if we could.
elif val.lower() in ("false", "f", "0"):
return False

raise BadOptionUsage(
f"--{name}", f"`--{name}` value '{val}` not a boolean or account identifier."
)


callback_factory = CallbackFactory()
safe_option = click.option("--safe", callback=callback_factory.safe_callback)
safe_argument = click.argument("safe", callback=callback_factory.safe_callback)
submitter_option = click.option(
"--submitter", help="Account to execute", callback=callback_factory.submitter_callback
)
sender_option = click.option("--sender", callback=callback_factory.sender_callback)
execute_option = click.option("--execute", callback=callback_factory.execute_callback)
69 changes: 13 additions & 56 deletions ape_safe/_cli/pending.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,24 @@

import click
import rich
from ape import accounts
from ape.api import AccountAPI
from ape.cli import ConnectedProviderCommand
from ape.exceptions import SignatureError
from ape.types import AddressType, MessageSignature
from click.exceptions import BadOptionUsage
from eth_typing import ChecksumAddress, Hash32
from eth_utils import humanize_hash
from hexbytes import HexBytes

from ape_safe import SafeAccount
from ape_safe._cli.click_ext import SafeCliContext, safe_cli_ctx, safe_option, txn_ids_argument
from ape_safe._cli.click_ext import (
SafeCliContext,
execute_option,
safe_cli_ctx,
safe_option,
sender_option,
submitter_option,
txn_ids_argument,
)
from ape_safe.accounts import get_signatures
from ape_safe.client import UnexecutedTxData
from ape_safe.utils import get_safe_tx_hash
Expand Down Expand Up @@ -98,33 +104,6 @@ def _list(cli_ctx, safe, verbose) -> None:
click.echo()


# NOTE: The handling of the `--execute` flag in the `pending` CLI
# all happens here EXCEPT if a pending tx is executable and no
# value of `--execute` was provided.
def _handle_execute_cli_arg(ctx, param, val) -> Optional[Union[AccountAPI, bool]]:
"""
Either returns the account or ``False`` meaning don't execute
"""

if val is None:
# Was not given any value.
# If it is determined in `pending` that a tx can execute,
# the user will get prompted.
# Avoid this by always doing `--execute false`.
return None

elif submitter := _load_submitter(ctx, param, val):
return submitter

# Saying "no, do not execute", even if we could.
elif val.lower() in ("false", "f", "0"):
return False

raise BadOptionUsage(
"--execute", f"`--execute` value '{val}` not a boolean or account identifier."
)


@pending.command(cls=ConnectedProviderCommand)
@safe_cli_ctx()
@safe_option
Expand All @@ -133,7 +112,7 @@ def _handle_execute_cli_arg(ctx, param, val) -> Optional[Union[AccountAPI, bool]
@click.option("--value", type=int, help="Transaction value", default=0)
@click.option("--to", "receiver", type=ChecksumAddress, help="Transaction receiver")
@click.option("--nonce", type=int, help="Transaction nonce")
@click.option("--sender", callback=_handle_execute_cli_arg)
@sender_option
@click.option("--execute", help="Execute if possible after proposal", is_flag=True)
def propose(cli_ctx, ecosystem, safe, data, gas_price, value, receiver, nonce, sender, execute):
"""
Expand Down Expand Up @@ -192,33 +171,11 @@ def propose(cli_ctx, ecosystem, safe, data, gas_price, value, receiver, nonce, s
_execute(safe, new_tx, sender)


def _load_submitter(ctx, param, val):
if val is None:
return None

elif val in accounts.aliases:
return accounts.load(val)

# Account address - execute using this account.
elif val in accounts:
return accounts[val]

# Saying "yes, execute". Use first "local signer".
elif val.lower() in ("true", "t", "1"):
safe = accounts.load(ctx.params["alias"])
if not safe.local_signers:
ctx.obj.abort("Cannot use `--execute TRUE` without a local signer.")

return safe.select_signer(for_="submitter")

return None


@pending.command(cls=ConnectedProviderCommand)
@safe_cli_ctx()
@safe_option
@txn_ids_argument
@click.option("--execute", callback=_handle_execute_cli_arg)
@execute_option
def approve(cli_ctx: SafeCliContext, safe, txn_ids, execute):
submitter: Optional[AccountAPI] = execute if isinstance(execute, AccountAPI) else None
pending_transactions = list(
Expand Down Expand Up @@ -272,7 +229,7 @@ def approve(cli_ctx: SafeCliContext, safe, txn_ids, execute):
@safe_option
@txn_ids_argument
# NOTE: Doesn't use --execute because we don't need BOOL values.
@click.option("--submitter", help="Account to execute", callback=_load_submitter)
@submitter_option
@click.option("--nonce", help="Submitter nonce")
def execute(cli_ctx, safe, txn_ids, submitter, nonce):
"""
Expand Down Expand Up @@ -313,7 +270,7 @@ def _execute(safe: SafeAccount, txn: UnexecutedTxData, submitter: AccountAPI, **
@safe_cli_ctx()
@safe_option
@txn_ids_argument
@click.option("--execute", callback=_handle_execute_cli_arg)
@execute_option
def reject(cli_ctx: SafeCliContext, safe, txn_ids, execute):
"""
Reject one or more pending transactions
Expand Down
6 changes: 4 additions & 2 deletions ape_safe/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,17 @@ def create_client(self, key: str) -> BaseSafeClient:
return safe.client

elif key in self.addresses:
return self[cast(AddressType, key)].client
account = cast(SafeAccount, self[cast(AddressType, key)])
return account.client

elif key in self.aliases:
return self.load_account(key).client

else:
address = self.conversion_manager.convert(key, AddressType)
if address in self.addresses:
return self[cast(AddressType, key)].client
account = cast(SafeAccount, self[cast(AddressType, key)])
return account.client

# Is not locally managed.
return SafeClient(address=address, chain_id=self.chain_manager.provider.chain_id)
Expand Down
Loading