Prevent the migration algorithm from creating money. #77
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
The library currently defines the dust threshold as the maximum size of a dust coin, rather than the minimum size of a non-dust coin. It is necessary to define it this way to ensure that zero-valued coins are always eliminated from adjusted selections. (It is not possible to specify a dust threshold lower than zero, and therefore not possible to prevent zero-valued coins from being eliminated.)
This function provides a reusable indicator for whether or not a given coin is a dust coin.
We can now eliminate the auxilliary `noDust` function.
The migration algorithm should satisfy the following property: >>> Total value of all inputs >= sum of selection change coins Before this change, it was possible to cause the algorithm to violate this property by giving it a single input value lower than the dust threshold. This change adds a guard to the `mkCoinSelection` function, preventing it from creating a change coin if that change coin would be higher than the total value of all inputs.
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.
Related Issue
#74
Summary
The migration algorithm should satisfy the following property:
However, recent test failures have showed that the existing implementation occasionally failed to respect this property, in very rare situations.
In particular, supplying a coin selection where the total value of all inputs (in a given batch) is less than the minimum value of a non-dust coin will reliably produce a set of change coins whose total value is greater than the total value of all inputs. (See #74 for examples of test failures.)
This PR fixes the above issue by adding a guard to the
mkCoinSelectionfunction, preventing it from creating an initial change coin if that change coin would be higher than the total value of all inputs.Demonstration (with one million tests per property):
selectCoins properties No coin selection has outputs +++ OK, passed 1000000 tests. Every coin in the selection change > dust threshold +++ OK, passed 1000000 tests. Total input UTxO value >= sum of selection change coins +++ OK, passed 1000000 tests. Every selection input is unique +++ OK, passed 1000000 tests. Every selection input is a member of the UTxO +++ OK, passed 1000000 tests. Every coin selection is well-balanced +++ OK, passed 1000000 tests.Other Changes
This PR also makes the following changes:
isDustto provide a uniform way to determine whether or not a given coin is a dust coin, and uses it to replace a number of inequality checks.