status-im / status-protocol-go

Status Protocol implementation in Go
Mozilla Public License 2.0
0 stars 1 forks source link

Handle messages in raw method as well #37

Closed cammellos closed 5 years ago

cammellos commented 5 years ago

Refactor handling of messages, the logic was getting a bit unwieldy with adding the datasync layer. The layers are:

Transport -> Whisper, provides authentication,integrity Encryption -> PFS, Bundle, multidevice processing, can provide authentication DataSync -> multiplexes messages ApplicationMetadata -> Provides authentication,integrity, canonical messageID Application -> Provides a parsed message

The idea is that any handling is done through this structure

type StatusMessage struct {
// TransportMessage is the parsed message received from the trasport layer, i.e the input
TransportMessage interface{}
// ParsedMessage is the parsed message by the application layer, i.e the output
ParsedMessage interface{}

// TransportLayerInfo contains information about the processing of the trasport layer message
TransportLayerInfo LayerInfo
// EncryptionLayerInfo contains information about the processing of the encryption layer message
EncryptionLayerInfo LayerInfo
// DataSyncLayerInfo contains information about the processing of the data-sync layer message
DataSyncLayerInfo LayerInfo
// ApplicationMetadataLayerInfo contains information about the processing of the application-metadata layer message
ApplicationMetadataLayerInfo LayerInfo
}

Each layer will be passed this structure in, use the method Payload() []byte to get the payload from the previous layer and will set a corresponding LayerInfo:

type LayerInfo struct {
// The ID calculated at this layer
ID []byte
// SigPubKey is the signature extracted at this layer
SigPubKey *ecdsa.PublicKey
// Error is any non-fatal error that happened at this layer
Error error
// Payload is the payload extracted at this layer, to be passed to the next
Payload []byte
}

and return one or (more in some cases, such as DataSync) StatusMessage. If an error is returned that means that the pipeline is to be interrupted (say a corrupted signature), otherwise for "soft" errors (say one message is corrupted, while the rest are fine), the Error field is set, and the struct is to be passed to the next layer.

The input is an empty struct with TransportMessage set. The output is a fully filled struct with ParsedMessage set.

For Raw methods we pass a flag to stop processing before ParsedMessage is se (i.e we skip the applicationlayer).

I haven't made it more generic that it ought to be as currently there's no need to swap layers, but once that need arise we can make it more generic (interfaces for each layer, same signature etc).

cammellos commented 5 years ago

@adambabik what do you think? I haven't gone around fixing all the stuff/test etc, but it would be good to check if you think it's ok as a structure. (there's still some stuff that needs to be moved etc, but overall architecture)

handleMessages is basically the pipeline for processing incoming messages.

adambabik commented 5 years ago

Refactor handling of messages, the logic was getting a bit unwieldy with adding the datasync layer.

Agree with this :)

Generally, I think it's a good direction. I am not sure about these TransportMessage and ParsedMessage as interfaces. Also, is that enough to have a single LayerInfo type? For example, the encryption layer might provide details for the transport layer when there is a negotiated secret and sending messages.

Another thing is whether we should group functionality into layers. The problem I see is that the state of StatusMessage is managed by some other entities instead of the StatusMessage itself. Maybe layers can only be a property of the application and message should have behaviours implemented using interfaces?

I also think this adapter thing is a mistake and not needed.

type StatusMessage struct {
    id []byte

    message interface{} // Message, PairMessage, etc.
    encodedMessage []byte // support raw (transit-encoded) messages

    transportMeta struct {
        hash []byte
        sigPublicKey *ecdsa.PublicKey
        chatID string
        public bool
    }
    encryptedPayload []byte   

    encryptionMeta struct {
        spec *ProtocolMessageSpec
    }
    decryptedPayload []byte
}

func (m *StatusMessage) Decrypt(dec Decryptor) (err error) {
    // If StatusMessage does not support data sync,
    // m.decryptedPayload would be assigned to m.encodedMessage as well. 
    // It can a feature flag of StatusMessage or a separate struct using composition.
    m.decryptedPayload, err = dec.Decrypt(m.encryptedPayload)
    return
}

func (m *StatusMessage) Demux(datasync DataSync) ([]*StatusMessage, error) {
    messages, err := datasync.Handle(m.decryptedPayload)
    try(err)

    var result []*StatusMessage
    for _, message := range messages {
        sm := m.clone()
        sm.encodedMessage = message
    }

    return result, nil
}

func (m *StatusMessage) Decode(dec Decoder) (err error) {
    m.message, err = dec.Decode(m.encodedMessage)
    return
}

In the application, we could have very simple flow:

// Here, we could have methods like handleTransportLayer, handlerEncryptionLayer etc
// if it would make more sense. Especially, if one layer requires a few operations which is 
// not our case for now.
func (app) HandleMessage(m *StatusMessage) ([]*protocol.StatusMessage, error) {
    err := app.tryDecrypt(m)
    try(err)

    messages, err = app.tryDemux(m)
    try(err)

    for _, m := range messages {
        err := app.tryDecode(m)
        try(err)
    }

    return messages, nil
}

func (app) tryDecrypt(m Decryptable) error {
    if app.skipDecrypt { 
        return nil 
    }
    return m.Decrypt(app.decryptor)
}

func (app) tryDemux(m *StatusMessage) ([]*StatusMessage, error) {
    if app.featureFlags.datasync {
        demuxable, ok := m.(Demuxable)
        if !ok { ... }
        return m.Demux(app.datasync)
    }
    return []*StatusMessage{m}
}

Sending messages would look similar:

func (app) SendMessage(cfg MessageConfig) ([]byte, error) {
    msg, err := cfg.Build()
    try(err)

    err = app.tryEncode(msg)
    try(err)

    err = app.tryWrapMessageV1(msg)
    try(err)

    err = app.tryMux(msg)
    try(err)

    err = msg.Encrypt(app.encryptor)
    try(err)

    return app.Send(msg)
}

I left the transport layer out of StatusMessage in order to simplify things but it could be included as Receive(Receiver) method on StatusMessage. I included transportMeta field which has important transport-layer metadata required for by other layers.

Some fields like encryptionMeta would be used only when sending a message. For more clarity, we could have StatusMessageOut and StatusMessageIn.

cammellos commented 5 years ago

@adambabik thanks for the feedback, I have incorporated most of the suggestions, (if not all :) ), I have kept the naming as HandleXLayer as in some cases it does more than Decrypting/Demuxing etc, so it might be slightly misleading. Also for now I have only refactored the receiving flow, I will leave sending for the next PR.

pedropombeiro commented 5 years ago

Great refactor @cammellos, thanks!