Refactor group chat for better message ID creation

groupChat/send.go

@@ -15,9 +15,7 @@ import (
 	"gitlab.com/elixxir/client/cmix"
 	"gitlab.com/elixxir/client/cmix/message"
 	gs "gitlab.com/elixxir/client/groupChat/groupStore"
-	"gitlab.com/elixxir/crypto/cyclic"
 	"gitlab.com/elixxir/crypto/group"
-	"gitlab.com/elixxir/primitives/format"
 	"gitlab.com/xx_network/primitives/id"
 	"gitlab.com/xx_network/primitives/netTime"
 	"io"
@@ -65,19 +63,12 @@ func (m *manager) Send(groupID *id.ID, tag string, message []byte) (
 	timeNow := netTime.Now().Round(0)
 
 	// Create a cMix message for each group member
-	groupMessages, err := m.newMessages(g, tag, message, timeNow)
+	groupMessages, msgId, err := m.newMessages(g, tag, message, timeNow)
 	if err != nil {
 		return 0, time.Time{}, group.MessageID{},
 			errors.Errorf(newCmixMsgErr, g.Name, g.ID, err)
 	}
 
-	// Obtain message ID
-	msgId, err := getGroupMessageId(
-		m.getE2eGroup(), groupID, m.getReceptionIdentity().ID, timeNow, message)
-	if err != nil {
-		return 0, time.Time{}, group.MessageID{}, err
-	}
-
 	// Send all the groupMessages
 	param := cmix.GetDefaultCMIXParams()
 	param.DebugTag = "group.Message"
@@ -94,17 +85,49 @@ func (m *manager) Send(groupID *id.ID, tag string, message []byte) (
 
 // newMessages builds a list of messages, one for each group chat member.
 func (m *manager) newMessages(g gs.Group, tag string, msg []byte,
-	timestamp time.Time) ([]cmix.TargetedCmixMessage, error) {
+	timestamp time.Time) ([]cmix.TargetedCmixMessage, group.MessageID, error) {
 
 	// Create list of cMix messages
 	messages := make([]cmix.TargetedCmixMessage, 0, len(g.Members))
 	rng := m.getRng().GetStream()
 	defer rng.Close()
 
-	// fixme: maybe make internal message here, pass it into
-	//  newCmixMsg and return, instead of making it continuously?
+	// Generate initial internal message
+	maxCmixMessageLength := m.getCMix().GetMaxMessageLength()
 
-	// Create cMix messages in parallel
+	// Generate public message to determine what length internal message can be
+	pubMsg, err := newPublicMsg(maxCmixMessageLength)
+	if err != nil {
+		return nil, group.MessageID{}, errors.Errorf(newPublicMsgErr, err)
+	}
+
+	// Generate internal message
+	intlMsg, err := newInternalMsg(pubMsg.GetPayloadSize())
+	if err != nil {
+		return nil, group.MessageID{}, errors.Errorf(newInternalMsgErr, err)
+	}
+
+	// Return an error if the message is too large to fit in the payload
+	if intlMsg.GetPayloadMaxSize() < len(msg) {
+		return nil, group.MessageID{}, errors.Errorf(
+			messageLenErr, len(msg), intlMsg.GetPayloadMaxSize())
+	}
+
+	// Generate internal message
+	internalMessagePayload := setInternalPayload(intlMsg, timestamp,
+		m.getReceptionIdentity().ID, msg)
+
+	mar, _ := json.Marshal(NewPublicInternalMessage_DeleteThis(intlMsg))
+
+	jww.INFO.Printf("GROUP MSG ID DEBUG (newCmixMsg):"+
+		"senders group ID: %s\n, "+
+		"internalMessage marshal: %s\n"+
+		"internalMessage json: %s\n",
+		g.ID,
+		base64.StdEncoding.EncodeToString(internalMessagePayload),
+		string(mar))
+
+	// Create cMix messages
 	for _, member := range g.Members {
 		// Do not send to the sender
 		if m.getReceptionIdentity().ID.Cmp(member.ID) {
@@ -112,21 +135,22 @@ func (m *manager) newMessages(g gs.Group, tag string, msg []byte,
 		}
 
 		// Add cMix message to list
-		cMixMsg, err := newCmixMsg(g, tag, msg, timestamp, member, rng,
-			m.getReceptionIdentity().ID, m.getCMix().GetMaxMessageLength())
+		cMixMsg, err := newCmixMsg(g, tag, timestamp, member, rng, maxCmixMessageLength,
+			internalMessagePayload)
 		if err != nil {
-			return nil, err
+			return nil, group.MessageID{}, err
 		}
 		messages = append(messages, cMixMsg)
 	}
 
-	return messages, nil
+	return messages, group.NewMessageID(g.ID, internalMessagePayload), nil
 }
 
 // newCmixMsg generates a new cmix.TargetedCmixMessage for the given group
 // member
-func newCmixMsg(g gs.Group, tag string, msg []byte, timestamp time.Time,
-	mem group.Member, rng io.Reader, senderId *id.ID, maxCmixMessageSize int) (
+func newCmixMsg(g gs.Group, tag string, timestamp time.Time,
+	mem group.Member, rng io.Reader, maxCmixMessageSize int,
+	internalMessagePayload []byte) (
 	cmix.TargetedCmixMessage, error) {
 
 	// Initialize targeted message
@@ -139,18 +163,12 @@ func newCmixMsg(g gs.Group, tag string, msg []byte, timestamp time.Time,
 		},
 	}
 
-	// Create three message layers
-	pubMsg, intlMsg, err := newMessageParts(maxCmixMessageSize)
+	// Generate public message
+	pubMsg, err := newPublicMsg(maxCmixMessageSize)
 	if err != nil {
 		return cmixMsg, err
 	}
 
-	// Return an error if the message is too large to fit in the payload
-	if intlMsg.GetPayloadMaxSize() < len(msg) {
-		return cmixMsg, errors.Errorf(
-			messageLenErr, len(msg), intlMsg.GetPayloadMaxSize())
-	}
-
 	// Generate 256-bit salt
 	salt, err := newSalt(rng)
 	if err != nil {
@@ -166,21 +184,9 @@ func newCmixMsg(g gs.Group, tag string, msg []byte, timestamp time.Time,
 		return cmixMsg, errors.WithMessage(err, newKeyErr)
 	}
 
-	// Generate internal message
-	payload := setInternalPayload(intlMsg, timestamp, senderId, msg)
-
-	mar, _ := json.Marshal(NewPublicInternalMessage_DeleteThis(intlMsg))
-
-	jww.INFO.Printf("GROUP MSG ID DEBUG (newCmixMsg):"+
-		"senders group ID: %s\n, "+
-		"internalMessage marshal: %s\n"+
-		"internalMessage json: %s\n",
-		g.ID,
-		base64.StdEncoding.EncodeToString(intlMsg.Marshal()),
-		string(mar))
-
 	// Encrypt internal message
-	encryptedPayload := group.Encrypt(key, cmixMsg.Fingerprint, payload)
+	encryptedPayload := group.Encrypt(key, cmixMsg.Fingerprint,
+		internalMessagePayload)
 
 	// Generate public message
 	cmixMsg.Payload = setPublicPayload(pubMsg, salt, encryptedPayload)
@@ -212,46 +218,6 @@ func NewPublicInternalMessage_DeleteThis(msg internalMsg) *PublicInternalMessage
 	}
 }
 
-// getGroupMessageId builds the group message ID.
-func getGroupMessageId(grp *cyclic.Group, groupId, senderId *id.ID,
-	timestamp time.Time, msg []byte) (group.MessageID, error) {
-	cmixMsg := format.NewMessage(grp.GetP().ByteLen())
-	_, intlMsg, err := newMessageParts(cmixMsg.ContentsSize())
-	if err != nil {
-		return group.MessageID{}, errors.WithMessage(err,
-			"Failed to make message parts for message ID")
-	}
-
-	intlMsgMarshal := setInternalPayload(intlMsg, timestamp, senderId, msg)
-
-	mar, _ := json.Marshal(NewPublicInternalMessage_DeleteThis(intlMsg))
-	jww.INFO.Printf("GROUP MSG ID DEBUG (getGroupMsgId): "+
-		"senders group ID: %s\n, "+
-		"internalMessage marshal: %s\n"+
-		"internalMessage json: %s\n",
-		groupId,
-		base64.StdEncoding.EncodeToString(intlMsgMarshal),
-		string(mar))
-
-	return group.NewMessageID(groupId, intlMsgMarshal), nil
-}
-
-// newMessageParts generates a public payload message and the internal payload
-// message. An error is returned if the messages cannot fit in the payloadSize.
-func newMessageParts(payloadSize int) (publicMsg, internalMsg, error) {
-	pubMsg, err := newPublicMsg(payloadSize)
-	if err != nil {
-		return pubMsg, internalMsg{}, errors.Errorf(newPublicMsgErr, err)
-	}
-
-	intlMsg, err := newInternalMsg(pubMsg.GetPayloadSize())
-	if err != nil {
-		return pubMsg, intlMsg, errors.Errorf(newInternalMsgErr, err)
-	}
-
-	return pubMsg, intlMsg, nil
-}
-
 // newSalt generates a new salt of the specified size.
 func newSalt(rng io.Reader) ([group.SaltLen]byte, error) {
 	var salt [group.SaltLen]byte

groupChat/send_test.go

@@ -78,9 +78,9 @@ func TestGroup_newCmixMsg_SaltReaderError(t *testing.T) {
 	m, _ := newTestManager(t)
 
 	_, err := newCmixMsg(
-		gs.Group{ID: id.NewIdFromString("test", id.User, t)}, "",
-		[]byte{}, time.Time{}, group.Member{}, strings.NewReader(""),
-		m.getReceptionIdentity().ID, m.getCMix().GetMaxMessageLength())
+		gs.Group{ID: id.NewIdFromString("test", id.User, t)},
+		"", time.Time{}, group.Member{}, strings.NewReader(""),
+		m.getCMix().GetMaxMessageLength(), []byte("internal Message"))
 	if err == nil || !strings.Contains(err.Error(), expectedErr) {
 		t.Errorf("newCmixMsg failed to return the expected error"+
 			"\nexpected: %s\nreceived: %+v", expectedErr, err)
@@ -97,40 +97,16 @@ func TestGroup_newCmixMsg_InternalMsgSizeError(t *testing.T) {
 
 	// Create test parameters
 	testMsg := make([]byte, 1500)
-	mem := group.Member{ID: id.NewIdFromString("memberID", id.User, t)}
 
 	// Create cMix message
 	prng = rand.New(rand.NewSource(42))
-	_, err := newCmixMsg(g, "", testMsg, netTime.Now(), mem, prng,
-		m.getReceptionIdentity().ID, m.getCMix().GetMaxMessageLength())
+	_, _, err := m.newMessages(g, "", testMsg, netTime.Now())
 	if err == nil || !strings.Contains(err.Error(), expectedErr) {
 		t.Errorf("newCmixMsg failed to return the expected error"+
 			"\nexpected: %s\nreceived: %+v", expectedErr, err)
 	}
 }
 
-// Error path: payload size too small to fit publicMsg.
-func Test_newMessageParts_PublicMsgSizeErr(t *testing.T) {
-	expectedErr := strings.SplitN(newPublicMsgErr, "%", 2)[0]
-
-	_, _, err := newMessageParts(publicMinLen - 1)
-	if err == nil || !strings.Contains(err.Error(), expectedErr) {
-		t.Errorf("newMessageParts did not return the expected error."+
-			"\nexpected: %s\nreceived: %+v", expectedErr, err)
-	}
-}
-
-// Error path: payload size too small to fit internalMsg.
-func Test_newMessageParts_InternalMsgSizeErr(t *testing.T) {
-	expectedErr := strings.SplitN(newInternalMsgErr, "%", 2)[0]
-
-	_, _, err := newMessageParts(publicMinLen)
-	if err == nil || !strings.Contains(err.Error(), expectedErr) {
-		t.Errorf("newMessageParts did not return the expected error."+
-			"\nexpected: %s\nreceived: %+v", expectedErr, err)
-	}
-}
-
 // Tests the consistency of newSalt.
 func Test_newSalt_Consistency(t *testing.T) {
 	prng := rand.New(rand.NewSource(42))