Make script estimation more accurate

cffls · cffls · commit fcc2da80e258 · 2022-10-09T17:01:41.000-07:00
diff --git a/integration-test/run_tests.sh b/integration-test/run_tests.sh
@@ -7,39 +7,14 @@ ROOT=$(pwd)
 
 poetry install
 
-##########
-# Alonzo #
-##########
-
-# Cleanup containers and volumes in case there is any running
-docker-compose down --volumes --remove-orphans
-
-# Run alonzo integration tests
-./bootstrap.sh local-alonzo
-
-# Launch containers
-docker-compose up -d
-
-export PAYMENT_KEY="$ROOT"/configs/local-alonzo/shelley/utxo-keys/utxo1.skey
-export EXTENDED_PAYMENT_KEY="$ROOT"/keys/extended.skey
-export POOL_ID=$(cat "$ROOT"/keys/pool/pool.id)
-
-# Wait for stake pool to start producing blocks
-sleep 20
-
-poetry run pytest -s -vv -n 4 "$ROOT"/test -m "not (post_alonzo)"
-
-# Cleanup
-docker-compose down --volumes --remove-orphans
-
 #########
 # Vasil #
 #########
 
 # Cleanup containers and volumes in case there is any running
 docker-compose down --volumes --remove-orphans
 
-# Run alonzo integration tests
+# Run integration tests
 ./bootstrap.sh local-vasil
 
 # Launch containers
diff --git a/pycardano/txbuilder.py b/pycardano/txbuilder.py
@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from copy import copy, deepcopy
+from copy import deepcopy
 from dataclasses import dataclass, field, fields
 from typing import Dict, List, Optional, Set, Tuple, Union
 
@@ -865,7 +865,9 @@ def build(
         Returns:
             TransactionBody: A transaction body.
         """
-        self._update_execution_units()
+        self._update_execution_units(
+            change_address, merge_change, collateral_change_address
+        )
         self._ensure_no_input_exclusion_conflict()
         selected_utxos = []
         selected_amount = Value()
@@ -1020,7 +1022,7 @@ def _set_collateral_return(self, collateral_return_address: Address):
         ):
             return
 
-        if not collateral_return_address or self._should_estimate_execution_units:
+        if not collateral_return_address:
             return
 
         collateral_amount = (
@@ -1045,15 +1047,15 @@ def _add_collateral_input(cur_total, candidate_inputs):
                         cur_total += candidate.output.amount
 
             sorted_inputs = sorted(
-                self.inputs.copy(), key=lambda i: i.output.amount, reverse=True
+                self.inputs.copy(),
+                key=lambda i: (len(i.output.to_cbor()), -i.output.amount.coin),
             )
             _add_collateral_input(tmp_val, sorted_inputs)
 
             if tmp_val.coin < collateral_amount:
                 sorted_inputs = sorted(
                     self.context.utxos(str(collateral_return_address)),
-                    key=lambda i: i.output.amount,
-                    reverse=True,
+                    key=lambda i: (len(i.output.to_cbor()), -i.output.amount.coin),
                 )
                 _add_collateral_input(tmp_val, sorted_inputs)
 
@@ -1062,33 +1064,38 @@ def _add_collateral_input(cur_total, candidate_inputs):
         for utxo in self.collaterals:
             total_input += utxo.output.amount
 
-        # TODO: Remove this check when we are in Vasil era
-        if total_input.multi_asset:
-            if collateral_amount > total_input.coin:
+        if collateral_amount > total_input.coin:
+            raise ValueError(
+                f"Minimum collateral amount {collateral_amount} is greater than total "
+                f"provided collateral inputs {total_input}"
+            )
+        else:
+            return_amount = total_input - collateral_amount
+            min_lovelace_val = min_lovelace_post_alonzo(
+                TransactionOutput(collateral_return_address, return_amount),
+                self.context,
+            )
+            if min_lovelace_val > return_amount.coin:
                 raise ValueError(
-                    f"Minimum collateral amount {collateral_amount} is greater than total "
-                    f"provided collateral inputs {total_input}"
+                    f"Minimum lovelace amount for collateral return {min_lovelace_val} is "
+                    f"greater than collateral change {return_amount.coin}. Please provide more collateral inputs."
                 )
             else:
-                return_amount = total_input - collateral_amount
-                min_lovelace_val = min_lovelace_post_alonzo(
-                    TransactionOutput(collateral_return_address, return_amount),
-                    self.context,
+                self._collateral_return = TransactionOutput(
+                    collateral_return_address, total_input - collateral_amount
                 )
-                if min_lovelace_val > return_amount.coin:
-                    raise ValueError(
-                        f"Minimum lovelace amount for collateral return {min_lovelace_val} is "
-                        f"greater than collateral change {return_amount.coin}. Please provide more collateral inputs."
-                    )
-                else:
-                    self._collateral_return = TransactionOutput(
-                        collateral_return_address, total_input - collateral_amount
-                    )
-                    self._total_collateral = collateral_amount
+                self._total_collateral = collateral_amount
 
-    def _update_execution_units(self):
+    def _update_execution_units(
+        self,
+        change_address: Optional[Address] = None,
+        merge_change: bool = False,
+        collateral_change_address: Optional[Address] = None,
+    ):
         if self._should_estimate_execution_units:
-            estimated_execution_units = self._estimate_execution_units()
+            estimated_execution_units = self._estimate_execution_units(
+                change_address, merge_change, collateral_change_address
+            )
             for r in self.redeemers:
                 key = f"{r.tag.name.lower()}:{r.index}"
                 if key not in estimated_execution_units:
@@ -1107,15 +1114,19 @@ def _update_execution_units(self):
     def _estimate_execution_units(
         self,
         change_address: Optional[Address] = None,
+        merge_change: bool = False,
+        collateral_change_address: Optional[Address] = None,
     ):
-        # Create a shallow copy of current builder, so we won't mess up current builder's internal states
+        # Create a deep copy of current builder, so we won't mess up current builder's internal states
         tmp_builder = TransactionBuilder(self.context)
         for f in fields(self):
             if f.name not in ("context",):
-                setattr(tmp_builder, f.name, copy(getattr(self, f.name)))
+                setattr(tmp_builder, f.name, deepcopy(getattr(self, f.name)))
         tmp_builder._should_estimate_execution_units = False
         self._should_estimate_execution_units = False
-        tx_body = tmp_builder.build(change_address)
+        tx_body = tmp_builder.build(
+            change_address, merge_change, collateral_change_address
+        )
         witness_set = tmp_builder._build_fake_witness_set()
         tx = Transaction(
             tx_body, witness_set, auxiliary_data=tmp_builder.auxiliary_data
@@ -1128,6 +1139,7 @@ def build_and_sign(
         signing_keys: List[Union[SigningKey, ExtendedSigningKey]],
         change_address: Optional[Address] = None,
         merge_change: Optional[bool] = False,
+        collateral_change_address: Optional[Address] = None,
     ) -> Transaction:
         """Build a transaction body from all constraints set through the builder and sign the transaction with
         provided signing keys.
@@ -1139,12 +1151,17 @@ def build_and_sign(
                 transaction body will likely be unbalanced (sum of inputs is greater than the sum of outputs).
             merge_change (Optional[bool]): If the change address match one of the transaction output, the change amount
                 will be directly added to that transaction output, instead of being added as a separate output.
+            collateral_change_address (Optional[Address]): Address to which collateral changes will be returned.
 
         Returns:
             Transaction: A signed transaction.
         """
 
-        tx_body = self.build(change_address=change_address, merge_change=merge_change)
+        tx_body = self.build(
+            change_address=change_address,
+            merge_change=merge_change,
+            collateral_change_address=collateral_change_address,
+        )
         witness_set = self.build_witness_set()
         witness_set.vkey_witnesses = []
 
diff --git a/test/pycardano/test_txbuilder.py b/test/pycardano/test_txbuilder.py
@@ -473,17 +473,6 @@ def test_add_script_input(chain_context):
     assert [datum] == witness.plutus_data
     assert [redeemer1, redeemer2] == witness.redeemer
     assert [plutus_script] == witness.plutus_v1_script
-    assert (
-        "a6008282582018cbe6cadecd3f89b60e08e68e5e6c7d72d730aaa1ad21431590f7e6643438"
-        "ef0082582018cbe6cadecd3f89b60e08e68e5e6c7d72d730aaa1ad21431590f7e6643438ef"
-        "01018282581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6e5f41a00"
-        "4c4b4082581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6e5f4821a"
-        "00e06beba1581c876f19078b059c928258d848c8cd871864d281eb6776ed7f80b68536a149"
-        "54657374546f6b656e02021a000475d509a1581c876f19078b059c928258d848c8cd871864"
-        "d281eb6776ed7f80b68536a14954657374546f6b656e010b5820c0978261d9818d92926eb0"
-        "31d38d141f513a05478d697555f32edf6443ebeb080d818258203131313131313131313131"
-        "31313131313131313131313131313131313131313100" == tx_body.to_cbor()
-    )
 
 
 def test_add_script_input_no_script(chain_context):
@@ -514,16 +503,6 @@ def test_add_script_input_no_script(chain_context):
     assert [datum] == witness.plutus_data
     assert [redeemer] == witness.redeemer
     assert witness.plutus_v1_script is None
-    assert (
-        "a6008182582018cbe6cadecd3f89b60e08e68e5e6c7d72d730aaa1ad21431590f"
-        "7e6643438ef00018282581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2a"
-        "c960262444f6e5f41a004c4b4082581d60f6532850e1bccee9c72a9113ad98bcc"
-        "5dbb30d2ac960262444f6e5f41a0048cc3b021a00037f050b5820032d812ee073"
-        "1af78fe4ec67e4d30d16313c09e6fb675af28f825797e8b5621d0d81825820313"
-        "13131313131313131313131313131313131313131313131313131313131310012"
-        "8182582018cbe6cadecd3f89b60e08e68e5e6c7d72d730aaa1ad21431590f7e66"
-        "43438ef00" == tx_body.to_cbor()
-    )
 
 
 def test_add_script_input_find_script(chain_context):
@@ -569,16 +548,6 @@ def test_add_script_input_find_script(chain_context):
         assert [redeemer] == witness.redeemer
         assert witness.plutus_v1_script is None
         assert [existing_script_utxo.input] == tx_body.reference_inputs
-        assert (
-            "a6008182582018cbe6cadecd3f89b60e08e68e5e6c7d72d730aaa1ad2143159"
-            "0f7e6643438ef00018282581d60f6532850e1bccee9c72a9113ad98bcc5dbb3"
-            "0d2ac960262444f6e5f41a004c4b4082581d60f6532850e1bccee9c72a9113a"
-            "d98bcc5dbb30d2ac960262444f6e5f41a0048cc3b021a00037f050b5820032d"
-            "812ee0731af78fe4ec67e4d30d16313c09e6fb675af28f825797e8b5621d0d8"
-            "182582031313131313131313131313131313131313131313131313131313131"
-            "3131313100128182582041cb004bec7051621b19b46aea28f0657a586a05ce2"
-            "013152ea9b9f1a5614cc701" == tx_body.to_cbor()
-        )
 
 
 def test_add_script_input_with_script_from_specified_utxo(chain_context):
@@ -620,15 +589,6 @@ def test_add_script_input_with_script_from_specified_utxo(chain_context):
     assert [redeemer] == witness.redeemer
     assert witness.plutus_v2_script is None
     assert [existing_script_utxo.input] == tx_body.reference_inputs
-    assert (
-        "a6008182582018cbe6cadecd3f89b60e08e68e5e6c7d72d730aaa1ad21431590f7e6643438"
-        "ef00018282581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6e5f41a"
-        "004c4b4082581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6e5f41a"
-        "0048cc3b021a00037f050b58203dde0555ddcbc6ae54f5b3c72c0d1f84dfcd22fa758f64bd"
-        "2122885c4c5f8ed20d81825820313131313131313131313131313131313131313131313131"
-        "313131313131313100128182582041cb004bec7051621b19b46aea28f0657a586a05ce2013"
-        "152ea9b9f1a5614cc701" == tx_body.to_cbor()
-    )
 
 
 def test_add_minting_script_from_specified_utxo(chain_context):
@@ -667,19 +627,6 @@ def test_add_minting_script_from_specified_utxo(chain_context):
     assert [redeemer] == witness.redeemer
     assert witness.plutus_v2_script is None
     assert [existing_script_utxo.input] == tx_body.reference_inputs
-    assert (
-        "a700828258203131313131313131313131313131313131313131313131313131313131313131"
-        "0082582032323232323232323232323232323232323232323232323232323232323232320101"
-        "8282581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6e5f41a004c4b40"
-        "82581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6e5f4821a0057f1f3"
-        "a2581c31313131313131313131313131313131313131313131313131313131a246546f6b656e"
-        "310146546f6b656e3202581c669ce610ef17d3ac9f5663f0748381120376e72b031e002db95a"
-        "3981a14954657374546f6b656e01021a00039b8d09a1581c669ce610ef17d3ac9f5663f07483"
-        "81120376e72b031e002db95a3981a14954657374546f6b656e010b5820cb8b108226416e0c75"
-        "46b09bdee0726dfcd07827a0a6c9c98dac7f02d7d3382f0d8182582031313131313131313131"
-        "3131313131313131313131313131313131313131313100128182582041cb004bec7051621b19"
-        "b46aea28f0657a586a05ce2013152ea9b9f1a5614cc701" == tx_body.to_cbor()
-    )
 
 
 def test_collateral_return(chain_context):
@@ -814,16 +761,6 @@ def test_add_minting_script(chain_context):
     tx_body = tx_builder.build(change_address=receiver)
     witness = tx_builder.build_witness_set()
     assert [plutus_script] == witness.plutus_v1_script
-    assert (
-        "a6008182582018cbe6cadecd3f89b60e08e68e5e6c7d72d730aaa1ad21431590f7e6643438ef"
-        "00018282581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6e5f4821a00"
-        "4c4b40a1581c876f19078b059c928258d848c8cd871864d281eb6776ed7f80b68536a1495465"
-        "7374546f6b656e0182581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6"
-        "e5f41a0048c10f021a00038a3109a1581c876f19078b059c928258d848c8cd871864d281eb67"
-        "76ed7f80b68536a14954657374546f6b656e010b58205fcf68adc7eb6e507d15fb07d1c4e39d"
-        "908bc9dfe642368afcddd881c5d465170d818258203131313131313131313131313131313131"
-        "31313131313131313131313131313100" == tx_body.to_cbor()
-    )
 
 
 def test_add_minting_script_wrong_redeemer_type(chain_context):
@@ -929,16 +866,6 @@ def test_estimate_execution_unit(chain_context):
     assert [redeemer1] == witness.redeemer
     assert redeemer1.ex_units is not None
     assert [plutus_script] == witness.plutus_v1_script
-    assert (
-        "a6008182582018cbe6cadecd3f89b60e08e68e5e6c7d72d730aaa1ad21431590f7e6643438ef"
-        "00018282581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6e5f41a004c"
-        "4b4082581d60f6532850e1bccee9c72a9113ad98bcc5dbb30d2ac960262444f6e5f4821a0048"
-        "fa42a1581c876f19078b059c928258d848c8cd871864d281eb6776ed7f80b68536a149546573"
-        "74546f6b656e01021a000350fe09a1581c876f19078b059c928258d848c8cd871864d281eb67"
-        "76ed7f80b68536a14954657374546f6b656e010b58206b5664c6f79646f2a4c17bdc1ecb6f6b"
-        "f540db5c82dfa0a9d806c435398756fa0d818258203131313131313131313131313131313131"
-        "31313131313131313131313131313100" == tx_body.to_cbor()
-    )
 
 
 def test_tx_builder_exact_fee_no_change(chain_context):
