do not enforce non-empty outputs for unsigned tx #2255
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KtorZ
added
the
ADDING FEATURE
hasufell
approved these changes
Oct 19, 2020
| @@ -970,7 +970,7 @@ data UnsignedTx input = UnsignedTx | |||
| { unsignedInputs | |||
| :: NonEmpty input | |||
| , unsignedOutputs | |||
| :: NonEmpty TxOut | |||
| :: [TxOut] | |||
There was a problem hiding this comment.
Should add a comment here why we don't want NonEmpty. Will bite someone later, maybe.
There was a problem hiding this comment.
Actually, there are many use-cases for empty outputs. This occurs in particular often when performing a selection for delegation: in this scenario, there are typically no outputs at all and, depending on the size of the selected input(s), there might be no change at all (because of the minUTxO value).
KtorZ
force-pushed
the
KtorZ/2200/non-empty-outputs
branch
from
c60c5a6 to
2bdbb29
Compare
|
bors merge |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 19, 2020
2255: do not enforce non-empty outputs for unsigned tx r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #2200 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - c60c5a6 📍 **do not enforce non-empty outputs for unsigned tx** Actually, there are many use-cases for empty outputs. This occurs in particular often when performing a selection for delegation: in this scenario, there are typically no outputs at all and, depending on the size of the selected input(s), there might be no change at all (because of the minUTxO value). # Comments <!-- Additional comments or screenshots to attach if any --> Should fix: - #2213 (comment) - #2213 (comment) Thanks @piotr-iohk 🙏 <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]>
|
Build failed: Same as: #2249 (comment) |
|
What the heck o.O |
|
bors merge |
|
Build succeeded: |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 26, 2020
2268: Remove outdated comments about empty output lists in transactions. r=KtorZ a=jonathanknowles # Issue Number Cleanup after #2247, #2255, and other related PRs. # Overview This PR: * removes a couple of outdated comments about disallowing empty lists of outputs in transactions. It's been established that having an empty list of outputs is perfectly reasonable in certain circumstances (such as a transaction that includes just a delegation certificate). * updates the `Arbitrary ApiTransaction` instance to generate possibly-empty lists of outputs. Co-authored-by: Jonathan Knowles <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Oct 26, 2020
2268: Remove outdated comments about empty output lists in transactions. r=KtorZ a=jonathanknowles # Issue Number Cleanup after #2247, #2255, and other related PRs. # Overview This PR: * removes a couple of outdated comments about disallowing empty lists of outputs in transactions. It's been established that having an empty list of outputs is perfectly reasonable in certain circumstances (such as a transaction that includes just a delegation certificate). * updates the `Arbitrary ApiTransaction` instance to generate possibly-empty lists of outputs. Co-authored-by: Jonathan Knowles <[email protected]>
iohk-bors bot
added a commit
that referenced
this pull request
Oct 26, 2020
2268: Remove outdated comments about empty output lists in transactions. r=jonathanknowles a=jonathanknowles # Issue Number Cleanup after #2247, #2255, and other related PRs. # Overview This PR: * removes a couple of outdated comments about disallowing empty lists of outputs in transactions. It's been established that having an empty list of outputs is perfectly reasonable in certain circumstances (such as a transaction that includes just a delegation certificate). * updates the `Arbitrary ApiTransaction` instance to generate possibly-empty lists of outputs. Co-authored-by: Jonathan Knowles <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Number
#2200
Overview
📍 do not enforce non-empty outputs for unsigned tx
Actually, there are many use-cases for empty outputs. This occurs in
particular often when performing a selection for delegation: in this
scenario, there are typically no outputs at all and, depending on the
size of the selected input(s), there might be no change at all
(because of the minUTxO value).
Comments
Should fix:
Thanks @piotr-iohk 🙏