Fix wallet state transition functions to properly account for script validation failures. #3037
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
jonathanknowles
force-pushed
the
jonathanknowles/adp-1039/increase-testability-of-wallet-state-transition-functions
branch
3 times, most recently
from
d987b7c to
91bb49f
Compare
The `Coin` type has its own `Semigroup` and `Monoid` instances that correspond to addition and summation. So there's no need to unwrap `Coin` values in order to sum them.
It's a singular `Coin` sum value, rather than a plurality of withdrawals.
This will make it possible to test the function in isolation.
This will make it possible to test the function in isolation.
This will make it possible to test the function in isolation.
This commit splits `applyTx` into the following two functions: - `applyOurTxToUTxO`: conditionally applies a single transaction to a UTxO set, if the transaction is relevant to the wallet. - `applyOurTx`: conditionally applies a single transaction to a UTxO set in the context of an accumulating list of already applied transactions. The `applyOurTxToUTxO` function will be easier to test in isolation, since there's no need to supply an accumulating list argument.
This commit restricts the scope of `mkTxMeta` so that it is an inner function of `applyOurTxToUTxO`. This will mean that `mkTxMeta` can reuse any terms that are brought into scope by `applyOurTxToUTxO`.
This will make it easier to turn `applyOurTxToUTxO` into a top-level function, as it no longer has implict arguments.
This will make it possible to test the function in isolation.
The name `applyBlockToUTxO` describes the purpose of this function more accurately, as it really does compute the result of applying a block to a UTxO set.
jonathanknowles
force-pushed
the
jonathanknowles/adp-1039/increase-testability-of-wallet-state-transition-functions
branch
from
62feea3 to
e3381d2
Compare
jonathanknowles
changed the title
jonathanknowles
force-pushed
the
jonathanknowles/adp-1039/increase-testability-of-wallet-state-transition-functions
branch
from
e3381d2 to
8a6e5b7
Compare
This function is never called in a context where an extra monadic context is necessary. Therefore, there's no need to impose the extra burden of providing such a context on callers.
This property demonstrates the equivalence between `applyTxToUTxO` and `applyOurTxToUTxO` in contexts where all transaction inputs and outputs are marked as belonging to the wallet.
jonathanknowles
force-pushed
the
jonathanknowles/adp-1039/increase-testability-of-wallet-state-transition-functions
branch
2 times, most recently
from
ccb4ebd to
631fbf1
Compare
jonathanknowles
force-pushed
the
jonathanknowles/adp-1039/increase-testability-of-wallet-state-transition-functions
branch
from
631fbf1 to
5764d28
Compare
jonathanknowles
changed the title
We now use: - `result` - `haveResult` - `shouldHaveResult` In the case where we have a result, we may or may not have an updated UTxO.
sevanspowell
approved these changes
Dec 1, 2021
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Dec 1, 2021
3037: Fix wallet state transition functions to properly account for script validation failures. r=jonathanknowles a=jonathanknowles ## Issue Number ADP-1039 ## Summary This PR: - breaks up wallet state transition functions into smaller functions for easier testing. - adds function `isOurTx`, which returns `True` if and only if a transaction is relevant to the wallet. - adjusts the withdrawal sum computation within `applyOurTxToUTxO` to correctly account for script validation failure. - adds a property test for `applyOurTxToUTxO` that relates it to `isOurTx` and `applyTxToUTxO`. - fixes the termination logic of `applyOurTxToUTxO` so that it returns a result only when appropriate. Co-authored-by: Jonathan Knowles <[email protected]>
|
Build failed: Failures: |
|
bors r+ |
|
Build succeeded: |
iohk-bors
bot
deleted the
jonathanknowles/adp-1039/increase-testability-of-wallet-state-transition-functions
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-1039
Summary
This PR:
isOurTx, which returnsTrueif and only if a transaction is relevant to the wallet.applyOurTxToUTxOto correctly account for script validation failure.applyOurTxToUTxOthat relates it toisOurTxandapplyTxToUTxO.applyOurTxToUTxOso that it returns a result only when appropriate.