Use approved codecs from Ouroboros.Consensus.NodeNetwork #744
Conversation
| channel | ||
| (LocalTxSub.localTxSubmissionClientPeer | ||
| (txSubmissionClientSingle tx)) | ||
| case result of | ||
| Nothing -> traceWith tracer TraceLowLevelAccepted | ||
| Just msg -> traceWith tracer (TraceLowLevelRejected $ show msg) | ||
| } | ||
| where | ||
| codecs = protocolCodecs cfg (mostRecentNetworkProtocolVersion (Proxy @ blk)) |
There was a problem hiding this comment.
Is this correct?
IntersectMBO/ouroboros-network#1633 and IntersectMBO/ouroboros-network#1522 are not yet closed/merge. In other words: consensus supports newer protocol versions (to support the LocalStateQuery protocol and to include the block size in the headers sent across the ChainSync protocol), but I'm not sure that network has enabled them. So using the most recent protocol version might not be correct.
However, I think you happen to be safe because the two local protocols you're using are the same for all current protocol versions.
But if we add another version that does affect one of those two protocols, then this will immediately use that, which might not be what we want.
There was a problem hiding this comment.
Thx for the references to the related issues.
At the moment I'm just looking at the places that use codes with are not defined in Ouroboros.Consensus.NodeNetwork and try to find out if it is possible to export all codecs from a single API.
Yes this will need carefully reviewing before it is ready.
|
@MarcFontaine can I close this PR? |
|
Yes it is outdated |
Issue
This PR results/does not result in breaking changes to upstream dependencies.
Checklist
This PR contains all the work required to resolve the linked issue.
The work contained has sufficient documentation to describe what it does and how to do it.
The work has sufficient tests and/or testing.
I have committed clear and descriptive commits. Be considerate as somebody else will have to read these.
I have added the appropriate labels to this PR.