Skip to content

add_inputs, add_outputs & apply method to facilitate method chaining #124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

add_inputs, add_outputs & apply method to facilitate method chaining #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d091a19
introduced apply & add_input supports list of utxos
Nov 5, 2022
46ca732
callback takes *args and **kwargs; added add_inputs & add_outputs met…
Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 99 additions & 7 deletions poetry.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

74 changes: 63 additions & 11 deletions pycardano/txbuilder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from copy import deepcopy
from dataclasses import dataclass, field, fields
from typing import Dict, List, Optional, Set, Tuple, Union
from typing import Dict, List, Optional, Set, Tuple, Union, Callable, no_type_check

from pycardano.address import Address, AddressType
from pycardano.backend.base import ChainContext
Expand Down Expand Up @@ -143,18 +143,52 @@ class TransactionBuilder:

_should_estimate_execution_units: bool = field(init=False, default=None)

@no_type_check
def apply(
self,
callback: Callable[[TransactionBuilder], None],
*args, **kwargs
) -> TransactionBuilder:
"""Apply the provided callback function to the transaction.

Args:
callback (Callable[TransactionBuilder], None]): A callback function.
The first parameter of the callback has to be the TransactionBuilder object itself.
Furthermore, the callback should not return a value as nothing will be done with it.

Returns:
TransactionBuilder: Current transaction builder.
"""
if callable(callback):
callback(self, *args, **kwargs)
else:
raise ValueError("Not a callable.")
return self

def add_input(self, utxo: UTxO) -> TransactionBuilder:
"""Add a specific UTxO to transaction's inputs.
"""Add a specific UTxO to the transaction's inputs.

Args:
utxo (UTxO): UTxO to be added.
utxo (UTxO): UTxO to be added to the transaction.

Returns:
TransactionBuilder: Current transaction builder.
"""
self.inputs.append(utxo)
return self

def add_inputs(self, utxos: List[UTxO]) -> TransactionBuilder:
"""Add a list of UTxOs to the transaction's inputs.

Args:
utxos (List[UTxO]): List of UTxOs to be added to the transaction.

Returns:
TransactionBuilder: Current transaction builder.
"""
self.inputs.extend(utxos)
return self

def _consolidate_redeemer(self, redeemer):
if self._should_estimate_execution_units is None:
if redeemer.ex_units:
Expand Down Expand Up @@ -190,7 +224,7 @@ def add_script_input(
datum: Optional[Datum] = None,
redeemer: Optional[Redeemer] = None,
) -> TransactionBuilder:
"""Add a script UTxO to transaction's inputs.
"""Add a script UTxO to the transaction's inputs.

Args:
utxo (UTxO): Script UTxO to be added.
Expand Down Expand Up @@ -247,7 +281,7 @@ def add_minting_script(
script: Union[UTxO, NativeScript, PlutusV1Script, PlutusV2Script],
redeemer: Optional[Redeemer] = None,
) -> TransactionBuilder:
"""Add a minting script along with its datum and redeemer to this transaction.
"""Add a minting script along with its redeemer to the transaction.

Args:
script (Union[UTxO, PlutusV1Script, PlutusV2Script]): A plutus script.
Expand All @@ -273,13 +307,13 @@ def add_minting_script(
return self

def add_input_address(self, address: Union[Address, str]) -> TransactionBuilder:
"""Add an address to transaction's input address.
Unlike :meth:`add_input`, which deterministically adds a UTxO to the transaction's inputs, `add_input_address`
will not immediately select any UTxO when called. Instead, it will delegate UTxO selection to
:class:`UTxOSelector`s of the builder when :meth:`build` is called.
"""Address to select UTxOs from to add to the transaction's inputs.
Unlike :meth:`add_input(s)`, which deterministically adds UTxO(s) to the transaction's inputs,
`add_input_address` will not immediately select any UTxO when called. Instead, it will delegate
UTxO selection to :class:`UTxOSelector`s of the builder when :meth:`build` is called.

Args:
address (Union[Address, str]): Address to be added.
address (Union[Address, str]): Address from which UTxOs will be selected.

Returns:
TransactionBuilder: The current transaction builder.
Expand Down Expand Up @@ -310,6 +344,24 @@ def add_output(
self.datums[datum_hash(datum)] = datum
return self

def add_outputs(self, tx_out: TransactionOutput, *tx_outs) -> TransactionBuilder:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have the parameter type as a List[TransactionOutput], similar to add_inputs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree List[TransactionOutput] is better even though it's pretty clever to allow users to provide multiple TransactionOutput through argument unpacking.

Copy link
Author

@Et9797 Et9797 Nov 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but I wouldn't recommend not allowing varargs at all, because I think there's a strong case here for having them, as it saves typing out list brackets every time. The option to have List[TransactionOutput] + varargs as well as a single TransactionOutput + varargs would make it even more flexible syntactically imo.

In contrast, I don't think varargs syntax is justified for add_inputs, because most of the times you'll just be using filtered UTxOs retrieved from chain_context.utxos rather than writing your own UTxO objects yourself:

   def construct_tx1(self) -> Transaction:

    utxos: list[UTxO] = filter_func(chain_context.utxos(addr)) # more commonly

    builder1 = TransactionBuilder(chain_context)
    (
    builder1
      .add_inputs(
         UTxO(
            TransactionInput(tx_hash, id )
            TransactionOutput(addr, amount, ....)
         ), # rarely
         UTxO(
            TransactionInput(txhash, id),
            TransactionOutput(addr, amount, ....)
         ) # rarely
      )
    )

    (
    builder1
      .add_inputs(utxos) #more commonly
    )

Without varargs, extra brackets required:

   def construct_tx1(self) -> Transaction:
    builder1 = TransactionBuilder(chain_context)
    (
    builder1
      .add_outputs([
        TransactionOutput(address=usr2_addr, amount=5_000_000),
        TransactionOutput(address=usr2_addr, amount=5_000_000),
        TransactionOutput(address=usr2_addr, amount=8_000_000)
      ])
    )

Copy link
Collaborator

@cffls cffls Nov 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your points. I was thinking more about the consistency in these APIs. What do you think of this:

def add_inputs(self, *utxos: UTxO]) -> TransactionBuilder

def add_outputs(self, *tx_outs: TransactionOutput) -> TransactionBuilder

This will make add_inputs and add_outputs more similar and less inconsistent, and users can pass both list and multi-args to them.

Outputs passed as a list:

my_output_list = get_output_list() # Get an output list from somewhere
builder1 = TransactionBuilder(chain_context)
    (
    builder1
      .add_outputs(*my_output_list)
    )

Outputs passed as multiple positional args:

builder1 = TransactionBuilder(chain_context)
    (
    builder1
      .add_outputs(
        TransactionOutput(address=usr2_addr, amount=5_000_000),
        TransactionOutput(address=usr2_addr, amount=5_000_000),
        TransactionOutput(address=usr2_addr, amount=8_000_000)
      )
    )

Copy link
Author

@Et9797 Et9797 Nov 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think this * version or the alternative List[...] only variant would then be the most succinct solutions. In the latter case then just stick with having to add brackets.
Only drawback with the * version is that it may seem that the * argument is optional, which it really isn't. Maybe good to have a check to see if it is empty and throw an error if it is. Not strictly required though because if it is empty it will just extend an empty tuple which doesn't throw a runtime error

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, my preference is still using List[...], as it is more commonly used in Python.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only drawback with the * version is that it may seem that the * argument is optional, which it really isn't. Maybe good to have a check to see if it is empty and throw an error if it is. Not strictly required though because if it is empty it will just extend an empty tuple which doesn't throw a runtime error

This concern is valid in my opinion.
When things are vague, explicit is better than implicit just like zen of python.
List[...] seems better for users since IDEs will display the expected type more clearly.

"""Add multiple transaction outputs.

Args:
tx_out (TransactionOutput): The transaction output to be added.
*tx_outs (TransactionOutput): Optionally add more transaction outputs.

Returns:
TransactionBuilder: Current transaction builder.
"""
self.outputs.append(tx_out)
if not all(isinstance(o, TransactionOutput) for o in tx_outs):
raise InvalidArgumentException(
"Not all provided arguments are of type \"TransactionOutput\"."
)
self.outputs.extend(tx_outs)
return self

@property
def inputs(self) -> List[UTxO]:
return self._inputs
Expand Down Expand Up @@ -1142,7 +1194,7 @@ def build_and_sign(
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.
the provided signing keys.

Args:
signing_keys (List[Union[SigningKey, ExtendedSigningKey]]): A list of signing keys that will be used to
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ cose = "0.9.dev8"
pprintpp = "^0.4.0"
mnemonic = "^0.20"
ECPy = "^1.2.5"
Deprecated = "^1.2.13"

[tool.poetry.dev-dependencies]
Sphinx = "^4.3.2"
Expand Down