fix: use api to set confirmations on the tx after adding more (#38)

* fix: use api

* test: add approve tests

ape_safe/_cli/pending.py

@@ -191,7 +191,6 @@ def approve(cli_ctx: SafeCliContext, safe, txn_ids, execute):
 
         safe_tx = safe.create_safe_tx(**txn.model_dump(by_alias=True, mode="json"))
         num_confirmations = len(txn.confirmations)
-        signatures_added = {}
 
         if num_confirmations < safe.confirmations_required:
             signatures_added = safe.add_signatures(safe_tx, confirmations=txn.confirmations)
@@ -216,7 +215,8 @@ def approve(cli_ctx: SafeCliContext, safe, txn_ids, execute):
                 submitter = safe.select_signer(for_="submitter")
 
         if submitter:
-            txn.confirmations = {**txn.confirmations, **signatures_added}
+            safe_tx_hash = get_safe_tx_hash(safe_tx)
+            txn.confirmations = safe.client.get_confirmations(safe_tx_hash)
             _execute(safe, txn, submitter)
 
     # If any txn_ids remain, they were not handled.

ape_safe/client/mock.py

@@ -62,7 +62,9 @@ def _all_transactions(
         self,
     ) -> Iterator[SafeApiTxData]:
         for nonce in sorted(self.transactions_by_nonce.keys(), reverse=True):
-            yield from map(self.transactions.get, self.transactions_by_nonce[nonce])
+            for tx in map(self.transactions.get, self.transactions_by_nonce[nonce]):
+                if tx:
+                    yield tx
 
     def get_confirmations(self, safe_tx_hash: SafeTxID) -> Iterator[SafeTxConfirmation]:
         tx_hash = cast(SafeTxID, HexBytes(safe_tx_hash).hex())

tests/integration/test_pending_cli.py

@@ -1,3 +1,8 @@
+from datetime import datetime
+
+from ape_safe.client import ExecutedTxData
+
+
 def test_help(runner, cli):
     result = runner.invoke(cli, ["pending", "--help"], catch_exceptions=False)
     assert result.exit_code == 0, result.output
@@ -114,3 +119,59 @@ def test_list_no_txns(runner, cli, one_safe, chain):
     )
     assert result.exit_code == 0, result.output
     assert "There are no pending transactions" in result.output
+
+
+def test_approve_transaction_not_found(runner, cli, one_safe, chain):
+    tx_hash = "0x123"
+    result = runner.invoke(
+        cli,
+        ["pending", "approve", tx_hash, "--network", chain.provider.network_choice],
+        catch_exceptions=False,
+    )
+    assert result.exit_code != 0, result.output
+    assert f"Pending transaction(s) '{tx_hash}' not found." in result.output
+
+
+def test_approve(receiver, runner, cli, one_safe, chain):
+    # First, fund the safe so the tx does not fail.
+    receiver.transfer(one_safe, "1 ETH")
+    tx_hash = "0x123"
+    nonce = 1
+
+    one_safe.client.transactions_by_nonce[nonce] = tx_hash
+    one_safe.client.transactions[tx_hash] = ExecutedTxData(
+        executionDate=datetime.now(),
+        blockNumber=0,
+        transactionHash=tx_hash,
+        executor=receiver.address,
+        isExecuted=False,
+        isSuccessful=True,
+        ethGasPrice=0,
+        maxFeePerGas=1000,
+        maxPriorityFeePerGas=1000,
+        gasUsed=100,
+        fee=10,
+        origin="ape",
+        dataDecoded=None,
+        confirmationsRequired=0,
+        safeTxHash=tx_hash,
+        submissionDate=datetime.now(),
+        modified=datetime.now(),
+        nonce=nonce,
+        refundReceiver=receiver.address,
+        gasPrice=0,
+        baseGas=0,
+        safeTxGas=0,
+        gasToken=receiver.address,
+        operation=0,
+        value=0,
+        to=receiver.address,
+        safe=one_safe.address,
+    )
+
+    result = runner.invoke(
+        cli,
+        ["pending", "approve", tx_hash, "--network", chain.provider.network_choice],
+        catch_exceptions=False,
+    )
+    assert result.exit_code != 0, result.output