fix: readonly simulate extra opcode budget handling (#180)

* fix: readonly simulate extra opcode budget handling

The simulate for readonly calls now uses a fixed maximum opcode budget.
Updates tests to reflect the new behavior.

* chore: remove max fee enforcement for readonly calls

* fix: adjust test and clarify some docs

* chore: bump urllib (pip audit)

---------

Co-authored-by: Neil Campbell <[email protected]>

Author: aorumbayev

poetry.lock

@@ -95,6 +95,9 @@
 # TEAL opcodes for constant blocks
 BYTE_CBLOCK = 38  # bytecblock opcode
 INT_CBLOCK = 32  # intcblock opcode
+MAX_SIMULATE_OPCODE_BUDGET = (
+    20_000 * 16
+)  # https://github.com/algorand/go-algorand/blob/807b29a91c371d225e12b9287c5d56e9b33c4e4c/ledger/simulation/trace.go#L104
 
 T = TypeVar("T")  # For generic return type in _handle_call_errors
 
@@ -1183,7 +1186,6 @@ def call(
         :param params: Parameters for the application call including method and transaction options
         :param send_params: Send parameters
         :return: The result of sending or simulating the transaction, including ABI return value if applicable
-        :raises ValueError: If the transaction is read-only and `max_fee` is not provided
         """
         is_read_only_call = (
             params.on_complete == algosdk.transaction.OnComplete.NoOpOC or params.on_complete is None
@@ -1194,14 +1196,10 @@ def call(
             readonly_send_params = send_params or SendParams()
 
             # Read-only calls do not require fees to be paid, as they are only simulated on the network.
-            # Therefore there is no value in calculating the minimum fee needed for a successful app call with inners.
-            # As a a result we only need to send a single simulate call,
-            # however to do this successfully we need to ensure fees for the transaction are fully covered using maxFee.
-            if readonly_send_params.get("cover_app_call_inner_transaction_fees"):
-                if params.max_fee is None:
-                    raise ValueError(
-                        "Please provide a `max_fee` for the transaction when `cover_app_call_inner_transaction_fees` is enabled."  # noqa: E501
-                    )
+            # With maximum opcode budget provided, ensure_budget won't create inner transactions,
+            # so fee coverage is no longer a concern for read-only calls.
+            # If max_fee is provided, use it as static_fee for potential benefits.
+            if readonly_send_params.get("cover_app_call_inner_transaction_fees") and params.max_fee is not None:
                 readonly_params = replace(readonly_params, static_fee=params.max_fee, extra_fee=None)
 
             method_call_to_simulate = self._algorand.new_group().add_app_call_method_call(
@@ -1215,11 +1213,13 @@ def run_simulate() -> SendAtomicTransactionComposerResults:
                         skip_signatures=True,
                         allow_more_logs=True,
                         allow_empty_signatures=True,
-                        extra_opcode_budget=None,
+                        extra_opcode_budget=MAX_SIMULATE_OPCODE_BUDGET,
                         exec_trace_config=None,
                         simulation_round=None,
                     )
                 except Exception as e:
+                    # For read-only calls with max opcode budget, fee issues should be rare
+                    # but we can still provide helpful error message if they occur
                     if readonly_send_params.get("cover_app_call_inner_transaction_fees") and "fee too small" in str(e):
                         raise ValueError(
                             "Fees were too small. You may need to increase the transaction `maxFee`."

src/algokit_utils/applications/app_client.py

@@ -623,64 +623,67 @@ def test_handles_expensive_abi_calls_with_ensure_budget(self) -> None:
         assert len(result.confirmation.get("inner-txns", [])) == 9  # type: ignore[union-attr]
         self._assert_min_fee(self.app_client1, params, expected_fee)
 
-    def test_readonly_handles_expensive_abi_calls_with_ensure_budget(self) -> None:
-        """Test fee handling with expensive readonly ABI method calls that use ensure_budget to op-up"""
+    @pytest.mark.parametrize("cover_inner_fees", [True, False])
+    def test_readonly_uses_fixed_opcode_budget_without_op_up_inner_transactions(self, cover_inner_fees: bool) -> None:  # noqa: FBT001
+        """Test that readonly calls use fixed opcode budget and don't require inner transactions for op-ups
+        regardless of fee coverage setting"""
 
-        expected_fee = 12_000
         params = AppClientMethodCallParams(
             method="burn_ops_readonly",
-            args=[6200],
-            max_fee=AlgoAmount.from_micro_algo(expected_fee),
+            args=[6200],  # This would normally require op-ups via inner transactions
         )
-        result = self.app_client1.send.call(params, send_params={"cover_app_call_inner_transaction_fees": True})
-
-        assert result.transaction.raw.fee == expected_fee
-        assert len(result.confirmation.get("inner-txns", [])) == 9  # type: ignore[union-attr]
-
-    def test_readonly_throws_when_no_max_fee(self) -> None:
-        """Test that error is thrown when no max fee is supplied for a readonly method call"""
-        with pytest.raises(
-            ValueError,
-            match="Please provide a `max_fee` for the transaction when `cover_app_call_inner_transaction_fees` is enabled",  # noqa: E501
-        ):
-            self.app_client1.send.call(
-                AppClientMethodCallParams(
-                    method="burn_ops_readonly",
-                    args=[6200],
-                ),
-                send_params={
-                    "cover_app_call_inner_transaction_fees": True,
-                },
-            )
-
-    def test_readonly_throws_when_inner_fees_not_covered(self) -> None:
-        """Test that error is thrown when a readonly method call inner transaction fees are not covered"""
-
-        expected_fee = 7000
-        params = AppClientMethodCallParams(
-            method="burn_ops_readonly",
-            args=[6200],
-            max_fee=AlgoAmount.from_micro_algo(expected_fee),
+        result = self.app_client1.send.call(
+            params, send_params={"cover_app_call_inner_transaction_fees": cover_inner_fees}
+        )
+
+        # No op-up inner transactions needed regardless of fee coverage setting
+        assert len(result.confirmation.get("inner-txns", [])) == 0  # type: ignore[union-attr]
+        assert result.transaction.raw.fee == 1_000
+        assert len(result.tx_ids) == 1
+
+    def test_readonly_alters_fee_handling_inner_transactions(self) -> None:
+        """Test that inner transaction can be covered using the max_fee"""
+        # Force `send_inners_with_fees` to be marked as readonly
+        for method in self.app_client1._app_spec.methods:  # noqa: SLF001
+            if method.name == "send_inners_with_fees":
+                method.readonly = True
+                break
+
+        # The expected_fee differs to non readonly method call,as we don't want to
+        # run simulate twice (once for resolving the minimum fee and once for the actual transaction result).
+        # Because no fees are actually paid with readonly calls,
+        # we simply use the maxFee value (if set) and skip any minimum fee calculations.
+        # If this method is running in a non readonly context, the minimum fee would be calculated as 5300n.
+        expected_fee = 12_000
+        result = self.app_client1.send.call(
+            AppClientMethodCallParams(
+                method="send_inners_with_fees",
+                args=[self.app_client2.app_id, self.app_client3.app_id, [1000, 0, 200, 0, [500, 0]]],
+                max_fee=AlgoAmount.from_micro_algo(expected_fee),
+            ),
+            send_params={
+                "cover_app_call_inner_transaction_fees": True,
+            },
         )
 
-        with pytest.raises(Exception, match="fee too small"):
-            self.app_client1.send.call(
-                params,
-                send_params={
-                    "cover_app_call_inner_transaction_fees": False,
-                },
-            )
+        assert result.transaction.raw.fee == expected_fee
+        assert len(result.confirmation.get("inner-txns", [])) == 4  # type: ignore[union-attr]
+        assert len(result.tx_ids) == 1
 
     def test_readonly_throws_when_max_fee_too_small(self) -> None:
         """Test that error is thrown when readonly method call max fee is too small to cover inner transaction fees"""
 
-        expected_fee = 7000
+        # Force `send_inners_with_fees` to be marked as readonly
+        for method in self.app_client1._app_spec.methods:  # noqa: SLF001
+            if method.name == "send_inners_with_fees":
+                method.readonly = True
+                break
+
         params = AppClientMethodCallParams(
-            method="burn_ops_readonly",
-            args=[6200],
-            max_fee=AlgoAmount.from_micro_algo(expected_fee),
+            method="send_inners_with_fees",
+            args=[self.app_client2.app_id, self.app_client3.app_id, [1000, 0, 200, 0, [500, 0]]],
+            max_fee=AlgoAmount.from_micro_algo(2000),
         )
-
         with pytest.raises(ValueError, match="Fees were too small. You may need to increase the transaction `maxFee`."):
             self.app_client1.send.call(
                 params,