-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: balance tx fails fast on txcoll or collRet #3343
Conversation
51068ee
to
fd0fcc0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat!
For testing I think it would be very nice if we could:
-
make
prop_balanceTransactionValid
run in bothAlonzo
andBabbage
which would be valuable irrespective of this task. I think and hope this is not that complicated. -
Generate a tiny amount of totalCollateral and returnCollateral in https://github.com/input-output-hk/cardano-wallet/blob/f5cff1c23260c6985f9007b0b5ac8fb29be631ae/lib/core/src/Cardano/Api/Gen.hs#L1335-L1337
And actually, we could then perhaps add some checks here to mimic the ledger rules
to make the tests fail unless we have guardExisting{Return, Total}Collateral
.
8d2b101
to
573c6e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Unisay
Thanks for making this PR! I've left a few suggestions and comments.
a0d8556
to
4fa7e55
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot, LGTM! As discussed it would be cool to extend prop_balanceTransactionValid
with the collateral rules for a "higher level" property, but we don't have to do that now.
But I would like ask to split up at least the swagger-reformatting into a separate commit before merging (see comments).
4fa7e55
to
285c79d
Compare
285c79d
to
06a7648
Compare
bors r+ |
Build succeeded: |
balanceTransaction
fails gracefully if Tx body contains either total collateral attribute or collateral return outputs attribute.Issue Number
ADP-1653