-
Notifications
You must be signed in to change notification settings - Fork 86
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
Fixed cddl spec for tx-submission protocol #4580
Conversation
We should use `tx` rather than `txId` in the cddl. It only works because we use `int` for both.
acb6114
to
367755f
Compare
8046456
to
d8eedfb
Compare
@bolt12 I changed this PR considerably, so please review it again. We should use this idea in other mini-protocols / specs as well. The only question is if we should modify data structure in |
d8eedfb
to
43586d7
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.
Nice, is there an open issue about this? Would be nice to track this nice improvement
ouroboros-network-protocols/testlib/Ouroboros/Network/Protocol/TxSubmission2/Test.hs
Outdated
Show resolved
Hide resolved
I created the issue: #4595 |
43586d7
to
2a00a1c
Compare
I had to also fix the |
4009b63
to
dcdd47a
Compare
`cddl` comes with `any` definition, it can be used to indicate that the data structures in our spec are not specified in `ouroboros-network`. This patch introduces `Any` type which can be used for the same reason on Haskell side. This patch only introduces it in `TxSubmission` and `LocalTxSubmission` mini-protocols, but we should do that for other mini-protocols too. In this patch we modified the definition of `Tx` and `TxId` used in `ouroboros-network-protocols:test` as well as `ouroboros-network-protocols:cddl` components.
These are already run in `ouroboros-network-framework:test`. Also fixed a typo.
dcdd47a
to
2b8ded6
Compare
; transactions (tx) are themselves, `ouroboros-network` is polymorphic over | ||
; them. | ||
txId = any | ||
tx = any |
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.
Where can I find specification of what is expected here by the actual cardano-node implementation?
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.
We should use
tx
rather thantxId
in the cddl. It only worksbecause we use
int
for both.