protocol-messages

[ineiti]

Old issue coming up again. For convenience we define protocol-messages in two steps:

// The actual message transmitted between nodes
type InitReply struct {
    Public abstract.Point
}

// The message with the TreeNode embedded so that we can call `msg.Parent`, `msg.TreeNode`, ...
type chanInitReply struct {
    *onet.TreeNode
    InitReply
}

// A simple dispatch method that receives a message from a children and sends it to his own parent.
func (p *Protocol)Dispatch(){
    msg := <- p.chanInitReply
    // This is the same - we're the parent of our children.
    log.Print(msg.Parent, p.TreeNode())
    // Send the same message to our parent.
    p.SendTo(p.Parent(), msg.InitReply)
}

This is inconvenient and confuses users. The problem is that we cannot protobuf.Encode(&chanInitReply) if the embedded *TreeNode is nil. So we send &InitReply and receive &chanInitReply.

I can think of two simple changes to make it possible to send and receive &chanInitReply and then use only one structure:

1. Change embedded *onet.TreeNode to onet.TreeNode Disadvantage: adds 20 bytes to each message

2. Change embedded *onet.TreeNode to field TN *onet.TreeNode Disadvantage: now the user must write msg.TN.Parent to access the parent instead of msg.Parent as before.

I tend towards the 2.

[nikkolasg]

The problem is that we cannot protobuf.Encode(&chanInitReply) if the embedded *TreeNode is nil

I don't get why do you want to protobuf this message. It's only in treenode.go that creates that wrapper using reflect to give it to the protocol but this should never be sent in the wire anyway... ? We had an issue about that but I can't find it again.

[ineiti]

my goal is to make it possible to send and receive &chanInitReply - so I need to be able to marshal it correctly. And I didn't find the old issue, neither, this is why I create a new one.

[nikkolasg]

Ok, I still have the same question :D Why do you want to send it through the network ? The overlay is supposed to take care of sending / receiving the Tree... Basically, if you do, you'll send the whole tree alongside your message. This is why onet used TreeMarshal and the likes. But you already have the tree in your protocol ! I miss something here... See the struct:

type TreeNode struct {
    // The Id represents that node of the tree
    ID TreeNodeID
    // The ServerIdentity points to the corresponding host. One given host
    // can be used more than once in a tree.
    ServerIdentity *network.ServerIdentity
    // RosterIndex is the index in the Roster where the `ServerIdentity` is located
    RosterIndex int
    // Parent link
    Parent *TreeNode
    // Children links
    Children []*TreeNode
    // Aggregate public key for *this* subtree,i.e. this node's public key + the
    // aggregate of all its children's aggregate public key
    PublicAggregateSubTree abstract.Point
}

[ineiti]

I want to avoid having two structures in a protocol-definition, when one structure should be enough. So instead of

// The actual message transmitted between nodes
type InitReply struct {
    Public abstract.Point
}

// The message with the TreeNode embedded so that we can call `msg.Parent`, `msg.TreeNode`, ...
type chanInitReply struct {
    *onet.TreeNode
    InitReply
}

I want to have

// My protocol-message - can be sent (treenode will be ignored) 
// and received (treenode will be filled in by onet)
type InitReply struct {
    TN *onet.TreeNode
    Public abstract.Point
}

[nikkolasg]

I see. Mhhhh. I'm a bit afraid of using something like this since 1) we will be using even more stranger rules with reflect, so nothing very clear and 2) this is even more going in the direction of "helping students" while we wanted to try to get away from this direction to get a more solid proof codebase.

That being said, I agree that something must be done for that. Initially I thought maybe a simple `protobuf:"-"` struct tag should suffice. I searched to do that automatically doing reflect but it seems quite complex and maybe not possible with embedded fields. But if we say users must do that, then it can work I think. Something like:

type InitReply struct {
    *onet.TreeNode `protobuf:"-"`
     Public abstract.Point
}

Otherwise, we might think deeper of what's the problem at the root. To me, our problem is dispatching in a nice way message from onet to a Protocol using channels. So not (almost) using reflect, we could also make the TreeNodeInstance create and return the channel and use a generic struct.

// in treenode.go
type ProtoChannel struct {
    *onet.TreeNode
    Msg interface{}
}

// in our protocol initialization (tni = TreeNodeInstance)
chanInit := tni.GetChannel(&InitReply{})

// during our processing
packet := <-chanInit
processInitReply(packet.Msg.(*InitReply))

This approach

• uses a lesser amount of reflect code,
• is generic,
• makes the user only declare his own packet with only the fields he needs
• does not return an error !

but

• makes the user cast hist message to the right type

In some way I find it more clear than our previous approach. I think that's one weakness in our design: the fact that even though a lot stuff is done automatically for the user (like casting), the user has to do a tremendous amount of setup in their code (registrations, special structures etc). The user have to know about all of that, whereas if it was given a specific API that can do X and Y, then the user would just have to think about how to use this API to make it do what it wants. ... Anyway.

Also it's in the same spirit as the network packet format that Service uses and that makes their packet easily transcompiled in others languages or protobuf format etc..

Finally, I think at the beginning we were too afraid of casting in Golang, but now with our experience, I hugely prefer to cast instead of using hidden reflect rules, and that seems to be more idiomatic to Golang too.

[ineiti]

Letting the user do it's own thing

We do have the possibility of registering one message and then let the protocol do the separation - see https://github.com/dedis/cothority/blob/ba38213662cfeb27e971e9949fcaa0829363b2a3/cosi/protocol/packets.go#L41

So that's the basics - if somebody doesn't want to use the nice and cozy system. But we should get this thing better documented.

Cuddling the user

I like the idea of having less overhead for the user, specifically with regard to registering messages. However, even in your approach using ProtoChannel, registering of the messages is still needed, reflection is still needed - except if you replace Msg interface{} with Msg []byte. I don't see how it's more generic than the embedding, and what is this thing about does not return an error? So, in the end, you replace the embedding of onet.TreeNode with a cast - I prefer embedding.

Registration of messages

We still have the problem of registering every message twice:

• once to the networking-layer, so he knows what protocol to instantiate if a new message arrives
• once in the protocol-creation to tell which message goes into which channel or handler

This is something we could handle by adding a better protocol-registration function that also takes into account the messages themselves. But I propose to do some whiteboarding for that.

[nikkolasg]

Ok, I may have explained in a wrong way because some of your statements about my approach are not correct, so let me try again to highlight the "lesser overhead" benefits ;)

In onet, there is this struct:

// in treenode.go
type ChannelPacket struct {
    *onet.TreeNode
    Msg interface{}
}

Onet defines it, not the user. And this is really an interface{} as this is what gets unmarshalled by protobuf, and the treenode do not need to to anything with the packet really. This brings multiple benefits:

• one less overhead for the user, as currently the user has to declare another struct with the treenode inside (your chanInitReply). The current (and proposed) approach still forces the user to embed a special struct in his packets, the user have know he must do that on top of registering the channel. It's different than here where the user have no choice to get a channel to receive messages. It's like searching for the correct API vs the correct API forces you. I prefer the latter.
• one less overhead in using reflect. Indeed, the only "reflect" operation needed here is to get the type of the packet with reflect.TypeOf() to dispatch to the correct channel. But that's it, no need to instantiate the structure and set the fields with reflect. In short, we can get rid of a lot of reflect magic in treenode.go:
  • Get rid of the reflect magic in RegisterChannel which cause trouble when one does not know what it expects.
  • Get rid of most of the reflect magic in DispatchChannel, the only thing needed here is to get the type of the incoming message by using reflect.TypeOf(). No runtime instantiation; protobuf already did that for us.

Then, in the user's protocol, we can have something like:

// the packet used by the protocol
// No need to declare anything else than what one expects
type InitReply struct {
    Public abstract.Point
}

type ShinyProtocol struct {
   *onet.TreeNodeInstance
    // the channel onet gives to receive InitReply packets
    initChan chan onet.ChannelPacket
}
func NewShinyProtocol(t *onet.TreeNodeInstance) (onet.ProtocolInstance,error) {
    // channel giving all init reply messages *without* any error due to wrongly formatted struct etc
     chanInitReply := t.Channel(InitReply{})
     ....
     return &ShinyProtocol{t,chanInitReply}
}

func (s *ShinyProtocol) Dispatch() {
       protoPacket := <-s.initChan
       // casting to the already known type in advance
       s.processInitReply(protoPacket.(*InitReply)
}

The only real overhead is the cast in s.processInitReply(protoPacket.(*InitReply). Maybe at this point we are talking about preferences but to me a single cast is much better than using reflection with shady (non-documented enough) rules as how to declare your struct.