-
-
Notifications
You must be signed in to change notification settings - Fork 76
Reference UTxOs are UTxOs (not TransactionInputs) #181
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.
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
Conversation
They are used as UTxOs in the scripts too
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #181 +/- ##
==========================================
- Coverage 86.23% 85.78% -0.45%
==========================================
Files 26 26
Lines 2767 2814 +47
Branches 655 672 +17
==========================================
+ Hits 2386 2414 +28
- Misses 287 296 +9
- Partials 94 104 +10
... and 7 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
pycardano/txbuilder.py
Outdated
reference_inputs: Set[TransactionInput] = field( | ||
init=False, default_factory=lambda: set() | ||
) | ||
reference_inputs: List[UTxO] = field(init=False, default_factory=lambda: list()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be a backward-imcompatible change. The reason of using set was to avoid input duplication. What do you think of changing the type to Set[Union[TransactionInput, UTxO]]
or Union[Set[TransactionInput], Set[UTxo]]
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I resolve the duplication when building the tx. I think UTxOs are not hashable either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UTxOs are in fact hashable:
pycardano/pycardano/transaction.py
Lines 467 to 470 in a065bce
def __hash__(self): | |
return hash( | |
blake2b(self.input.to_cbor("bytes") + self.output.to_cbor("bytes"), 32) | |
) |
It would be less amount of change to keep using Set as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I will make it a Set of UTxOs.
If we allow Union[TransactionInput, UTxO] we need to filter out again later too. Also building the ScriptContext fails then. But it could be desirable if someone just wants to make sure another transaction is in the UTxO while this tx is being processed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for making this change!
Reference UTxOs have been so much requested by Smart Contract developers because they grant the Smart Contract access to the content (i.e. corresponding output, datums, scripts) of the referenced input.
This implies that the whole UTxO is actually required to build the ScriptContext properly. This PR changes the structure of the TxBuilder class such that it stores the entire reference UTxO instead of just the TransactionInput, enabling generating the ScriptContext for redeemers from it (see OpShin/opshin#92)