blinklabs-io/gouroboros

Go implementation of the Cardano Ouroboros protocol

gouroboros's Issues

Implement functional options pattern for config

Our OuroborosOptions struct is starting to get a bit unwieldy.

https://golang.cafe/blog/golang-functional-options-pattern.html

Implement pipeline mini-protocol of chain-sync

https://github.com/input-output-hk/ouroboros-network/blob/756fdd6d6c592d95066af9512d7dbba6362cc169/docs/diffusion-pipelining-design/block-diffusion-pipelining.tex#L513

Improve muxer/mini-protocol error handling

Any problem with decoding CBOR or sending/receiving messages in the wrong protocol state should cause the connection to be immediately terminated. We're currently just bubbling the errors up to the user and assuming they will do the right thing.

Instead, we should be breaking out of receive loops and closing connections within the library. We can still pass the error up to the user at the same time. Because everything is async, we'll need to figure out how to break out of all receive loops, or at least prevent the spewing of more errors from things trying to read from closed sockets.

This may be related to #24

Compare network magic value in handshake on server side

We currently only check for the highest version, but the version data (network magic) also needs to match.

Refactor mini-protocols to use a shared parent object

Use stored CBOR of message when sending

When sending a message in Protocol.SendMessage(), we should check if the .Cbor() function returns any data and use that instead of doing the encoding ourselves.

Figure out a way to get friendly message names into error messages

With the move to centralized handling of protocol state and send/receive, we lost the name of protocol messages in error messages. Figure out a way to have the protocol.Message objects return a "name" string, or perhaps just have a map of numeric message type to a "name" string.

Require protocols to be explicitly started

We currently register the protocol handlers for all relevant protocols after the handshake completes. Instead, we should require the consumer to opt-in to each protocol.

The Ouroboros object should still call .New() on each protocol, but the call to the muxer's RegisterProtocol() function should be hidden behind a .Start() function. It probably also makes sense to move the protocol-specific callback function config to an arg to this .Start() function and out of the top level OuroborosOptions struct.

Refactor protocol state machine

Move the management into the parent object and create a map in each mini-protocol.

CBOR parsing failure on local-tx-submission rejection message

This was seen in cloudstruct/go-cardano-submit-api when a transaction gets rejected.

{"level":"error","timestamp":"2022-05-05T19:06:24Z","caller":"api/api.go:154","msg":"failure communicating with node: protocol error: local-tx-submission: decode error: cbor: invalid map key type: []uint8","stacktrace":"github.com/cloudstruct/go-cardano-submit-api/internal/api.handleSubmitTx.func4\n\t/code/internal/api/api.go:154"}

Support for specifying protocol diffusion mode

We currently only support the initiator-only diffusion mode, but we should at least make it explicit rather than implicit.

We should also add checks for messages that are marked as being from the initiator (not a response) when acting as a client in initiator-only mode. The cardano-node will produce an error like this when receiving a message marked as from the initiator while in this mode:

MuxError MuxInitiatorOnly "id = MiniProtocolNum 7"

Implement Local State Query mini-protocol

Implement handshake refusal

We currently can't properly cope when there are no matching versions in the handshake when acting as a server. This will require figuring out the various refusal reasons (we have consts for their IDs) and how they are formatted, and adding checks for those cases.

Implement pipelining limits in various mini-protocols

The network spec doc says that each mini-protocol has its own built-in limit for the number of queued messages that it will handle between the muxer output and the mini-protocol input and that violating that limit is treated as a protocol violation which will result in a disconnect. We've definitely seen some weirdness with larger values of pipelined messages with chain-sync over NtN (but it resulted in a hang rather than a disconnect). Figure out the value for each mini-protocol and codify the limits on both the client and server side.

Finish populating localstatequery message unit tests

Implement P2P peer selection

nil pointer reference when calling Close() after Dial() failed

Implement server side of handshake

This will also involve adding some general way to specify whether a connection is acting as a client or a server.

Document the code

Documentation can be easily generated from the Go code, but we need to add comments and consider what pieces that we actually want to be exported.

https://pkg.go.dev/github.com/cloudstruct/go-ouroboros-network
https://pkg.go.dev/about
https://go.dev/blog/godoc
https://tip.golang.org/doc/comment
https://mdaverde.com/posts/golang-local-docs/

Create Close() function for Ouroboros object

This should "gracefully" shut down the connection. This means setting an object flag and/or poking a channel to do things like stop reads, stop error collection, close the connection, etc.

Create "new" functions for all protocol message types

We have them for a few, but we should create NewMsg...() functions for every message type

Create integration test framework

This will require an actual cardano-node instance with an actual chain to sync. This could be the official testnet or a custom testnet that we generate blocks for.

Fix race condition around handshake and protocol registration

There's a race condition between the muxer and the registration of additional mini-protocols after the handshake completes. We need a way to "stop" the muxer from receiving new messages after the handshake until we have registered all of the mini-protocols.

Without this, it's possible for a client to send a message that we're not ready for and it throw an error because there's not an appropriate protocol handler registered yet.

Finish populating handshake message unit tests

Finish populating localtxsubmission message unit tests

Add pipelining support for sending messages

We already support pipelining on receive, but we need it on send, too. Update the chain-sync tests to take advantage of pipelining.

Finish populating blockfetch message unit tests

This will also involve fixing the encoding of messages dealing with block content.

Finish implementing local-state-query

The messages and protocol state machine were added in #10, but there are no helper functions, test programs, or query/result handling.

Embed Protocol type in each mini-protocol type

This will make some code slightly cleaner and expose things like SendMessage() directly to the consumer

Finish populating chainsync message unit tests

This will also involve fixing the encoding of the messages dealing with block data.

New()/Dial() functions hang when handshake fails

When the handshake fails, we're left sitting in setupConnection() waiting for the handshake to be complete. We need to figure out a way to get notified of the failure and bail out of the function

Implement Local TxMonitor mini-protocol

NOTE: this is not currently documented in the Ouroboros network spec document

Here's an example implementation that's not Haskell:

https://github.com/bloxbean/yaci/blob/main/core/src/main/java/com/bloxbean/cardano/yaci/core/protocol/localtxmonitor/LocalTxMonitorState.java

Split handshake protocol into client and server

We need to run separate initiator and responder instances of each mini-protocol and its state machine when running in full duplex mode.

Create unit test framework for mini-protocol message CBOR encoding/decoding

We need to be able to test that all of the mini-protocol message objects decode as expected from known CBOR and that an "identical" message object created from scratch properly encodes back into the same CBOR. We know that many of them will not for now, so we just want to create the framework and implement a few simple messages in each mini-protocol.

Finish populating txsubmission message unit tests

Protocol message handler needs to know whether message is response

We need to pass the value from <segment>.IsResponse() to the message handler function somehow.

Finish implementing tx-submission

The messages and protocol state machine were implemented in #8, but there are no helper functions or test program.

We'll also need to figure out a way to do custom CBOR encoding of the params for certain messages based on this note in the network spec:

; The codec only accepts infinite−length list encoding for tsIdList!

Support splitting large messages across multiple muxer segments

The max payload size for a muxer segment is 65535, but messages larger than that can be sent. They must be split across multiple muxer segments.

Support pipelining of client requests

This will probably involve only checking for incoming messages when the protocol state machine is in a state with client agency. As a side note, the CBOR for all of the messages in the buffer is added to each parsed message, and we should figure out how to save only the CBOR for the current message.

Figure out issue with zero byte reads in muxer

I'll occasionally run into an error accessing index 0 in an array when the remote host closes the connection unexpectedly (usually in response to a "bad" message sent to them). There should be some additional error checking around this to detect the zero byte read (that we still try to decode as CBOR) to give a more useful error.

Create framework for tracing and/or debug logging

We need a way to expose what's going on inside the library to the consumer

Implement Keep-Alive mini-protocol

Implement proper support for version negotiation

We currently hardcode a version number in the handshake for NtN and NtC. We need to map each protocol version number to various functionality to enable/disable.

https://input-output-hk.github.io/ouroboros-network/ouroboros-network/Ouroboros-Network-NodeToNode-Version.html
https://input-output-hk.github.io/ouroboros-network/ouroboros-network/Ouroboros-Network-NodeToClient-Version.html

Implement Local TxSubmission mini-protocol

Add general unit tests

Create unit tests around all our CBOR utility functions and anything else that can be reasonably unit tested without external infra. Any testing around encoding/decoding of CBOR for mini-protocol messages will be handled in #63

Pass negotiated protocol version to mini-protocols

They can use this to modify their behavior based on protocol version rather than having the flags in a global version table.

break out of Protocol recvLoop() on error

We currently pass along the error, but we don't exit the function, which causes the nil message object to be passed along to places that don't expect it, triggering the nil pointer reference and panic.

race condition when stopping muxer

panic: close of closed channel

goroutine 1078 [running]:
github.com/cloudstruct/go-ouroboros-network/muxer.(*Muxer).Stop(0x4000076e80)
        /go/pkg/mod/github.com/cloudstruct/[email protected]/muxer/muxer.go:57 +0x94
github.com/cloudstruct/go-ouroboros-network.(*Ouroboros).Close(0x4000324460)
        /go/pkg/mod/github.com/cloudstruct/[email protected]/ouroboros.go:107 +0x30
github.com/cloudstruct/go-ouroboros-network.(*Ouroboros).setupConnection.func2(0x4000324460)
        /go/pkg/mod/github.com/cloudstruct/[email protected]/ouroboros.go:175 +0xb4
created by github.com/cloudstruct/go-ouroboros-network.(*Ouroboros).setupConnection
        /go/pkg/mod/github.com/cloudstruct/[email protected]/ouroboros.go:171 +0x254

Implement TipSample mini-protocol

The messages look pretty simple to implement, but it's not clear how this protocol is actually used.

https://github.com/input-output-hk/ouroboros-network/blob/master/ouroboros-network/src/Ouroboros/Network/Protocol/TipSample/Codec.hs

Implement TxSubmission2 mini-protocol

We only need support for TxSubmission2 (not TxSubmission), because we've chosen not to support pre-Alonzo protocol versions which would use TxSubmission.