address feedback

services/shhext/chat/multidevice/service.go

@@ -8,7 +8,9 @@ import (
 )
 
 type Installation struct {
-	ID      string
+	// The installation-id of the device
+	ID string
+	// The last known protocol version of the device
 	Version uint32
 }
 
@@ -46,7 +48,7 @@ func (s *Service) GetOurActiveInstallations(identity *ecdsa.PublicKey) ([]*Insta
 	if err != nil {
 		return nil, err
 	}
-	// Move to layer above
+
 	installations = append(installations, &Installation{
 		ID:      s.config.InstallationID,
 		Version: s.config.ProtocolVersion,

services/shhext/chat/multidevice/sql_lite_persistence.go

@@ -79,53 +79,52 @@ func (s *SQLLitePersistence) AddInstallations(identity []byte, timestamp int64,
 			return err
 		}
 
-		// We update timestamp if present without changing enabled, only if this is a new bundle
-		// and we set the version to the latest we ever saw
-		if err != sql.ErrNoRows {
-			if oldVersion > installation.Version {
-				latestVersion = oldVersion
-			}
-
-			stmt, err = tx.Prepare(`UPDATE installations
-					        SET timestamp = ?,  enabled = ?, version = ?
-						WHERE identity = ?
-						AND installation_id = ?
-						AND timestamp < ?`)
+		if err == sql.ErrNoRows {
+			stmt, err = tx.Prepare(`INSERT INTO installations(identity, installation_id, timestamp, enabled, version)
+						VALUES (?, ?, ?, ?, ?)`)
 			if err != nil {
 				return err
 			}
+			defer stmt.Close()
 
 			_, err = stmt.Exec(
-				timestamp,
-				oldEnabled,
-				latestVersion,
 				identity,
 				installation.ID,
 				timestamp,
+				defaultEnabled,
+				latestVersion,
 			)
 			if err != nil {
 				return err
 			}
-			defer stmt.Close()
-
 		} else {
-			stmt, err = tx.Prepare(`INSERT INTO installations(identity, installation_id, timestamp, enabled, version)
-						VALUES (?, ?, ?, ?, ?)`)
+			// We update timestamp if present without changing enabled, only if this is a new bundle
+			// and we set the version to the latest we ever saw
+			if oldVersion > installation.Version {
+				latestVersion = oldVersion
+			}
+
+			stmt, err = tx.Prepare(`UPDATE installations
+					        SET timestamp = ?,  enabled = ?, version = ?
+						WHERE identity = ?
+						AND installation_id = ?
+						AND timestamp < ?`)
 			if err != nil {
 				return err
 			}
+			defer stmt.Close()
 
 			_, err = stmt.Exec(
+				timestamp,
+				oldEnabled,
+				latestVersion,
 				identity,
 				installation.ID,
 				timestamp,
-				defaultEnabled,
-				latestVersion,
 			)
 			if err != nil {
 				return err
 			}
-			defer stmt.Close()
 		}
 
 	}

services/shhext/chat/multidevice/sql_lite_persistence_test.go

@@ -5,7 +5,7 @@ import (
 	"os"
 	"testing"
 
-	appDB "github.com/status-im/status-go/services/shhext/chat/db"
+	chatDB "github.com/status-im/status-go/services/shhext/chat/db"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -27,7 +27,7 @@ type SQLLitePersistenceTestSuite struct {
 func (s *SQLLitePersistenceTestSuite) SetupTest() {
 	os.Remove(dbPath)
 
-	db, err := appDB.Open(dbPath, "", 0)
+	db, err := chatDB.Open(dbPath, "", 0)
 	s.Require().NoError(err)
 
 	s.service = NewSQLLitePersistence(db)

services/shhext/chat/protocol.go

@@ -13,12 +13,13 @@ import (
 const ProtocolVersion = 1
 const sharedSecretNegotiationVersion = 1
 const partitionedTopicMinVersion = 1
+const defaultMinVersion = 0
 
 type PartitionTopic int
 
 const (
 	PartitionTopicNoSupport PartitionTopic = iota
-	PartitionTopicV1        PartitionTopic = iota
+	PartitionTopicV1
 )
 
 type ProtocolService struct {
@@ -92,7 +93,7 @@ type ProtocolMessageSpec struct {
 func (p *ProtocolMessageSpec) MinVersion() uint32 {
 
 	if len(p.Installations) == 0 {
-		return 0
+		return defaultMinVersion
 	}
 
 	version := p.Installations[0].Version
@@ -197,6 +198,7 @@ func (p *ProtocolService) ProcessPublicBundle(myIdentityKey *ecdsa.PrivateKey, b
 		return nil, err
 	}
 
+	p.log.Info("Processing bundle", "bundle", bundle)
 	theirIdentityKey, err := ExtractIdentity(bundle)
 	if err != nil {
 		return nil, err
@@ -305,7 +307,7 @@ func (p *ProtocolService) HandleMessage(myIdentityKey *ecdsa.PrivateKey, theirPu
 
 func getProtocolVersion(bundles []*protobuf.Bundle, installationID string) uint32 {
 	if installationID == "" {
-		return 0
+		return defaultMinVersion
 	}
 
 	for _, bundle := range bundles {
@@ -317,12 +319,12 @@ func getProtocolVersion(bundles []*protobuf.Bundle, installationID string) uint3
 
 			signedPreKey := signedPreKeys[installationID]
 			if signedPreKey == nil {
-				return 0
+				return defaultMinVersion
 			}
 
 			return signedPreKey.GetProtocolVersion()
 		}
 	}
 
-	return 0
+	return defaultMinVersion
 }

services/shhext/chat/sharedsecret/persistence.go

@@ -6,8 +6,12 @@ import (
 )
 
 type PersistenceService interface {
+	// Add adds a shared secret, associated with an identity and an installationID
 	Add(identity []byte, secret []byte, installationID string) error
+	// Get returns a shared secret associated with multiple installationIDs
 	Get(identity []byte, installationIDs []string) (*Response, error)
+	// All returns an array of shared secrets, each one of them represented
+	// as a byte array
 	All() ([][][]byte, error)
 }
 

services/shhext/chat/sharedsecret/service_test.go

@@ -6,7 +6,7 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/crypto"
-	appDB "github.com/status-im/status-go/services/shhext/chat/db"
+	chatDB "github.com/status-im/status-go/services/shhext/chat/db"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -25,7 +25,7 @@ func (s *ServiceTestSuite) SetupTest() {
 	s.Require().NoError(err)
 	s.path = dbFile.Name()
 
-	db, err := appDB.Open(s.path, "", 0)
+	db, err := chatDB.Open(s.path, "", 0)
 
 	s.Require().NoError(err)
 

services/shhext/chat/sql_lite_persistence.go

@@ -9,7 +9,7 @@ import (
 
 	dr "github.com/status-im/doubleratchet"
 	ecrypto "github.com/status-im/status-go/services/shhext/chat/crypto"
-	appDB "github.com/status-im/status-go/services/shhext/chat/db"
+	chatDB "github.com/status-im/status-go/services/shhext/chat/db"
 	"github.com/status-im/status-go/services/shhext/chat/multidevice"
 	"github.com/status-im/status-go/services/shhext/chat/protobuf"
 	"github.com/status-im/status-go/services/shhext/chat/sharedsecret"
@@ -92,7 +92,7 @@ func (s *SQLLitePersistence) GetMultideviceStorage() multidevice.Persistence {
 
 // Open opens a file at the specified path
 func (s *SQLLitePersistence) Open(path string, key string) error {
-	db, err := appDB.Open(path, key, appDB.KdfIterationsNumber)
+	db, err := chatDB.Open(path, key, chatDB.KdfIterationsNumber)
 	if err != nil {
 		return err
 	}

services/shhext/filter/service.go

@@ -195,15 +195,15 @@ func ContactCodeTopic(identity string) string {
 	return "0x" + identity + "-contact-code"
 }
 
-// Get returns a negotiated filter given an identity
+// Get returns a negotiated chat given an identity
 func (s *Service) GetNegotiated(identity *ecdsa.PublicKey) *Chat {
 	s.mutex.Lock()
 	defer s.mutex.Unlock()
 
 	return s.chats[negotiatedID(identity)]
 }
 
-// GetByID returns a filter by chatID
+// GetByID returns a chat by chatID
 func (s *Service) GetByID(chatID string) *Chat {
 	s.mutex.Lock()
 	defer s.mutex.Unlock()
@@ -217,7 +217,7 @@ func (s *Service) ProcessNegotiatedSecret(secret *sharedsecret.Secret) (*Chat, e
 	defer s.mutex.Unlock()
 
 	chatID := negotiatedID(secret.Identity)
-	// If we already have a filter do nothing
+	// If we already have a chat do nothing
 	if _, ok := s.chats[chatID]; ok {
 		return s.chats[chatID], nil
 	}

services/shhext/filter/service_test.go

@@ -8,7 +8,7 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/crypto"
-	appDB "github.com/status-im/status-go/services/shhext/chat/db"
+	chatDB "github.com/status-im/status-go/services/shhext/chat/db"
 	"github.com/status-im/status-go/services/shhext/chat/sharedsecret"
 	whisper "github.com/status-im/whisper/whisperv6"
 	"github.com/stretchr/testify/suite"
@@ -63,7 +63,7 @@ func (s *ServiceTestSuite) SetupTest() {
 		s.keys = append(s.keys, testKey)
 	}
 
-	db, err := appDB.Open(s.path, "", 0)
+	db, err := chatDB.Open(s.path, "", 0)
 	s.Require().NoError(err)
 
 	// Build services

services/shhext/publisher/service.go

@@ -10,7 +10,7 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/golang/protobuf/proto"
 	"github.com/status-im/status-go/services/shhext/chat"
-	appDB "github.com/status-im/status-go/services/shhext/chat/db"
+	chatDB "github.com/status-im/status-go/services/shhext/chat/db"
 	"github.com/status-im/status-go/services/shhext/chat/multidevice"
 	"github.com/status-im/status-go/services/shhext/chat/protobuf"
 	"github.com/status-im/status-go/services/shhext/chat/sharedsecret"
@@ -94,25 +94,25 @@ func (s *Service) initProtocol(address, encKey, password string) error {
 	v4Path := filepath.Join(s.config.DataDir, fmt.Sprintf("%s.v4.db", s.config.InstallationID))
 
 	if password != "" {
-		if err := appDB.MigrateDBFile(v0Path, v1Path, "ON", password); err != nil {
+		if err := chatDB.MigrateDBFile(v0Path, v1Path, "ON", password); err != nil {
 			return err
 		}
 
-		if err := appDB.MigrateDBFile(v1Path, v2Path, password, encKey); err != nil {
+		if err := chatDB.MigrateDBFile(v1Path, v2Path, password, encKey); err != nil {
 			// Remove db file as created with a blank password and never used,
 			// and there's no need to rekey in this case
 			os.Remove(v1Path)
 			os.Remove(v2Path)
 		}
 	}
 
-	if err := appDB.MigrateDBKeyKdfIterations(v2Path, v3Path, encKey); err != nil {
+	if err := chatDB.MigrateDBKeyKdfIterations(v2Path, v3Path, encKey); err != nil {
 		os.Remove(v2Path)
 		os.Remove(v3Path)
 	}
 
 	// Fix IOS not encrypting database
-	if err := appDB.EncryptDatabase(v3Path, v4Path, encKey); err != nil {
+	if err := chatDB.EncryptDatabase(v3Path, v4Path, encKey); err != nil {
 		os.Remove(v3Path)
 		os.Remove(v4Path)
 	}
@@ -326,8 +326,8 @@ func (s *Service) ProcessMessage(dedupMessage dedup.DeduplicateMessage) error {
 	return nil
 }
 
-// InitializeDirectMessage sends a 1:1 chat message to the underlying transport
-func (s *Service) InitializeDirectMessage(signature string, destination hexutil.Bytes, DH bool, payload []byte) (*whisper.NewMessage, error) {
+// CreateDirectMessage creates a 1:1 chat message
+func (s *Service) CreateDirectMessage(signature string, destination hexutil.Bytes, DH bool, payload []byte) (*whisper.NewMessage, error) {
 	if !s.config.PfsEnabled {
 		return nil, ErrPFSNotEnabled
 	}
@@ -404,8 +404,8 @@ func (s *Service) directMessageToWhisper(myPrivateKey *ecdsa.PrivateKey, theirPu
 	return &whisperMessage, nil
 }
 
-// InitializePublicMessage sends a public chat message to the underlying transport
-func (s *Service) InitializePublicMessage(signature string, chatID string, payload []byte, wrap bool) (*whisper.NewMessage, error) {
+// CreatePublicMessage sends a public chat message to the underlying transport
+func (s *Service) CreatePublicMessage(signature string, chatID string, payload []byte, wrap bool) (*whisper.NewMessage, error) {
 	if !s.config.PfsEnabled {
 		return nil, ErrPFSNotEnabled
 	}
@@ -511,7 +511,7 @@ func (s *Service) sendContactCode() (*whisper.NewMessage, error) {
 
 	identity := fmt.Sprintf("%x", crypto.FromECDSAPub(&privateKey.PublicKey))
 
-	message, err := s.InitializePublicMessage("0x"+identity, filter.ContactCodeTopic(identity), nil, true)
+	message, err := s.CreatePublicMessage("0x"+identity, filter.ContactCodeTopic(identity), nil, true)
 	if err != nil {
 		s.log.Error("could not build contact code", "identity", identity, "err", err)
 		return nil, err

services/shhext/publisher/service_test.go

@@ -115,8 +115,8 @@ func (s *ServiceTestSuite) SetupTest() {
 	s.bobKey = key2
 }
 
-func (s *ServiceTestSuite) TestInitializeDirectMessage() {
-	newMessage, err := s.alice.InitializeDirectMessage(s.aliceKey.keyID, s.bobKey.publicKeyBytes, false, []byte("hello"))
+func (s *ServiceTestSuite) TestCreateDirectMessage() {
+	newMessage, err := s.alice.CreateDirectMessage(s.aliceKey.keyID, s.bobKey.publicKeyBytes, false, []byte("hello"))
 	s.Require().NoError(err)
 
 	message := &whisper.Message{
@@ -138,7 +138,7 @@ func (s *ServiceTestSuite) TestInitializeDirectMessage() {
 
 func (s *ServiceTestSuite) TestTopic() {
 	// We build an initial message
-	newMessage1, err := s.alice.InitializeDirectMessage(s.aliceKey.keyID, s.bobKey.publicKeyBytes, false, []byte("hello"))
+	newMessage1, err := s.alice.CreateDirectMessage(s.aliceKey.keyID, s.bobKey.publicKeyBytes, false, []byte("hello"))
 	s.Require().NoError(err)
 
 	message1 := &whisper.Message{
@@ -173,7 +173,7 @@ func (s *ServiceTestSuite) TestTopic() {
 	s.Require().NoError(err)
 
 	// We build another message, this time it should use the partitioned topic
-	newMessage3, err := s.alice.InitializeDirectMessage(s.aliceKey.keyID, s.bobKey.publicKeyBytes, false, []byte("hello"))
+	newMessage3, err := s.alice.CreateDirectMessage(s.aliceKey.keyID, s.bobKey.publicKeyBytes, false, []byte("hello"))
 	s.Require().NoError(err)
 
 	message3 := &whisper.Message{
@@ -196,7 +196,7 @@ func (s *ServiceTestSuite) TestTopic() {
 	s.Require().NoError(err)
 
 	// We build another message, this time it should use the negotiated topic
-	newMessage4, err := s.bob.InitializeDirectMessage(s.bobKey.keyID, s.aliceKey.publicKeyBytes, false, []byte("hello"))
+	newMessage4, err := s.bob.CreateDirectMessage(s.bobKey.keyID, s.aliceKey.publicKeyBytes, false, []byte("hello"))
 	s.Require().NoError(err)
 
 	message4 := &whisper.Message{
@@ -226,7 +226,7 @@ func (s *ServiceTestSuite) TestTopic() {
 	s.Require().NoError(err)
 
 	// Alice sends another message to Bob, this time it should use the negotiated topic
-	newMessage5, err := s.alice.InitializeDirectMessage(s.aliceKey.keyID, s.bobKey.publicKeyBytes, false, []byte("hello"))
+	newMessage5, err := s.alice.CreateDirectMessage(s.aliceKey.keyID, s.bobKey.publicKeyBytes, false, []byte("hello"))
 	s.Require().NoError(err)
 
 	message5 := &whisper.Message{