fix: handle user decline on transaction signing, fix install from source, and use ruff (#91)

* fix: handle user decline

* docs: show how to install from source

* fix: safe fixes

* refactor(Account): remove `SignatureError`; streamline logic

---------

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>

Author: antazoey

.github/workflows/test.yaml

@@ -26,14 +26,14 @@ jobs:
             python -m pip install --upgrade pip
             pip install .[lint]
 
-        - name: Run Black
-          run: black --check .
+        - name: Run Ruff Lint
+          run: ruff check .
 
-        - name: Run isort
-          run: isort --check-only .
+        - name: Run Ruff Format
+          run: ruff format --check .
 
-        - name: Run flake8
-          run: flake8 .
+        - name: Run mdformat
+          run: mdformat . --check
 
     type-check:
         runs-on: ubuntu-latest

.pre-commit-config.yaml

@@ -4,32 +4,21 @@ repos:
     hooks:
     -   id: check-yaml
 
--   repo: https://github.com/PyCQA/isort
-    rev: 5.13.2
+-   repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.12.0  # Latest ruff version
     hooks:
-      - id: isort
-
--   repo: https://github.com/psf/black
-    rev: 24.10.0
-
-    hooks:
-      - id: black
-        name: black
-
--   repo: https://github.com/pycqa/flake8
-    rev: 7.1.1
-    hooks:
-    -   id: flake8
-        additional_dependencies: [flake8-breakpoint, flake8-print, flake8-pydantic, flake8-type-checking]
+    -   id: ruff
+        args: [--fix, --exit-non-zero-on-fix]
+    -   id: ruff-format
 
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.13.0
+    rev: v1.16.1
     hooks:
     -   id: mypy
-        additional_dependencies: [types-requests, types-setuptools, pydantic]
+        additional_dependencies: [types-setuptools, types-requests, pydantic]
 
 -   repo: https://github.com/executablebooks/mdformat
-    rev: 0.7.19
+    rev: 0.7.22
     hooks:
     -   id: mdformat
         additional_dependencies: [mdformat-gfm, mdformat-frontmatter, mdformat-pyproject]

README.md

@@ -33,6 +33,21 @@ You can install the latest release via [`pip`](https://pypi.org/project/pip/):
 $ pip install ape-safe
 ```
 
+You can also install from source:
+
+```shell
+# Clone the source code.
+git clone git@github.com:ApeWorX/ape-safe.git
+cd ape-safe
+
+# Build the Safe manifests.
+ape pm compile
+ape run build
+
+# Install.
+pip install .
+```
+
 ### via `setuptools`
 
 You can clone the repository and use [`setuptools`](https://github.com/pypa/setuptools) for the most up-to-date version:

ape_safe/_cli/click_ext.py

@@ -135,15 +135,18 @@ def _get_execute_callback(cls, ctx, param, val, name: str = "execute"):
             return False
 
         raise BadOptionUsage(
-            f"--{name}", f"`--{name}` value '{val}` not a boolean or account identifier."
+            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
+    "--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

@@ -162,7 +162,9 @@ def propose(cli_ctx, ecosystem, safe, data, gas_price, value, receiver, nonce, s
     while new_tx is None and timeout > 0:
         new_tx = next(
             safe.client.get_transactions(
-                starting_nonce=safe.next_nonce, confirmed=False, filter_by_ids=[safe_tx_hash]
+                starting_nonce=safe.next_nonce,
+                confirmed=False,
+                filter_by_ids=[safe_tx_hash],
             ),
             None,
         )
@@ -290,7 +292,7 @@ def reject(cli_ctx: SafeCliContext, safe, txn_ids, execute):
     """
     from ape.api import AccountAPI
 
-    submit = False if execute in (False, None) else True
+    submit = execute not in (False, None)
     submitter = execute if isinstance(execute, AccountAPI) else None
     if submitter is None and submit:
         submitter = safe.select_signer(for_="submitter")
@@ -315,7 +317,11 @@ def reject(cli_ctx: SafeCliContext, safe, txn_ids, execute):
         elif click.confirm(f"{txn}\nCancel Transaction?"):
             try:
                 safe.transfer(
-                    safe, 0, nonce=txn.nonce, submit_transaction=submit, submitter=submitter
+                    safe,
+                    0,
+                    nonce=txn.nonce,
+                    submit_transaction=submit,
+                    submitter=submitter,
                 )
             except SignatureError:
                 # These are expected because of how the plugin works

ape_safe/accounts.py

@@ -21,7 +21,14 @@
 from ethpm_types.abi import ABIType, MethodABI
 from packaging.version import Version
 
-from .client import BaseSafeClient, MockSafeClient, SafeClient, SafeTx, SafeTxConfirmation, SafeTxID
+from .client import (
+    BaseSafeClient,
+    MockSafeClient,
+    SafeClient,
+    SafeTx,
+    SafeTxConfirmation,
+    SafeTxID,
+)
 from .config import SafeConfig
 from .exceptions import (
     ApeSafeError,
@@ -494,9 +501,10 @@ def propose(
 
     def pending_transactions(self) -> Iterator[tuple[SafeTx, list[SafeTxConfirmation]]]:
         for executed_tx in self.client.get_transactions(confirmed=False):
-            yield self.create_safe_tx(
-                **executed_tx.model_dump(mode="json", by_alias=True)
-            ), executed_tx.confirmations
+            yield (
+                self.create_safe_tx(**executed_tx.model_dump(mode="json", by_alias=True)),
+                executed_tx.confirmations,
+            )
 
     @property
     def local_signers(self) -> list[AccountAPI]:
@@ -849,13 +857,13 @@ def skip_signer(signer: AccountAPI):
                 sigs_by_signer,
                 **gas_args,
                 nonce=submitter_account.nonce,
+                # NOTE: Because of `ape_ethereum.transactions.BaseTransaction.serialize_transaction`
+                #       doing a recovered signer check, and we have to make sure the address matches
+                #       the recovered address of the signed transaction.
+                sender=submitter_account.address,
             )
-            txn = submitter_account.sign_transaction(exec_transaction, **signer_options)
-            # NOTE: Because of `ape_ethereum.transactions.BaseTransaction.serialize_transaction`
-            #       doing a recovered signer check, and we gotta make sure the address matches
-            #       the recovered address of the signed transaction.
-            txn.sender = submitter_account.address
-            return txn
+            # NOTE: If `submitter` does not sign, this returns `None` which raises downstream
+            return submitter_account.sign_transaction(exec_transaction, **signer_options)
 
         elif submit:
             # NOTE: User wanted to submit transaction, but we can't, so don't publish to API

ape_safe/client/__init__.py

@@ -160,7 +160,10 @@ def post_transaction(
                 b"",
             )
         )
-        post_dict: dict = {"signature": signature.hex() if signature else None, "origin": ORIGIN}
+        post_dict: dict = {
+            "signature": signature.hex() if signature else None,
+            "origin": ORIGIN,
+        }
 
         for key, value in tx_data.model_dump(by_alias=True, mode="json").items():
             if isinstance(value, HexBytes):

ape_safe/client/mock.py

@@ -30,7 +30,7 @@ def __init__(self, contract: "ContractInstance"):
         self.contract = contract
         self.transactions: dict[SafeTxID, SafeApiTxData] = {}
         self.transactions_by_nonce: dict[int, list[SafeTxID]] = {}
-        self.delegates: dict["AddressType", list["AddressType"]] = {}
+        self.delegates: dict[AddressType, list[AddressType]] = {}
 
     @property
     def safe_details(self) -> SafeDetails:
@@ -90,7 +90,10 @@ def get_confirmations(self, safe_tx_hash: SafeTxID) -> Iterator[SafeTxConfirmati
             yield from safe_tx_data.confirmations
 
     def post_transaction(
-        self, safe_tx: SafeTx, signatures: dict["AddressType", "MessageSignature"], **kwargs
+        self,
+        safe_tx: SafeTx,
+        signatures: dict["AddressType", "MessageSignature"],
+        **kwargs,
     ):
         owners = self.safe_details.owners
 

ape_safe/factory.py

@@ -1,12 +1,17 @@
 import secrets
-from functools import cache
-from typing import TYPE_CHECKING, Iterable, Optional, Union
+from collections.abc import Iterable
+from typing import TYPE_CHECKING, Optional, Union
 
 from ape.types import AddressType
 from ape.utils import ZERO_ADDRESS, ManagerAccessMixin
 from packaging.version import Version
 
-from ape_safe.packages import MANIFESTS_BY_VERSION, PackageType, get_factory, get_singleton
+from ape_safe.packages import (
+    MANIFESTS_BY_VERSION,
+    PackageType,
+    get_factory,
+    get_singleton,
+)
 
 if TYPE_CHECKING:
     from ape.api import AccountAPI, BaseAddress
@@ -22,7 +27,6 @@ def inject(cls, version: Version, deployer: "AccountAPI"):
         cls._singleton[version] = deployer.deploy(PackageType.SINGLETON(version))
         cls._factory[version] = deployer.deploy(PackageType.PROXY_FACTORY(version))
 
-    @cache
     def get_factory(self, version: Version) -> "ContractInstance":
         if injected_factory := self._factory.get(version):
             return injected_factory
@@ -33,7 +37,6 @@ def get_factory(self, version: Version) -> "ContractInstance":
     def contract(self) -> "ContractInstance":
         return self.get_factory(max(MANIFESTS_BY_VERSION))
 
-    @cache
     def get_singleton(self, version: Version) -> "ContractInstance":
         if injected_singleton := self._singleton.get(version):
             return injected_singleton

ape_safe/multisend.py

@@ -52,17 +52,18 @@ class MultiSend(ManagerAccessMixin):
     def __init__(
         self,
         safe: Optional[SafeAccount] = None,
-        version: Union[Version, str] = max(MANIFESTS_BY_VERSION),
+        version: Union[Version, str, None] = None,
     ) -> None:
         """
         Initialize a new MultiSend session object. By default, there are no calls to make.
         """
+        version = version or max(MANIFESTS_BY_VERSION)
         self.calls: list[dict] = []
         self.safe = safe
         self.version = version if isinstance(version, Version) else Version(version)
 
     @classmethod
-    def inject(cls, version: Union[Version, str] = max(MANIFESTS_BY_VERSION)):
+    def inject(cls, version: Union[Version, str, None] = None):
         """
         Create the multisend module contract on-chain, so we can use it.
         Must use a provider that supports ``debug_setCode``.
@@ -71,11 +72,13 @@ def inject(cls, version: Union[Version, str] = max(MANIFESTS_BY_VERSION)):
 
             from ape_safe.multisend import MultiSend
 
+
             @pytest.fixture(scope="session")
             def multisend():
                 MultiSend.inject()
                 return MultiSend()
         """
+        version = version or max(MANIFESTS_BY_VERSION)
         active_provider = cls.network_manager.active_provider
         assert active_provider, "Must be connected to an active network to deploy"
         MultiSend = PackageType.MULTISEND(version)