Address feedback from code review and add quoted image

VERSION

@@ -1 +1 @@
-0.53.2
+0.54.0

params/config.go

@@ -82,6 +82,7 @@ type WhisperConfig struct {
 	DataDir string
 
 	// MinimumPoW minimum PoW for Whisper messages
+	// We enforce a minimum as a bland spam prevention mechanism.
 	MinimumPoW float64
 
 	// MailServerPassword for symmetric encryption of whisper message history requests.
@@ -145,14 +146,15 @@ type WakuConfig struct {
 	// EnableMailServer is mode when node is capable of delivering expired messages on demand
 	EnableMailServer bool
 
-	// DataDir is the file system folder Whisper should use for any data storage needs.
+	// DataDir is the file system folder Waku should use for any data storage needs.
 	// For instance, MailServer will use this directory to store its data.
 	DataDir string
 
-	// MinimumPoW minimum PoW for Whisper messages
+	// MinimumPoW minimum PoW for Waku messages
+	// We enforce a minimum as a bland spam prevention mechanism.
 	MinimumPoW float64
 
-	// MailServerPassword for symmetric encryption of whisper message history requests.
+	// MailServerPassword for symmetric encryption of waku message history requests.
 	// (if no account file selected, then this password is used for symmetric encryption).
 	MailServerPassword string
 
@@ -165,7 +167,7 @@ type WakuConfig struct {
 	// TTL time to live for messages, in seconds
 	TTL int
 
-	// MaxMessageSize is a maximum size of a devp2p packet handled by the Whisper protocol,
+	// MaxMessageSize is a maximum size of a devp2p packet handled by the Waku protocol,
 	// not only the size of envelopes sent in that packet.
 	MaxMessageSize uint32
 
@@ -603,13 +605,13 @@ func (c *NodeConfig) UpdateWithMobileDefaults() {
 		c.APIModules = "net,web3,eth"
 	}
 
-	// Override defaultMinPoW passed by the mobile
+	// Override defaultMinPoW passed by the client
 	if c.WakuConfig.Enabled {
 		c.WakuConfig.MinimumPoW = WakuMinimumPoW
 	}
 
 	if c.WhisperConfig.Enabled {
-		c.WakuConfig.MinimumPoW = WhisperMinimumPoW
+		c.WhisperConfig.MinimumPoW = WhisperMinimumPoW
 	}
 
 }

params/config_test.go

@@ -47,8 +47,6 @@ func TestNewNodeConfigWithDefaults(t *testing.T) {
 	// assert peers limits
 	assert.Contains(t, c.RequireTopics, params.WhisperDiscv5Topic)
 	assert.Contains(t, c.RequireTopics, discv5.Topic(params.LesTopic(int(c.NetworkID))))
-	// assert incentivisation
-	assert.Equal(t, false, c.IncentivisationConfig.Enabled)
 	// assert other
 	assert.Equal(t, false, c.HTTPEnabled)
 	assert.Equal(t, false, c.IPCEnabled)
@@ -376,103 +374,6 @@ func TestNodeConfigValidate(t *testing.T) {
 			}`,
 			Error: "field BackupDisabledDataDir is required if PFSEnabled is true",
 		},
-		{
-			Name: "Valid JSON config with incentivisation",
-			Config: `{
-				"NetworkId": 1,
-				"DataDir": "/tmp/data",
-				"BackupDisabledDataDir": "/tmp/data",
-				"KeyStoreDir": "/tmp/data",
-				"NoDiscovery": true,
-				"IncentivisationConfig": {
-				  "Enabled": true,
-				  "IP": "127.0.0.1",
-				  "Port": 300,
-				  "RPCEndpoint": "http://test.com",
-				  "ContractAddress": "0xfffff"
-				}
-			}`,
-		},
-		{
-			Name: "Missing RPCEndpoint",
-			Config: `{
-				"NetworkId": 1,
-				"DataDir": "/tmp/data",
-				"BackupDisabledDataDir": "/tmp/data",
-				"KeyStoreDir": "/tmp/data",
-				"NoDiscovery": true,
-				"IncentivisationConfig": {
-				  "Enabled": true,
-				  "IP": "127.0.0.1",
-				  "Port": 300,
-				  "ContractAddress": "0xfffff"
-				}
-			}`,
-			FieldErrors: map[string]string{
-				"RPCEndpoint": "required",
-			},
-			Error: "RPCEndpoint is required if incentivisation is enabled",
-		},
-		{
-			Name: "Missing contract address",
-			Config: `{
-				"NetworkId": 1,
-				"DataDir": "/tmp/data",
-				"BackupDisabledDataDir": "/tmp/data",
-				"KeyStoreDir": "/tmp/data",
-				"NoDiscovery": true,
-				"IncentivisationConfig": {
-				  "Enabled": true,
-				  "IP": "127.0.0.1",
-				  "Port": 300,
-				  "RPCEndpoint": "http://test.com"
-				}
-			}`,
-			FieldErrors: map[string]string{
-				"ContractAddress": "required",
-			},
-			Error: "field ContractAddress is required if incentivisation is enabled",
-		},
-		{
-			Name: "Missing ip address",
-			Config: `{
-				"NetworkId": 1,
-				"DataDir": "/tmp/data",
-				"BackupDisabledDataDir": "/tmp/data",
-				"KeyStoreDir": "/tmp/data",
-				"NoDiscovery": true,
-				"IncentivisationConfig": {
-				  "Enabled": true,
-				  "Port": 300,
-				  "RPCEndpoint": "http://test.com",
-				  "ContractAddress": "0xfffff"
-				}
-			}`,
-			FieldErrors: map[string]string{
-				"IP": "required",
-			},
-			Error: "field IP is required if incentivisation is enabled",
-		},
-		{
-			Name: "Missing port",
-			Config: `{
-				"NetworkId": 1,
-				"DataDir": "/tmp/data",
-				"BackupDisabledDataDir": "/tmp/data",
-				"KeyStoreDir": "/tmp/data",
-				"NoDiscovery": true,
-				"IncentivisationConfig": {
-				  "Enabled": true,
-				  "IP": "127.0.0.1",
-				  "RPCEndpoint": "http://test.com",
-				  "ContractAddress": "0xfffff"
-				}
-			}`,
-			FieldErrors: map[string]string{
-				"Port": "required",
-			},
-			Error: "field Port is required if incentivisation is enabled",
-		},
 		{
 			Name:   "Missing APIModules",
 			Config: `{"NetworkId": 1, "DataDir": "/tmp/data", "KeyStoreDir": "/tmp/data", "APIModules" :""}`,

params/defaults.go

@@ -22,12 +22,14 @@ const (
 	DefaultGas = 180000
 
 	// WhisperMinimumPoW amount of work for Whisper message to be added to sending queue
+	// We enforce a minimum as a bland spam prevention mechanism.
 	WhisperMinimumPoW = 0.000002
 
 	// WhisperTTL is time to live for messages, in seconds
 	WhisperTTL = 120
 
 	// WakuMinimumPoW amount of work for Whisper message to be added to sending queue
+	// We enforce a minimum as a bland spam prevention mechanism.
 	WakuMinimumPoW = 0.000002
 
 	// WakuTTL is time to live for messages, in seconds

protocol/images/type.go

@@ -29,15 +29,16 @@ func webp(buf []byte) bool {
 }
 
 func ImageType(buf []byte) protobuf.ImageMessage_ImageType {
-	if jpeg(buf) {
+	switch {
+	case jpeg(buf):
 		return protobuf.ImageMessage_JPEG
-	} else if png(buf) {
+	case png(buf):
 		return protobuf.ImageMessage_PNG
-	} else if gif(buf) {
+	case gif(buf):
 		return protobuf.ImageMessage_GIF
-	} else if webp(buf) {
+	case webp(buf):
 		return protobuf.ImageMessage_WEBP
+	default:
+		return protobuf.ImageMessage_UNKNOWN_IMAGE_TYPE
 	}
-
-	return protobuf.ImageMessage_UNKNOWN_IMAGE_TYPE
 }

protocol/message.go

@@ -20,6 +20,8 @@ type QuotedMessage struct {
 	// From is a public key of the author of the message.
 	From string `json:"from"`
 	Text string `json:"text"`
+	// Base64Image is the converted base64 image
+	Base64Image string `json:"image,omitempty"`
 }
 
 type CommandState int
@@ -239,19 +241,11 @@ func (m *Message) parseImage() error {
 	encBuf := make([]byte, maxEncLen)
 
 	e64.Encode(encBuf, payload)
-	var mime string
 
-	switch image.Type {
+	mime, err := getImageMessageMIME(image)
-	case protobuf.ImageMessage_PNG:
+
-		mime = "png"
+	if err != nil {
-	case protobuf.ImageMessage_JPEG:
+		return err
-		mime = "jpeg"
-	case protobuf.ImageMessage_WEBP:
-		mime = "webp"
-	case protobuf.ImageMessage_GIF:
-		mime = "gif"
-	default:
-		return errors.New("image format not supported")
 	}
 
 	m.Base64Image = fmt.Sprintf("data:image/%s;base64,%s", mime, encBuf)
@@ -272,3 +266,17 @@ func (m *Message) PrepareContent() error {
 	m.RTL = isRTL(m.Text)
 	return m.parseImage()
 }
+
+func getImageMessageMIME(i *protobuf.ImageMessage) (string, error) {
+	switch i.Type {
+	case protobuf.ImageMessage_PNG:
+		return "png", nil
+	case protobuf.ImageMessage_JPEG:
+		return "jpeg", nil
+	case protobuf.ImageMessage_WEBP:
+		return "webp", nil
+	case protobuf.ImageMessage_GIF:
+		return "gif", nil
+	}
+	return "", errors.New("image format not supported")
+}

protocol/message_persistence.go

@@ -82,6 +82,7 @@ func (db sqlitePersistence) tableUserMessagesAllFieldsJoin() string {
 		m1.response_to,
 		m2.source,
 		m2.text,
+		m2.image_base64,
 		c.alias,
 		c.identicon`
 }
@@ -97,6 +98,7 @@ type scanner interface {
 func (db sqlitePersistence) tableUserMessagesScanAllFields(row scanner, message *Message, others ...interface{}) error {
 	var quotedText sql.NullString
 	var quotedFrom sql.NullString
+	var quotedImage sql.NullString
 	var alias sql.NullString
 	var identicon sql.NullString
 
@@ -135,6 +137,7 @@ func (db sqlitePersistence) tableUserMessagesScanAllFields(row scanner, message
 		&message.ResponseTo,
 		&quotedFrom,
 		&quotedText,
+		&quotedImage,
 		&alias,
 		&identicon,
 	}
@@ -145,8 +148,9 @@ func (db sqlitePersistence) tableUserMessagesScanAllFields(row scanner, message
 
 	if quotedText.Valid {
 		message.QuotedMessage = &QuotedMessage{
-			From: quotedFrom.String,
+			From:        quotedFrom.String,
-			Text: quotedText.String,
+			Text:        quotedText.String,
+			Base64Image: quotedImage.String,
 		}
 	}
 	message.Alias = alias.String

protocol/message_processor.go

@@ -435,8 +435,8 @@ func messageSpecToWhisper(spec *encryption.ProtocolMessageSpec) (*types.NewMessa
 }
 
 // calculatePoW returns the PoWTarget to be used.
-// We check the size and arbitrarely set it to a lower PoW if the packet is
+// We check the size and arbitrarily set it to a lower PoW if the packet is
-// greater than 50KB. We do this as the defaultPoW is to high for clients to send
+// greater than 50KB. We do this as the defaultPoW is too high for clients to send
 // large messages.
 func calculatePoW(payload []byte) float64 {
 	if len(payload) > largeSizeInBytes {

protocol/message_test.go

@@ -31,3 +31,29 @@ func TestPrepareContentImage(t *testing.T) {
 	require.NoError(t, message.PrepareContent())
 	require.Equal(t, message.Base64Image, expectedJPEG)
 }
+
+func TestGetImageMessageMIME(t *testing.T) {
+	jpeg := &protobuf.ImageMessage{Type: protobuf.ImageMessage_JPEG}
+	mime, err := getImageMessageMIME(jpeg)
+	require.NoError(t, err)
+	require.Equal(t, "jpeg", mime)
+
+	png := &protobuf.ImageMessage{Type: protobuf.ImageMessage_PNG}
+	mime, err = getImageMessageMIME(png)
+	require.NoError(t, err)
+	require.Equal(t, "png", mime)
+
+	webp := &protobuf.ImageMessage{Type: protobuf.ImageMessage_WEBP}
+	mime, err = getImageMessageMIME(webp)
+	require.NoError(t, err)
+	require.Equal(t, "webp", mime)
+
+	gif := &protobuf.ImageMessage{Type: protobuf.ImageMessage_GIF}
+	mime, err = getImageMessageMIME(gif)
+	require.NoError(t, err)
+	require.Equal(t, "gif", mime)
+
+	unknown := &protobuf.ImageMessage{Type: protobuf.ImageMessage_UNKNOWN_IMAGE_TYPE}
+	_, err = getImageMessageMIME(unknown)
+	require.Error(t, err)
+}