💥 Off-chain messages: Naming conventions and type export for wire serialization

[manoranjith]

Location

[client, wire]

Problem

Currently, multiple conventions are used for naming structs that represent off-chain messages.

1. Exported types and suffix Msg (see wire/controlmsgs.go): ShutdownMsg, PingMsg etc.,
2. Exported types with no suffix or prefix (see client/proposalmsgs.go): LedgerChannelProposal, LedgerChannelProposalAcc etc.,
3. Unexported types with no suffix or prefix (see client/updatemsgs.go): virtualChannelFundingProposal and virtualChannelSettlementProposal
4. Unexported types with prefix msg (see client/updatemsgs.go): msgChannelUpdate, msgChannelUpdateAcc

The has two issues:

1. For implementing the protobuff adapter, all the types that represent off-chain messages must be exported.
2. It would be better to use a uniform naming convention, to make it more readable.

Proposal

My proposal is to consistently use the first style (exported types with suffix Msg for all the types.

[manoranjith]

@matthiasgeihs

[matthiasgeihs]

OK, could you make a list of which types this would affect and how you would rename them? And roughly maybe which other packages this would affect? @manoranjith

[manoranjith]

@matthiasgeihs Here's a list of names:

(u) indicates unchanged.

S.No	Old name	Name name
1	PingMsg	PingMsg (u)
2	PongMsg	PongMsg (u)
3	ShutdownMsg	ShutdownMsg (u)
4	AuthResponseMsg	AuthResponseMsg (u)
5	LedgerChannelProposal	LedgerChannelProposalMsg
6	LedgerChannelProposalAcc	LedgerChannelProposalAccMsg
7	SubChannelProposal	SubChannelProposalMsg
8	SubChannelProposalAcc	SubChannelProposalAccMsg
9	VirtualChannelProposal	VirtualChannelProposalMsg
10	VirtualChannelProposalAcc	VirtualChannelProposalAccMsg
11	ChannelProposalRej	ChannelProposalRejMsg
12	msgChannelUpdate	ChannelUpdateMsg
13	msgChannelUpdateAcc	ChannelUpdateAccMsg
14	msgChannelUpdateRej	ChannelUpdateRejMsg
15	virtualChannelFundingProposal	VirtualChannelFundingProposalMsg
16	virtualChannelSettlementProposal	VirtualChannelSettlementProposalMsg
17	msgChannelSync	ChannelSyncMsg

[matthiasgeihs]

OK, looks good. As I understand, exposing the internal message types is just an intermediate step, before we move the message types to the wire package. Not sure if we actually then could actually unexpose some of them again? If that is the case, you could add a comment to the types that indicates this.

[manoranjith]

Nope, the messages will have to remain exported forever, even after moving them to wire package.

Because, the encoders/decoders for these messages will be implemented in a different packages (say perunio or protobuf).

@matthiasgeihs