Fixes for box population and errors from a rejected app call transaction

.github/workflows/check-python.yaml

@@ -23,14 +23,16 @@ jobs:
 
       - name: Audit with pip-audit
         run: |
+          # GHSA-4xh5-x5gv-qwph is safe to ignore since we are using a python version that is not affected
+          #                     can remove once pip has fix
           # audit non dev dependencies, no exclusions
-          poetry export --without=dev > requirements.txt && poetry run pip-audit -r requirements.txt
+          poetry export --without=dev > requirements.txt && poetry run pip-audit -r requirements.txt --ignore-vuln GHSA-4xh5-x5gv-qwph 
 
           # audit all dependencies, with exclusions. 
           # If a vulnerability is found in a dev dependency without an available fix,
           # it can be temporarily ignored by adding --ignore-vuln e.g.
           # --ignore-vuln "GHSA-hcpj-qp55-gfph" # GitPython vulnerability, dev only dependency
-          poetry run pip-audit
+          poetry run pip-audit --ignore-vuln GHSA-4xh5-x5gv-qwph 
 
       - name: Check formatting with Ruff
         run: |

src/algokit_utils/errors/logic_error.py

@@ -20,7 +20,7 @@
 
 
 LOGIC_ERROR = (
-    ".*transaction (?P<transaction_id>[A-Z0-9]+): logic eval error: (?P<message>.*). Details: .*pc=(?P<pc>[0-9]+).*"
+    ".*transaction (?P<transaction_id>[A-Z0-9]+): (logic eval error: )?(?P<message>.*). Details: .*pc=(?P<pc>[0-9]+).*"
 )
 
 

src/algokit_utils/transactions/transaction_composer.py

@@ -21,7 +21,7 @@
 from algosdk.transaction import OnComplete, SuggestedParams
 from algosdk.v2client.algod import AlgodClient
 from algosdk.v2client.models.simulate_request import SimulateRequest
-from typing_extensions import deprecated
+from typing_extensions import Never, deprecated
 
 from algokit_utils.applications.abi import ABIReturn, ABIValue
 from algokit_utils.applications.app_manager import AppManager
@@ -667,7 +667,7 @@ def _encode_lease(lease: str | bytes | None) -> bytes | None:
         raise TypeError(f"Unknown lease type received of {type(lease)}")
 
 
-def _get_group_execution_info(  # noqa: C901, PLR0912
+def _get_group_execution_info(
     atc: AtomicTransactionComposer,
     algod: AlgodClient,
     populate_app_call_resources: bool | None = None,
@@ -682,6 +682,7 @@ def _get_group_execution_info(  # noqa: C901, PLR0912
         txn_groups=[],
         allow_unnamed_resources=True,
         allow_empty_signatures=True,
+        exec_trace_config=algosdk.v2client.models.SimulateTraceConfig(enable=True),
     )
 
     # Clone ATC with null signers
@@ -710,26 +711,14 @@ def _get_group_execution_info(  # noqa: C901, PLR0912
             f"Required for transactions: {', '.join(str(i) for i in app_call_indexes_without_max_fees)}"
         )
 
-    # Get fee parameters
-    per_byte_txn_fee = suggested_params.fee if suggested_params else 0
-    min_txn_fee = int(suggested_params.min_fee) if suggested_params else 1000  # type: ignore[unused-ignore]
-
     # Simulate transactions
     result = empty_signer_atc.simulate(algod, simulate_request)
 
     group_response = result.simulate_response["txn-groups"][0]
 
     if group_response.get("failure-message"):
-        msg = group_response["failure-message"]
-        if cover_app_call_inner_transaction_fees and "fee too small" in msg:
-            raise ValueError(
-                "Fees were too small to resolve execution info via simulate. "
-                "You may need to increase an app call transaction maxFee."
-            )
-        failed_at = group_response.get("failed-at", [0])[0]
-        raise ValueError(
-            f"Error resolving execution info via simulate in transaction {failed_at}: "
-            f"{group_response['failure-message']}"
+        _handle_simulation_error(
+            group_response, cover_app_call_inner_transaction_fees=cover_app_call_inner_transaction_fees
         )
 
     # Build execution info
@@ -743,27 +732,12 @@ def _get_group_execution_info(  # noqa: C901, PLR0912
 
         required_fee_delta = 0
         if cover_app_call_inner_transaction_fees:
-            # Calculate parent transaction fee
-            parent_per_byte_fee = per_byte_txn_fee * (original_txn.estimate_size() + 75)
-            parent_min_fee = max(parent_per_byte_fee, min_txn_fee)
-            parent_fee_delta = parent_min_fee - original_txn.fee
-
-            if isinstance(original_txn, algosdk.transaction.ApplicationCallTxn):
-                # Calculate inner transaction fees recursively
-                def calculate_inner_fee_delta(inner_txns: list[dict], acc: int = 0) -> int:
-                    for inner_txn in reversed(inner_txns):
-                        current_fee_delta = (
-                            calculate_inner_fee_delta(inner_txn["inner-txns"], acc)
-                            if inner_txn.get("inner-txns")
-                            else acc
-                        ) + (min_txn_fee - inner_txn["txn"]["txn"].get("fee", 0))
-                        acc = max(0, current_fee_delta)
-                    return acc
-
-                inner_fee_delta = calculate_inner_fee_delta(txn_result.get("inner-txns", []))
-                required_fee_delta = inner_fee_delta + parent_fee_delta
-            else:
-                required_fee_delta = parent_fee_delta
+            required_fee_delta = _calculate_required_fee_delta(
+                original_txn,
+                txn_result,
+                per_byte_txn_fee=suggested_params.fee if suggested_params else 0,
+                min_txn_fee=int(suggested_params.min_fee) if suggested_params else 1000,
+            )
 
         txn_results.append(
             ExecutionInfoTxn(
@@ -782,6 +756,64 @@ def calculate_inner_fee_delta(inner_txns: list[dict], acc: int = 0) -> int:
     )
 
 
+def _handle_simulation_error(
+    group_response: dict[str, Any], *, cover_app_call_inner_transaction_fees: bool | None
+) -> Never:
+    msg = group_response["failure-message"]
+    if cover_app_call_inner_transaction_fees and "fee too small" in msg:
+        raise ValueError(
+            "Fees were too small to resolve execution info via simulate. "
+            "You may need to increase an app call transaction maxFee."
+        )
+    failed_at = group_response.get("failed-at", [0])[0]
+    details = ""
+    if "logic eval error" not in msg:
+        # extract last pc from trace so we can format an error that can be parsed into a LogicError
+        try:
+            trace = group_response["txn-results"][failed_at]["exec-trace"]
+        except (KeyError, IndexError):
+            pass
+        else:
+            try:
+                program_trace = trace["approval-program-trace"]
+            except KeyError:
+                program_trace = trace["clear-program-trace"]
+            pc = program_trace[-1]["pc"]
+            details = f". Details: pc={pc}"
+    raise ValueError(
+        f"Error resolving execution info via simulate in transaction {failed_at}: "
+        f"{group_response['failure-message']}{details}"
+    )
+
+
+def _calculate_required_fee_delta(
+    txn: transaction.Transaction, txn_result: dict[str, Any], *, per_byte_txn_fee: int, min_txn_fee: int
+) -> int:
+    # Calculate parent transaction fee
+    original_txn_size = txn.estimate_size()
+    assert isinstance(original_txn_size, int), "expected txn size to be an int"
+    parent_per_byte_fee = per_byte_txn_fee * (original_txn_size + 75)
+    parent_min_fee = max(parent_per_byte_fee, min_txn_fee)
+    original_txn_fee = txn.fee
+    assert isinstance(original_txn_fee, int), "expected original txn fee to be an int"
+    parent_fee_delta = parent_min_fee - original_txn_fee
+
+    if isinstance(txn, algosdk.transaction.ApplicationCallTxn):
+        # Calculate inner transaction fees recursively
+        def calculate_inner_fee_delta(inner_txns: list[dict], acc: int = 0) -> int:
+            for inner_txn in reversed(inner_txns):
+                current_fee_delta = (
+                    calculate_inner_fee_delta(inner_txn["inner-txns"], acc) if inner_txn.get("inner-txns") else acc
+                ) + (min_txn_fee - inner_txn["txn"]["txn"].get("fee", 0))
+                acc = max(0, current_fee_delta)
+            return acc
+
+        inner_fee_delta = calculate_inner_fee_delta(txn_result.get("inner-txns", []))
+        return inner_fee_delta + parent_fee_delta
+    else:
+        return parent_fee_delta
+
+
 def _find_available_transaction_index(
     txns: list[TransactionWithSigner], reference_type: str, reference: str | dict[str, Any] | int
 ) -> int:
@@ -1041,13 +1073,15 @@ def is_appl_below_limit(t: TransactionWithSigner) -> bool:
             foreign_apps.append(app_id)
             app_txn.foreign_apps = foreign_apps
         elif ref_type == "box":
+            # ensure app_id is added before calling translate_box_reference
+            app_id = box_ref[0]
+            if app_id != 0:
+                foreign_apps = list(getattr(app_txn, "foreign_apps", []) or [])
+                foreign_apps.append(app_id)
+                app_txn.foreign_apps = foreign_apps
             boxes = list(getattr(app_txn, "boxes", []) or [])
             boxes.append(BoxReference.translate_box_reference(box_ref, app_txn.foreign_apps or [], app_txn.index))  # type: ignore[arg-type]
             app_txn.boxes = boxes
-            if box_ref[0] != 0:
-                foreign_apps = list(getattr(app_txn, "foreign_apps", []) or [])
-                foreign_apps.append(box_ref[0])
-                app_txn.foreign_apps = foreign_apps
         elif ref_type == "asset":
             asset_id = int(cast(str | int, reference))
             foreign_assets = list(getattr(app_txn, "foreign_assets", []) or [])

tests/applications/test_app_client.py

@@ -8,6 +8,7 @@
 import pytest
 from algosdk.atomic_transaction_composer import TransactionSigner, TransactionWithSigner
 
+from algokit_utils import SendParams
 from algokit_utils._legacy_v2.application_specification import ApplicationSpecification
 from algokit_utils.algorand import AlgorandClient
 from algokit_utils.applications.abi import ABIType
@@ -166,45 +167,16 @@ def testing_app_puya_arc32_app_spec() -> ApplicationSpecification:
     return ApplicationSpecification.from_json(raw_json_spec.read_text())
 
 
-@pytest.fixture
-def testing_app_puya_arc32_app_id(
-    algorand: AlgorandClient, funded_account: SigningAccount, testing_app_puya_arc32_app_spec: ApplicationSpecification
-) -> int:
-    global_schema = testing_app_puya_arc32_app_spec.global_state_schema
-    local_schema = testing_app_puya_arc32_app_spec.local_state_schema
-
-    response = algorand.send.app_create(
-        AppCreateParams(
-            sender=funded_account.address,
-            approval_program=testing_app_puya_arc32_app_spec.approval_program,
-            clear_state_program=testing_app_puya_arc32_app_spec.clear_program,
-            schema={
-                "global_byte_slices": int(global_schema.num_byte_slices) if global_schema.num_byte_slices else 0,
-                "global_ints": int(global_schema.num_uints) if global_schema.num_uints else 0,
-                "local_byte_slices": int(local_schema.num_byte_slices) if local_schema.num_byte_slices else 0,
-                "local_ints": int(local_schema.num_uints) if local_schema.num_uints else 0,
-            },
-        )
-    )
-    return response.app_id
-
-
 @pytest.fixture
 def test_app_client_puya(
-    algorand: AlgorandClient,
-    funded_account: SigningAccount,
-    testing_app_puya_arc32_app_spec: ApplicationSpecification,
-    testing_app_puya_arc32_app_id: int,
+    algorand: AlgorandClient, funded_account: SigningAccount, testing_app_puya_arc32_app_spec: ApplicationSpecification
 ) -> AppClient:
-    return AppClient(
-        AppClientParams(
-            default_sender=funded_account.address,
-            default_signer=funded_account.signer,
-            app_id=testing_app_puya_arc32_app_id,
-            algorand=algorand,
-            app_spec=testing_app_puya_arc32_app_spec,
-        )
+    factory = algorand.client.get_app_factory(
+        app_spec=testing_app_puya_arc32_app_spec,
+        default_sender=funded_account.address,
     )
+    app_client, _ = factory.send.bare.create()
+    return app_client
 
 
 def test_clone_overriding_default_sender_and_inheriting_app_name(
@@ -709,6 +681,29 @@ def test_box_methods_with_arc4_returns_parametrized(
         assert abi_decoded_boxes[0].value == arg_value
 
 
+@pytest.mark.parametrize(
+    "populate",
+    [
+        True,
+        # False, # enable this test once rejected transactions contain pc information
+    ],
+)
+def test_txn_with_reject(test_app_client_puya: AppClient, *, populate: bool) -> None:
+    with pytest.raises(LogicError, match="expect this txn to be rejected"):
+        test_app_client_puya.send.call(
+            AppClientMethodCallParams(method="rejected"), send_params=SendParams(populate_app_call_resources=populate)
+        )
+
+
+@pytest.mark.parametrize("populate", [True, False])
+def test_txn_with_logic_err(test_app_client_puya: AppClient, *, populate: bool) -> None:
+    with pytest.raises(LogicError, match="expect this to be a logic err") as exc:
+        test_app_client_puya.send.call(
+            AppClientMethodCallParams(method="logic_err"), send_params=SendParams(populate_app_call_resources=populate)
+        )
+    assert exc
+
+
 def test_abi_with_default_arg_method(
     algorand: AlgorandClient,
     funded_account: SigningAccount,