Perform verification of SelectionCollateralError values.
#2988
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 subset of the largest reported combination that is suitable for | ||
| -- use as collateral. This set should be exactly the same size as the | ||
| -- reporte largest combination. |
There was a problem hiding this comment.
Suggested change
| -- reporte largest combination. | |
| -- reported largest combination. |
There was a problem hiding this comment.
Haha, thanks!
sevanspowell
approved these changes
Oct 22, 2021
jonathanknowles
force-pushed
the
jonathanknowles/adp-1037/test-coin-selection-9
branch
from
14571e4 to
f2165d7
Compare
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 22, 2021
2988: Perform verification of `SelectionCollateralError` values. r=jonathanknowles a=jonathanknowles ## Issue Number ADP-1037 ## Summary This PR adds verification of `SelectionCollateralError` values. We verify that: - The reported `largestCombinationAvailable` value does not include any UTxO entries that are unsuitable for use as collateral. - The number of entries within `largestCombinationAvailable` is not greater than `maximumCollateralInputCount`. - The total value of entries within `largestCombinationAvailable` is not greater than the computed `minimumSelectionAmount` (as this would mean that the error was returned erroneously: we **_could_** have generated a selection). ## Example failures ### Collateral suitability If we introduce a mutation that deliberately removes the collateral suitability check: ```patch diff --git a/lib/core/src/Cardano/Wallet/Primitive/CoinSelection.hs b/lib/core/src/Cardano/Wallet/Primitive/CoinSelection.hs index 994103a68..a137cfc05 100644 --- a/lib/core/src/Cardano/Wallet/Primitive/CoinSelection.hs +++ b/lib/core/src/Cardano/Wallet/Primitive/CoinSelection.hs @@ -316,9 +316,8 @@ toCollateralConstraintsParams balanceResult (constraints, params) = } collateralParams = Collateral.SelectionParams { coinsAvailable = - Map.mapMaybeWithKey - (curry (view #utxoSuitableForCollateral constraints)) - (unUTxO (view #utxoAvailableForCollateral params)) + view (#tokens . #coin) <$> unUTxO + (view #utxoAvailableForCollateral params) , minimumSelectionAmount = computeMinimumCollateral ComputeMinimumCollateralParams { minimumCollateralPercentage = ``` Then we see a verification failure similar to:  ### Maximum number of collateral inputs If we introduce a mutation that deliberately passes an incorrect maximum number of collateral inputs to the collateral selection algorithm: ```patch diff --git a/lib/core/src/Cardano/Wallet/Primitive/CoinSelection.hs b/lib/core/src/Cardano/Wallet/Primitive/CoinSelection.hs index 6fc4d4f5d..1b18e0360 100644 --- a/lib/core/src/Cardano/Wallet/Primitive/CoinSelection.hs +++ b/lib/core/src/Cardano/Wallet/Primitive/CoinSelection.hs @@ -305,7 +305,7 @@ toCollateralConstraintsParams balanceResult (constraints, params) = where collateralConstraints = Collateral.SelectionConstraints { maximumSelectionSize = - view #maximumCollateralInputCount constraints + view #maximumCollateralInputCount constraints + 1 , searchSpaceLimit = -- We use the default search space limit here, as this value is -- used in the test suite for 'Collateral.performSelection'. We ``` Then we see a verification failure similar to:  Co-authored-by: Jonathan Knowles <[email protected]>
|
Build failed: Another cached failure. The cause seems to be from a completely unrelated PR that had several jobs that failed because of resource limitatons on hydra, and these job failures have propagated to the merge attempt for this PR. I've restarted all the failed jobs manually, and cleared the errors. |
|
bors r+ |
|
Build succeeded: |
iohk-bors
bot
deleted the
jonathanknowles/adp-1037/test-coin-selection-9
branch
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
ADP-1037
Summary
This PR adds verification of
SelectionCollateralErrorvalues.We verify that:
largestCombinationAvailablevalue does not include any UTxO entries that are unsuitable for use as collateral.largestCombinationAvailableis not greater thanmaximumCollateralInputCount.largestCombinationAvailableis not greater than the computedminimumSelectionAmount(as this would mean that the error was returned erroneously: we could have generated a selection).Example failures
Collateral suitability
If we introduce a mutation that deliberately removes the collateral suitability check:
Then we see a verification failure similar to:
Maximum number of collateral inputs
If we introduce a mutation that deliberately passes an incorrect maximum number of collateral inputs to the collateral selection algorithm:
Then we see a verification failure similar to: