Added functionality to local pairing that makes client calls (more) idempotent

VERSION

@@ -1 +1 @@
-0.163.11
+0.163.12

api/geth_backend.go

@@ -12,10 +12,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/status-im/status-go/common/dbsetup"
-	"github.com/status-im/status-go/images"
-	"github.com/status-im/status-go/walletdatabase"
-
 	"github.com/imdario/mergo"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -27,9 +23,11 @@ import (
 	"github.com/status-im/status-go/account"
 	"github.com/status-im/status-go/account/generator"
 	"github.com/status-im/status-go/appdatabase"
+	"github.com/status-im/status-go/common/dbsetup"
 	"github.com/status-im/status-go/connection"
 	"github.com/status-im/status-go/eth-node/crypto"
 	"github.com/status-im/status-go/eth-node/types"
+	"github.com/status-im/status-go/images"
 	"github.com/status-im/status-go/logutils"
 	"github.com/status-im/status-go/multiaccounts"
 	"github.com/status-im/status-go/multiaccounts/accounts"
@@ -43,6 +41,7 @@ import (
 	"github.com/status-im/status-go/protocol/identity/colorhash"
 	"github.com/status-im/status-go/protocol/requests"
 	"github.com/status-im/status-go/rpc"
+	"github.com/status-im/status-go/server/pairing/statecontrol"
 	"github.com/status-im/status-go/services/ext"
 	"github.com/status-im/status-go/services/personal"
 	"github.com/status-im/status-go/services/rpcfilters"
@@ -50,6 +49,7 @@ import (
 	"github.com/status-im/status-go/signal"
 	"github.com/status-im/status-go/sqlite"
 	"github.com/status-im/status-go/transactions"
+	"github.com/status-im/status-go/walletdatabase"
 )
 
 var (
@@ -81,18 +81,18 @@ type GethStatusBackend struct {
 	walletDB    *sql.DB
 	config      *params.NodeConfig
 
-	statusNode           *node.StatusNode
+	statusNode               *node.StatusNode
-	personalAPI          *personal.PublicAPI
+	personalAPI              *personal.PublicAPI
-	multiaccountsDB      *multiaccounts.Database
+	multiaccountsDB          *multiaccounts.Database
-	account              *multiaccounts.Account
+	account                  *multiaccounts.Account
-	accountManager       *account.GethManager
+	accountManager           *account.GethManager
-	transactor           *transactions.Transactor
+	transactor               *transactions.Transactor
-	connectionState      connection.State
+	connectionState          connection.State
-	appState             appState
+	appState                 appState
-	selectedAccountKeyID string
+	selectedAccountKeyID     string
-	log                  log.Logger
+	log                      log.Logger
-	allowAllRPC          bool // used only for tests, disables api method restrictions
+	allowAllRPC              bool // used only for tests, disables api method restrictions
-	localPairing         bool // used to disable login/logout signalling
+	LocalPairingStateManager *statecontrol.ProcessStateManager
 }
 
 // NewGethStatusBackend create a new GethStatusBackend instance
@@ -116,7 +116,8 @@ func (b *GethStatusBackend) initialize() {
 	b.personalAPI = personalAPI
 	b.statusNode.SetMultiaccountsDB(b.multiaccountsDB)
 	b.log = log.New("package", "status-go/api.GethStatusBackend")
-	b.localPairing = false
+	b.LocalPairingStateManager = new(statecontrol.ProcessStateManager)
+	b.LocalPairingStateManager.SetPairing(false)
 }
 
 // StatusNode returns reference to node manager
@@ -493,7 +494,7 @@ func (b *GethStatusBackend) StartNodeWithKey(acc multiaccounts.Account, password
 		return err
 	}
 	// get logged in
-	if !b.localPairing {
+	if !b.LocalPairingStateManager.IsPairing() {
 		return b.LoggedIn(acc.KeyUID, err)
 	}
 	return nil
@@ -761,7 +762,7 @@ func (b *GethStatusBackend) StartNodeWithAccount(acc multiaccounts.Account, pass
 		_ = b.StopNode()
 	}
 	// get logged in
-	if !b.localPairing {
+	if !b.LocalPairingStateManager.IsPairing() {
 		return b.LoggedIn(acc.KeyUID, err)
 	}
 	return err
@@ -1659,7 +1660,7 @@ func (b *GethStatusBackend) stopNode() error {
 	if b.statusNode == nil || !b.IsNodeRunning() {
 		return nil
 	}
-	if !b.localPairing {
+	if !b.LocalPairingStateManager.IsPairing() {
 		defer signal.SendNodeStopped()
 	}
 
@@ -2267,10 +2268,6 @@ func (b *GethStatusBackend) SwitchFleet(fleet string, conf *params.NodeConfig) e
 	return nil
 }
 
-func (b *GethStatusBackend) SetLocalPairing(value bool) {
-	b.localPairing = value
-}
-
 func (b *GethStatusBackend) getAppDBPath(keyUID string) string {
 	return filepath.Join(b.rootDataDir, fmt.Sprintf("%s-v4.db", keyUID))
 }

mobile/status.go

@@ -9,8 +9,6 @@ import (
 	"sync"
 	"unsafe"
 
-	"github.com/status-im/status-go/logutils"
-
 	validator "gopkg.in/go-playground/validator.v9"
 
 	"github.com/ethereum/go-ethereum/log"
@@ -27,6 +25,7 @@ import (
 	"github.com/status-im/status-go/exportlogs"
 	"github.com/status-im/status-go/extkeys"
 	"github.com/status-im/status-go/images"
+	"github.com/status-im/status-go/logutils"
 	"github.com/status-im/status-go/multiaccounts"
 	"github.com/status-im/status-go/multiaccounts/accounts"
 	"github.com/status-im/status-go/multiaccounts/settings"
@@ -1051,9 +1050,9 @@ func GetConnectionStringForBeingBootstrapped(configJSON string) string {
 		return makeJSONResponse(fmt.Errorf("no config given, PayloadSourceConfig is expected"))
 	}
 
-	statusBackend.SetLocalPairing(true)
+	statusBackend.LocalPairingStateManager.SetPairing(true)
 	defer func() {
-		statusBackend.SetLocalPairing(false)
+		statusBackend.LocalPairingStateManager.SetPairing(false)
 	}()
 
 	cs, err := pairing.StartUpReceiverServer(statusBackend, configJSON)
@@ -1080,9 +1079,9 @@ func GetConnectionStringForBootstrappingAnotherDevice(configJSON string) string
 		return makeJSONResponse(fmt.Errorf("no config given, SendingServerConfig is expected"))
 	}
 
-	statusBackend.SetLocalPairing(true)
+	statusBackend.LocalPairingStateManager.SetPairing(true)
 	defer func() {
-		statusBackend.SetLocalPairing(false)
+		statusBackend.LocalPairingStateManager.SetPairing(false)
 	}()
 
 	cs, err := pairing.StartUpSenderServer(statusBackend, configJSON)
@@ -1101,22 +1100,23 @@ func GetConnectionStringForBootstrappingAnotherDevice(configJSON string) string
 // Example: A mobile device (device with a camera) receiving account data from
 // a device with a screen (mobile or desktop devices)
 func InputConnectionStringForBootstrapping(cs, configJSON string) string {
+	var err error
 	if configJSON == "" {
 		return makeJSONResponse(fmt.Errorf("no config given, ReceiverClientConfig is expected"))
 	}
 
-	statusBackend.SetLocalPairing(true)
+	err = statusBackend.LocalPairingStateManager.StartPairing(cs)
-	defer func() {
+	defer func() { statusBackend.LocalPairingStateManager.StopPairing(cs, err) }()
-		statusBackend.SetLocalPairing(false)
-	}()
-
-	err := pairing.StartUpReceivingClient(statusBackend, cs, configJSON)
 	if err != nil {
 		return makeJSONResponse(err)
 	}
 
-	err = statusBackend.Logout()
+	err = pairing.StartUpReceivingClient(statusBackend, cs, configJSON)
-	return makeJSONResponse(err)
+	if err != nil {
+		return makeJSONResponse(err)
+	}
+
+	return makeJSONResponse(statusBackend.Logout())
 }
 
 // InputConnectionStringForBootstrappingAnotherDevice starts a pairing.SendingClient
@@ -1127,16 +1127,18 @@ func InputConnectionStringForBootstrapping(cs, configJSON string) string {
 //
 // Example: A mobile (device with camera) sending account data to a desktop device (device without camera)
 func InputConnectionStringForBootstrappingAnotherDevice(cs, configJSON string) string {
+	var err error
 	if configJSON == "" {
 		return makeJSONResponse(fmt.Errorf("no config given, SenderClientConfig is expected"))
 	}
 
-	statusBackend.SetLocalPairing(true)
+	err = statusBackend.LocalPairingStateManager.StartPairing(cs)
-	defer func() {
+	defer func() { statusBackend.LocalPairingStateManager.StopPairing(cs, err) }()
-		statusBackend.SetLocalPairing(false)
+	if err != nil {
-	}()
+		return makeJSONResponse(err)
+	}
 
-	err := pairing.StartUpSendingClient(statusBackend, cs, configJSON)
+	err = pairing.StartUpSendingClient(statusBackend, cs, configJSON)
 	return makeJSONResponse(err)
 }
 

server/pairing/statecontrol/state_management.go

@@ -0,0 +1,93 @@
+package statecontrol
+
+// TODO refactor into the pairing package once the backend dependencies have been removed.
+
+import (
+	"fmt"
+	"sync"
+)
+
+var (
+	ErrProcessStateManagerAlreadyPairing = fmt.Errorf("cannot start new LocalPairing session, already pairing")
+	ErrProcessStateManagerAlreadyPaired  = func(sessionName string) error {
+		return fmt.Errorf("given connection string already successfully used '%s'", sessionName)
+	}
+)
+
+// ProcessStateManager represents a g
+type ProcessStateManager struct {
+	pairing     bool
+	pairingLock sync.Mutex
+
+	// sessions represents a map[string]bool:
+	// where string is a ConnectionParams string and bool is the transfer success state of that connection string
+	sessions sync.Map
+}
+
+// IsPairing returns if the ProcessStateManager is currently in pairing mode
+func (psm *ProcessStateManager) IsPairing() bool {
+	psm.pairingLock.Lock()
+	defer psm.pairingLock.Unlock()
+	return psm.pairing
+}
+
+// SetPairing sets the ProcessStateManager pairing state
+func (psm *ProcessStateManager) SetPairing(state bool) {
+	psm.pairingLock.Lock()
+	defer psm.pairingLock.Unlock()
+	psm.pairing = state
+}
+
+// RegisterSession stores a sessionName with the default false value.
+// In practice a sessionName will be a ConnectionParams string provided by the server mode device.
+// The boolean value represents whether the ConnectionParams string session resulted in a successful transfer.
+func (psm *ProcessStateManager) RegisterSession(sessionName string) {
+	psm.sessions.Store(sessionName, false)
+}
+
+// CompleteSession updates a transfer session with a given transfer success state only if the session is registered.
+func (psm *ProcessStateManager) CompleteSession(sessionName string) {
+	r, c := psm.GetSession(sessionName)
+	if r && !c {
+		psm.sessions.Store(sessionName, true)
+	}
+}
+
+// GetSession returns two booleans for a given sessionName.
+// These represent if the sessionName has been registered and if it has resulted in a successful transfer
+func (psm *ProcessStateManager) GetSession(sessionName string) (bool, bool) {
+	completed, registered := psm.sessions.Load(sessionName)
+	if !registered {
+		return registered, false
+	}
+	return registered, completed.(bool)
+}
+
+// StartPairing along with StopPairing are the core functions of the ProcessStateManager
+// This function takes a sessionName, which in practice is a ConnectionParams string, and attempts to init pairing state management.
+// The function will return an error if the ProcessStateManager is already currently pairing or if the sessionName was previously successful.
+func (psm *ProcessStateManager) StartPairing(sessionName string) error {
+	if psm.IsPairing() {
+		return ErrProcessStateManagerAlreadyPairing
+	}
+
+	registered, completed := psm.GetSession(sessionName)
+	if completed {
+		return ErrProcessStateManagerAlreadyPaired(sessionName)
+	}
+	if !registered {
+		psm.RegisterSession(sessionName)
+	}
+
+	psm.SetPairing(true)
+	return nil
+}
+
+// StopPairing takes a sessionName and an error, if the error is nil the sessionName will be recorded as successful
+// the pairing state of the ProcessStateManager is set to false.
+func (psm *ProcessStateManager) StopPairing(sessionName string, err error) {
+	if err == nil {
+		psm.CompleteSession(sessionName)
+	}
+	psm.SetPairing(false)
+}

server/pairing/statecontrol/state_management_test.go

@@ -0,0 +1,58 @@
+package statecontrol
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	cs  = "cs2:4FHRnp:Q4:uqnn"
+	cs1 = cs + "1234"
+	cs2 = cs + "qwer"
+)
+
+func TestProcessStateManager_StartPairing(t *testing.T) {
+	psm := new(ProcessStateManager)
+
+	// A new psm should start with no error
+	err := psm.StartPairing(cs)
+	require.NoError(t, err)
+
+	// A started psm should return an ErrProcessStateManagerAlreadyPairing if another start is attempted
+	err = psm.StartPairing(cs)
+	require.EqualError(t, err, ErrProcessStateManagerAlreadyPairing.Error())
+
+	// A psm should start without error if the pairing process has been stopped with an error
+	psm.StopPairing(cs, err)
+	err = psm.StartPairing(cs)
+	require.NoError(t, err)
+
+	// A psm should return an error if starting with a conn string that previously succeeded (a nil error)
+	psm.StopPairing(cs, nil)
+	err = psm.StartPairing(cs)
+	require.EqualError(t, err, ErrProcessStateManagerAlreadyPaired(cs).Error())
+
+	// A psm should be able to start with a new connection string if the psm has been stopped
+	err = psm.StartPairing(cs1)
+	require.NoError(t, err)
+
+	// A started psm should return an ErrProcessStateManagerAlreadyPairing if another start is attempted
+	err = psm.StartPairing(cs1)
+	require.EqualError(t, err, ErrProcessStateManagerAlreadyPairing.Error())
+
+	// A started psm should return an ErrProcessStateManagerAlreadyPairing if another start is attempted regardless of
+	// the given connection string.
+	err = psm.StartPairing(cs2)
+	require.EqualError(t, err, ErrProcessStateManagerAlreadyPairing.Error())
+
+	// A psm should start without error if the pairing process has been stopped with an error
+	psm.StopPairing(cs1, err)
+	err = psm.StartPairing(cs2)
+	require.NoError(t, err)
+
+	// A psm should return an error if starting with a conn string that previously succeeded (a nil error)
+	psm.StopPairing(cs2, nil)
+	err = psm.StartPairing(cs2)
+	require.EqualError(t, err, ErrProcessStateManagerAlreadyPaired(cs2).Error())
+}