Merge branch 'main' into fix/singleton-check

.pre-commit-config.yaml

@@ -10,18 +10,18 @@ repos:
       - id: isort
 
 -   repo: https://github.com/psf/black
-    rev: 23.12.0
+    rev: 24.1.1
     hooks:
       - id: black
         name: black
 
 -   repo: https://github.com/pycqa/flake8
-    rev: 6.1.0
+    rev: 7.0.0
     hooks:
     -   id: flake8
 
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.7.1
+    rev: v1.8.0
     hooks:
     -   id: mypy
         additional_dependencies: [types-requests, types-setuptools, pydantic]

ape-config.yaml

@@ -1,5 +1,11 @@
 contracts_folder: tests/contracts
 
+plugins:
+  - name: foundry
+  - name: solidity
+  - name: etherscan
+  - name: alchemy
+
 ethereum:
   mainnet:
     default_provider: alchemy

ape_safe/_cli/click_ext.py

@@ -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
@@ -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]
@@ -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)

ape_safe/_cli/pending.py

@@ -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
@@ -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
@@ -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):
     """
@@ -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(
@@ -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):
     """
@@ -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

ape_safe/accounts.py

@@ -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)

ape_safe/client/base.py

@@ -34,34 +34,28 @@ def __init__(self, transaction_service_url: str):
 
     @property
     @abstractmethod
-    def safe_details(self) -> SafeDetails:
-        ...
+    def safe_details(self) -> SafeDetails: ...
 
     @abstractmethod
-    def get_next_nonce(self) -> int:
-        ...
+    def get_next_nonce(self) -> int: ...
 
     @abstractmethod
-    def _all_transactions(self) -> Iterator[SafeApiTxData]:
-        ...
+    def _all_transactions(self) -> Iterator[SafeApiTxData]: ...
 
     @abstractmethod
-    def get_confirmations(self, safe_tx_hash: SafeTxID) -> Iterator[SafeTxConfirmation]:
-        ...
+    def get_confirmations(self, safe_tx_hash: SafeTxID) -> Iterator[SafeTxConfirmation]: ...
 
     @abstractmethod
     def post_transaction(
         self, safe_tx: SafeTx, signatures: Dict[AddressType, MessageSignature], **kwargs
-    ):
-        ...
+    ): ...
 
     @abstractmethod
     def post_signatures(
         self,
         safe_tx_or_hash: Union[SafeTx, SafeTxID],
         signatures: Dict[AddressType, MessageSignature],
-    ):
-        ...
+    ): ...
 
     """Shared methods"""
 

ape_safe/exceptions.py

@@ -85,10 +85,10 @@ def __exit__(self, exc_type: Type[BaseException], exc: BaseException, tb):
         if (
             isinstance(exc, ContractLogicError)  # NOTE: Just for mypy
             and exc_type == ContractLogicError
-            and exc.message.startswith("GS")
-            and exc.message in SAFE_ERROR_CODES
         ):
-            raise SafeLogicError(exc.message) from exc
+            message = exc.message.replace("revert: ", "").strip()
+            if message.startswith("GS") and message in SAFE_ERROR_CODES:
+                raise SafeLogicError(exc.message.replace("revert: ", "")) from exc
 
         # NOTE: Will raise `exc` by default because we did not return anything