From eddc804f771b791d3891d3cfe6245edc98f90cf8 Mon Sep 17 00:00:00 2001
From: antazoey <admin@antazoey.me>
Date: Tue, 5 Mar 2024 14:24:47 -0700
Subject: [PATCH] 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              |  4 +-
 ape_safe/client/mock.py               |  4 +-
 tests/integration/test_pending_cli.py | 61 +++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/ape_safe/_cli/pending.py b/ape_safe/_cli/pending.py
index 83f364d..fe89f5e 100644
--- a/ape_safe/_cli/pending.py
+++ b/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.
diff --git a/ape_safe/client/mock.py b/ape_safe/client/mock.py
index d1f5e32..a054cb8 100644
--- a/ape_safe/client/mock.py
+++ b/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())
diff --git a/tests/integration/test_pending_cli.py b/tests/integration/test_pending_cli.py
index ba6665e..f0183f5 100644
--- a/tests/integration/test_pending_cli.py
+++ b/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