Refactor: break down ApiConstructTransactionData validation #3598
Conversation
| , mintBurn = Nothing | ||
| , delegations = Nothing | ||
| } -> liftHandler $ throwE ErrConstructTxWrongPayload | ||
| _ -> pure () |
There was a problem hiding this comment.
I remember the discussion about this in the #haskell channel!
I wonder if there's a way to extract this out as a general pattern?
On the one hand, it seems desirable that when we introduce a new field, we get a compilation failure on any record pattern match that's intended to be complete. On the other hand, if we updated all similar record pattern matches across the code base, it would introduce a lot of boilerplate.
Perhaps this is something that can only be fixed in GHC.
There was a problem hiding this comment.
I wonder if there's a way to extract this out as a general pattern?
Good question, unfortunately I don't see how could it be possible.
On the other hand, if we updated all similar record pattern matches across the code base, it would introduce a lot of boilerplate.
I'd argue then that boilerplate (it is syntactically more verbose than desired) is justified by the additional guarantees that it gives.
Co-authored-by: Jonathan Knowles <[email protected]>
Unisay
force-pushed
the
yura/ADP-2267/quit-stake-pool-balanceTx
branch
from
0de2644
to
a927c62
Compare
There was a problem hiding this comment.
Seems like a nice decomposition of previously big blob into logical steps/checks/validations that occur in constructTransaction. Thanks to that, many of those pieces could be reused in constructSharedTransaction. I would appreciate smaller commits next time to follow the changes. At this time although they seem correct I would say testing, especially integration one, will tell the truth if those changes are correct.
|
bors merge |
|
Build succeeded: |
…ata validation r=Unisay a=Unisay This PR is an increment towards the state where `Cardano.Wallet.Api.createTransaction` API request body validation is modular and is done in a spirit of [Parse, don’t validate](https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/). I intend to continue down this road, such that: - API layer is responsible for converting HTTP request into a data type that captures a transaction construction task (`TransactionCtx` atm). - `TransactionCtx` is passed to the `Cardano.Wallet.createTransaction` layer which takes care of the semantic validations and does all the logic. Such refactoring will make it possible to reimplement e.g. `quitStakePool` in terms of the `Cardano.Wallet.createTransaction`. ### Issue Number ADP-2267 Co-authored-by: Yura Lazarev <[email protected]> Source commit: 956fa1f
This PR is an increment towards the state where
Cardano.Wallet.Api.createTransactionAPI request body validation is modular and is done in a spirit of Parse, don’t validate.I intend to continue down this road, such that:
API layer is responsible for converting HTTP request into a data type that captures a transaction construction task (
TransactionCtxatm).TransactionCtxis passed to theCardano.Wallet.createTransactionlayer which takes care of the semantic validations and does all the logic.Such refactoring will make it possible to reimplement e.g.
quitStakePoolin terms of theCardano.Wallet.createTransaction.Issue Number
ADP-2267