Enable the LocalStateQuery protocol #1522
Conversation
|
This includes #1507 until that is merged. This PR is only about the final commit. The This should only be merged when we're ready to break "network compatibility". |
There was a problem hiding this comment.
LGTM - but I only looked under Ouroboros.Network.Protocol.LocalStateQuery. Somebody needs to review the rest.
You'll have a conflict with current master. To resolve it you will need to put Examples,Tests,Direct modules in the private library ouroboros-protocol-tests. If you need to re-use Examples in ouroboros-consensus you can leave it in the public library.
| import Ouroboros.Network.Protocol.LocalStateQuery.Type | ||
|
|
||
|
|
||
| newtype LocalStateQueryClient block query result m a = LocalStateQueryClient { |
There was a problem hiding this comment.
There's no need for the new type wrapper, but let it be this way. We plan to remove all the top level newtype wrappers from all of the mini-protocols.
There was a problem hiding this comment.
Ok. I'll leave it there for consistency.
| -- It must be prepared to handle either. | ||
| -- | ||
| data ClientStAcquiring block query result m a = ClientStAcquiring { | ||
| recvMsgAcquired :: ClientStAcquired block query result m a, |
There was a problem hiding this comment.
Initially, I aligned all state types on their type parameters, even when some had an m (..) around them. Now that I have added so many ms, there's no point in alignment on this line, I'll remove it.
Thanks for giving this a look! BTW, #1507 is the PR that I want to merge now, not this one, which is #1507 + the breaking change that enables it.
That's what Edsko did in #1507 🙂
No, I rebased this morning already. I have moved |
mrBliss
force-pushed
the
mrBliss/enable-local-state-query-protocol
branch
from
81f52e4
to
028bb7f
Compare
'NodeToClientV_2' version of node-to-client protocol supports local-state-query mini-protocol.
`maximumMiniProtocolLimits` we are not planning to change `node-to-client` protocol limits; `node-to-client` protocol is executed in a trusted environment, so there's no need to introduce tight bounds for mux ingress queue.
coot
force-pushed
the
mrBliss/enable-local-state-query-protocol
branch
from
028bb7f
to
9ca9de9
Compare
`Ouroboros.Network.NodeToClient.nodeToClientProtocols` takes now `NodeToClientVersion` as an argument, which provides the flexibility of running different set of mini-protocols per `NodeToClientVersion` version.
Added `ByronNetworkProtocolVersion1_1`.
coot
force-pushed
the
mrBliss/enable-local-state-query-protocol
branch
from
9ca9de9
to
15d61d0
Compare
|
@karknu , this re-introduces the config parameter for the node-to-client protocols, but it is only a -- We pass the 'BlockConfig' here, even though it is currently unused. If at any
-- point we want to introduce local protocols that for example send Byron blocks
-- or headers across, we will need to have the epoch size, which comes from the
-- Byron config. Unlike the full 'TopLevelConfig', it should not be difficult
-- for a wallet to construct the 'BlockConfig'.
--
-- NOTE: Somewhat confusingly, 'pcChainSyncCodec' currently /does/ send Byron
-- blocks across, but it does not deserialize them (the user of the codec is
-- itself responsible for doing that), which is why it currently does not need
-- the config. |
Following Karl's suggestion: we set 16ths bit to distinguish `NodeToClient` and `NodeToNode` protocols. This patch includes a round trip tests for `Serialise` class of `NodeToClientVersion` and `NodeToNodeVersion`, including cross failure tests which checks that we cannot deserialise `NodeToClientVersion` as `NodeToNodeVersion` (and vice versa).
| , testGroup "NodeToNodeVersion" | ||
| [ testCase "NodeToNodeVersion round-trip codec property" | ||
| (roundTripPropAll (Proxy :: Proxy NodeToNodeVersion)) | ||
| -- TODO: enable this test when `NodeToClientV_1` is removed: |
|
bors merge |
coot
changed the title
No description provided.