Remove the input selection limit from coin selection. #3921
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
Note that this necessitates removing some coverage checks that some selections will terminate due to reaching a limit.
All selections are now unlimited, so we no longer need to test coverage for that case.
It's no longer possible for this error to be created.
The selection input count can never exceed the limit, as there is no limit, so the condition can never fail.
There is nothing left to adjust, as there can never be a limit.
jonathanknowles
changed the title
SelectionLimit type.
|
bors r+ |
|
Build succeeded: |
WilliamKingNoel-Bot
pushed a commit
that referenced
this pull request
May 11, 2023
…lection. r=jonathanknowles a=jonathanknowles ## Issue ADP-3029 ## Summary This PR: - removes the notion of an "input selection limit" from coin selection. - removes the `computeSelectionLimit` field from all coin-selection-related constraint records. - removes the `SelectionLimit` type. - removes the `SelectionLimitReached` error, thus simplifying the hierarchy of errors returned by `balanceTransaction`. ## Context For many years, the coin selection algorithm has provided an ability to limit the number of inputs it selects. This ability was added as a way to prevent transactions from growing too large, which would render them invalid for submission to the network. However, the advent of the multi-asset era invalidated one of the assumptions on which this design was based: namely that all inputs should require roughly the same amount of space to encode. In the multi-asset era, each input can reference an output with an arbitrary selection of tokens, and thus different inputs can require wildly different amounts of space to encode the change that they generate. For example, it's quite possible to build a transaction with 10 inputs that requires more space than a transaction with 100 inputs. Since all wallet endpoints now use the `balanceTransaction` function to perform coin selection, and since `balanceTransaction` does not use the selection limit functionality in any way, there are now no code paths that require this limit to be present. Therefore, we can simply delete it. Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io> Source commit: 8bb1eeb
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
ADP-3029
Summary
This PR:
computeSelectionLimitfield from all coin-selection-related constraint records.SelectionLimittype.SelectionLimitReachederror, thus simplifying the hierarchy of errors returned bybalanceTransaction.Context
For many years, the coin selection algorithm has provided an ability to limit the number of inputs it selects.
This ability was added as a way to prevent transactions from growing too large, which would render them invalid for submission to the network.
However, the advent of the multi-asset era invalidated one of the assumptions on which this design was based: namely that all inputs should require roughly the same amount of space to encode. In the multi-asset era, each input can reference an output with an arbitrary selection of tokens, and thus different inputs can require wildly different amounts of space to encode the change that they generate.
For example, it's quite possible to build a transaction with 10 inputs that requires more space than a transaction with 100 inputs.
Since all wallet endpoints now use the
balanceTransactionfunction to perform coin selection, and sincebalanceTransactiondoes not use the selection limit functionality in any way, there are now no code paths that require this limit to be present.Therefore, we can simply delete it.