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