refactor!: new flow [APE-1140]

ape_safe/_cli.py

@@ -98,22 +98,34 @@ def remove(cli_ctx, alias):
     cls=NetworkBoundCommand, short_help="See pending transactions for a locally-tracked Safe"
 )
 @network_option()
+@click.option("sign_with_local_signers", "--sign", is_flag=True)
+@click.option("--execute", is_flag=False, flag_value=True, default=False)
 @existing_alias_argument(account_type=SafeAccount)
-def pending(network, alias):
+def pending(network, sign_with_local_signers, execute, alias):
     safe = accounts.load(alias)
-    local_signers = set(signer for signer in accounts if signer.address in safe.signers)
-    if local_signers:
-        click.echo("Local Signer(s) detected!")
-        sign_with_local_signers = click.confirm("Do you want to sign unconfirmed transactions")
 
-    else:
-        sign_with_local_signers = False
+    if isinstance(execute, str):
+        if execute in accounts.aliases:
+            submitter = accounts.load(execute)
+        else:
+            raise
+
+    elif execute is True:
+        submitter = safe.local_signers[0]
 
-    for txn in safe.client.get_transactions(
-        starting_nonce=safe.next_nonce,
-        filter_by_missing_signers=local_signers if sign_with_local_signers else None,
-    ):
-        click.echo(f"Txn {txn.nonce}: ({len(txn.confirmations)}/{txn.confirmationsRequired})")
+    for safe_tx, confirmations in safe.pending_transactions():
+        if sign_with_local_signers and len(confirmations) < safe.confirmations_required:
+            pass  # TODO: sign `safe_tx` with local signers not in `confirmations`
+
+        else:
+            click.echo(f"Txn {safe_tx.nonce}: ({len(confirmations)}/{safe.confirmations_required})")
+
+        if execute is not False:
+            signatures = safe.get_confirmations(safe_tx)
+            if len(signatures) >= safe.confirmations_required and click.confirm(
+                f"Submit Txn {safe_tx.nonce}"
+            ):
+                submitter.call(safe.create_execute_transaction(safe_tx, signatures))
 
 
 @cli.command(cls=NetworkBoundCommand, short_help="Reject one or more pending transactions")

ape_safe/accounts.py

@@ -11,11 +11,19 @@
 from ape.utils import ZERO_ADDRESS, cached_property
 from ape_ethereum.transactions import TransactionType
 from eip712.common import create_safe_tx_def
+from eip712.messages import hash_eip712_message
 from eth_utils import keccak, to_bytes, to_int
 from ethpm_types import ContractType
 
-from .client import SafeClient, SafeTx
-from .exceptions import NoLocalSigners, NotASigner, NotEnoughSignatures, handle_safe_logic_error
+from .client import BaseSafeClient, MockSafeClient, SafeClient, SafeTx, SafeTxConfirmation
+from .exceptions import (
+    NoLocalSigners,
+    NotASigner,
+    NotEnoughSignatures,
+    SafeClientException,
+    handle_safe_logic_error,
+)
+from .utils import order_by_signer
 
 
 class AccountContainer(AccountContainerAPI):
@@ -101,10 +109,13 @@ def fallback_handler(self) -> Optional[ContractInstance]:
             return None
 
     @cached_property
-    def client(self) -> SafeClient:
+    def client(self) -> BaseSafeClient:
         if self.provider.chain_id not in self.account_file["deployed_chain_ids"]:
             raise  # Not valid on this chain
 
+        if self.provider.network.name == "local":
+            return MockSafeClient(contract=self.contract)
+
         return SafeClient(address=self.address, chain_id=self.provider.chain_id)
 
     @property
@@ -171,6 +182,10 @@ def create_safe_tx(self, txn: Optional[TransactionAPI] = None, **safe_tx_kwargs)
 
         return self.safe_tx_def(**safe_tx)
 
+    def pending_transactions(self) -> Iterator[Tuple[SafeTx, List[SafeTxConfirmation]]]:
+        for unexec_tx_data in self.client.get_transactions(confirmed=False):
+            yield self.create_safe_tx(**unexec_tx_data.dict()), unexec_tx_data.confirmations
+
     @property
     def local_signers(self) -> List[AccountAPI]:
         # NOTE: Is not ordered by signing order
@@ -199,7 +214,9 @@ def create_execute_transaction(
         **txn_options,
     ) -> TransactionAPI:
         exec_args = list(safe_tx._body_["message"].values())[:-1]  # NOTE: Skip `nonce`
-        encoded_signatures = self._encode_signatures(signatures)
+        encoded_signatures = HexBytes(
+            b"".join(sig.encode_rsv() for sig in order_by_signer(signatures))
+        )
 
         # NOTE: executes a `ProviderAPI.prepare_transaction`, which may produce `ContractLogicError`
         return self.contract.execTransaction.as_transaction(
@@ -223,21 +240,32 @@ def compute_prev_signer(self, signer: Union[str, AddressType, BaseAddress]) -> A
         # NOTE: SENTINEL_OWNERS is the "previous" address to index 0
         return AddressType("0x0000000000000000000000000000000000000001")  # type: ignore[arg-type]
 
+    def load_submitter(
+        self,
+        submitter: Union[AddressType, str, None] = None,
+    ) -> AccountAPI:
+        if submitter is None:
+            if len(self.local_signers) == 0:
+                raise NoLocalSigners()
+
+            return self.local_signers[0]
+
+        elif (
+            submitter_address := self.conversion_manager.convert(submitter, AddressType)
+            in self.account_manager
+        ):
+            return self.account_manager[submitter_address]
+
+        elif isinstance(submitter, str) and submitter in self.account_manager.aliases:
+            return self.account_manager.load(submitter)
+
+        else:
+            raise  # Cannot handle `submitter=type(submitter)`
+
     def prepare_transaction(self, txn: TransactionAPI) -> TransactionAPI:
         # NOTE: Need to override `AccountAPI` behavior for balance checks
         return self.provider.prepare_transaction(txn)
 
-    def _encode_signatures(self, signatures: Dict[AddressType, MessageSignature]) -> HexBytes:
-        # NOTE: Must order signatures in ascending order of signer address (converted to int)
-        def addr_to_int(a: AddressType) -> int:
-            return to_int(hexstr=a)
-
-        return HexBytes(
-            b"".join(
-                signatures[signer].encode_rsv() for signer in sorted(signatures, key=addr_to_int)
-            )
-        )
-
     def _safe_tx_exec_args(self, safe_tx: SafeTx) -> List:
         return list(safe_tx._body_["message"].values())
 
@@ -281,7 +309,7 @@ def _impersonated_call(self, txn: TransactionAPI, **safe_tx_and_call_kwargs) ->
         )
         return self.contract.execTransaction(
             *safe_tx_exec_args[:-1],  # NOTE: Skip nonce
-            self._encode_signatures(signatures),
+            HexBytes(b"".join(sig.encode_rsv() for sig in order_by_signer(signatures))),
             **safe_tx_and_call_kwargs,
         )
 
@@ -297,6 +325,20 @@ def call(  # type: ignore[override]
 
         return super().call(txn, **call_kwargs)
 
+    def get_api_confirmations(self, safe_tx: SafeTx) -> Dict[AddressType, MessageSignature]:
+        safe_tx_hash = hash_eip712_message(safe_tx)
+        try:
+            client_confs = self.client.get_confirmations(safe_tx_hash)
+        except SafeClientException:
+            return {}
+
+        return {
+            conf.owner: MessageSignature(
+                r=conf.signature[:32], s=conf.signature[32:64], v=conf.signature[64]
+            )
+            for conf in client_confs
+        }
+
     def _contract_approvals(self, safe_tx: SafeTx) -> Dict[AddressType, MessageSignature]:
         safe_tx_exec_args = self._safe_tx_exec_args(safe_tx)
         safe_tx_hash = self.contract.getTransactionHash(*safe_tx_exec_args)
@@ -308,8 +350,24 @@ def _contract_approvals(self, safe_tx: SafeTx) -> Dict[AddressType, MessageSigna
         }
 
     def _all_approvals(self, safe_tx: SafeTx) -> Dict[AddressType, MessageSignature]:
-        # TODO: Combine with approvals from SafeAPI
-        return self._contract_approvals(safe_tx)
+        approvals = self.get_api_confirmations(safe_tx)
+        # NOTE: Do this last because it should take precedence
+        approvals.update(self._contract_approvals(safe_tx))
+        return approvals
+
+    def submit_safe_tx(
+        self,
+        safe_tx: SafeTx,
+        submitter: Union[AccountAPI, AddressType, str, None] = None,
+        **txn_options,
+    ) -> ReceiptAPI:
+        signatures = self._all_approvals(safe_tx)
+        txn = self.create_execute_transaction(safe_tx, signatures, **txn_options)
+
+        if not isinstance(submitter, AccountAPI):
+            submitter = self.load_submitter(submitter)
+
+        return submitter.call(txn)
 
     def sign_transaction(
         self,
@@ -327,25 +385,11 @@ def sign_transaction(
         if not submit and submitter:
             raise  # Cannot specify a submitter if not submitting
 
-        elif submit and not submitter:
-            if len(self.local_signers) == 0:
-                raise NoLocalSigners()
-
-            submitter = self.local_signers[0]
-            logger.info(f"No submitter specified, so using: {submitter}")
-
-        # NOTE: Works whether `submit` is set or not below here
-        elif (
-            submitter_address := self.conversion_manager.convert(submitter, AddressType)
-            in self.account_manager
-        ):
-            submitter = self.account_manager[submitter_address]
-
-        elif isinstance(submitter, str) and submitter in self.account_manager.aliases:
-            submitter = self.account_manager.load(submitter)
-
         elif not isinstance(submitter, AccountAPI):
-            raise  # Cannot handle `submitter=type(submitter)`
+            submitter_not_specified = submitter is None
+            submitter = self.load_submitter(submitter)
+            if submitter_not_specified:
+                logger.info(f"No submitter specified, so using: {submitter}")
 
         # Invariant: `submitter` should be either `AccountAPI` or we are not submitting here
         assert isinstance(submitter, AccountAPI) or not submit
@@ -397,8 +441,7 @@ def skip_signer(signer: AccountAPI):
             and len(sigs_by_signer) >= signatures_required
         ):
             # We need to encode the submitter's address for Safe to decode
-            # NOTE: Should only be triggered if the `submitter` is also a signer
-            if len(sigs_by_signer) < self.confirmations_required:
+            if submitter.address in self.signers:
                 sigs_by_signer[submitter.address] = self._preapproved_signature(submitter)
 
             # Inherit gas args from safe_tx, if set
@@ -434,5 +477,7 @@ def skip_signer(signer: AccountAPI):
             f"Collected {len(sigs_by_signer)}/{self.confirmations_required} signatures "
             f"for Safe {self.address}#{safe_tx.nonce}"  # TODO: put URI
         )
-        # TODO: Submit safe_tx and sigs to Safe API
+
+        # NOTE: Signatures don't have to be in order for Safe API post
+        self.client.post_transaction(safe_tx, sigs_by_signer)
         return None