Skip to content

Commit

Permalink
fix: proper usage of CreateAccount RootDataDir (#5284)
Browse files Browse the repository at this point in the history
* fix_: introduce CreateAccount.RootDataDir
* fix_: deprecate and remove ShhextConfig.BackupDisabledDataDir
  • Loading branch information
igor-sirotin committed Jun 5, 2024
1 parent 3906fcb commit a4f9d16
Show file tree
Hide file tree
Showing 20 changed files with 117 additions and 106 deletions.
2 changes: 1 addition & 1 deletion _assets/compose/wnode-test-cluster/wnode-config.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"NetworkId": 777,
"DataDir": "/data/ethereumtest/status",
"BackupDisabledDataDir": "/data/ethereumtest/status",
"RootDataDir": "/data/ethereumtest/status",
"KeyStoreDir": "/data/ethereumtest/keystore",
"NoDiscovery": false,
"Rendezvous": false,
Expand Down
100 changes: 62 additions & 38 deletions api/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -826,13 +826,13 @@ func TestLoginAccount(t *testing.T) {

b := NewGethStatusBackend()
createAccountRequest := &requests.CreateAccount{
DisplayName: "some-display-name",
CustomizationColor: "#ffffff",
Emoji: "some",
Password: password,
BackupDisabledDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
WakuV2Nameserver: &nameserver,
DisplayName: "some-display-name",
CustomizationColor: "#ffffff",
Emoji: "some",
Password: password,
RootDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
WakuV2Nameserver: &nameserver,
}
c := make(chan interface{}, 10)
signal.SetMobileSignalHandler(func(data []byte) {
Expand Down Expand Up @@ -1381,12 +1381,12 @@ func TestCreateWallet(t *testing.T) {

b := NewGethStatusBackend()
createAccountRequest := &requests.CreateAccount{
DisplayName: "some-display-name",
CustomizationColor: "#ffffff",
Emoji: "emoji",
Password: password,
BackupDisabledDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
DisplayName: "some-display-name",
CustomizationColor: "#ffffff",
Emoji: "emoji",
Password: password,
RootDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
}
c := make(chan interface{}, 10)
signal.SetMobileSignalHandler(func(data []byte) {
Expand Down Expand Up @@ -1440,12 +1440,12 @@ func TestSetFleet(t *testing.T) {

b := NewGethStatusBackend()
createAccountRequest := &requests.CreateAccount{
DisplayName: "some-display-name",
CustomizationColor: "#ffffff",
Password: password,
BackupDisabledDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
Emoji: "some",
DisplayName: "some-display-name",
CustomizationColor: "#ffffff",
Password: password,
RootDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
Emoji: "some",
}
c := make(chan interface{}, 10)
signal.SetMobileSignalHandler(func(data []byte) {
Expand Down Expand Up @@ -1509,12 +1509,12 @@ func TestWalletConfigOnLoginAccount(t *testing.T) {

b := NewGethStatusBackend()
createAccountRequest := &requests.CreateAccount{
DisplayName: "some-display-name",
CustomizationColor: "#ffffff",
Password: password,
BackupDisabledDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
Emoji: "some",
DisplayName: "some-display-name",
CustomizationColor: "#ffffff",
Password: password,
RootDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
Emoji: "some",
}
c := make(chan interface{}, 10)
signal.SetMobileSignalHandler(func(data []byte) {
Expand Down Expand Up @@ -1576,13 +1576,13 @@ func TestTestnetEnabledSettingOnCreateAccount(t *testing.T) {

// Creating an account with test networks enabled
createAccountRequest1 := &requests.CreateAccount{
DisplayName: "User-1",
CustomizationColor: "#ffffff",
Emoji: "some",
Password: "password123",
BackupDisabledDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
TestNetworksEnabled: true,
DisplayName: "User-1",
CustomizationColor: "#ffffff",
Emoji: "some",
Password: "password123",
RootDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
TestNetworksEnabled: true,
}
_, err := b.CreateAccountAndLogin(createAccountRequest1)
require.NoError(t, err)
Expand All @@ -1597,12 +1597,12 @@ func TestTestnetEnabledSettingOnCreateAccount(t *testing.T) {

// Creating an account with test networks disabled
createAccountRequest2 := &requests.CreateAccount{
DisplayName: "User-2",
CustomizationColor: "#ffffff",
Emoji: "some",
Password: "password",
BackupDisabledDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
DisplayName: "User-2",
CustomizationColor: "#ffffff",
Emoji: "some",
Password: "password",
RootDataDir: tmpdir,
LogFilePath: tmpdir + "/log",
}
_, err = b.CreateAccountAndLogin(createAccountRequest2)
require.NoError(t, err)
Expand All @@ -1615,3 +1615,27 @@ func TestTestnetEnabledSettingOnCreateAccount(t *testing.T) {

require.NoError(t, b.Logout())
}

func TestCreateAccountPathsValidation(t *testing.T) {
tmpdir := t.TempDir()

validation := &requests.CreateAccountValidation{
AllowEmptyDisplayName: false,
}

request := &requests.CreateAccount{
DisplayName: "User-1",
Password: "password123",
CustomizationColor: "#ffffff",
RootDataDir: tmpdir,
}

err := request.Validate(validation)
require.NoError(t, err)

request.RootDataDir = ""
request.BackupDisabledDataDir = tmpdir
err = request.Validate(validation)
require.NoError(t, err)
require.Equal(t, tmpdir, request.RootDataDir)
}
2 changes: 1 addition & 1 deletion api/create_account_and_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestCreateAccountAndLogin(t *testing.T) {
"wakuV2Nameserver":"1.1.1.1",
"mnemonic":null,
"verifyENSContractAddress":"0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e",
"backupDisabledDataDir":"%s",
"rootDataDir":"%s",
"password":"0x20756cad9b728c8225fd8cedb6badaf8731e174506950219ea657cd54f35f46c",
"displayName":"%s",
"logEnabled":true,
Expand Down
1 change: 0 additions & 1 deletion api/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ func defaultNodeConfig(installationID string, request *requests.CreateAccount, o
}

nodeConfig.ShhextConfig = params.ShhextConfig{
BackupDisabledDataDir: request.BackupDisabledDataDir,
InstallationID: installationID,
MaxMessageDeliveryAttempts: DefaultMaxMessageDeliveryAttempts,
MailServerConfirmations: true,
Expand Down
16 changes: 12 additions & 4 deletions api/geth_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"sync"
"time"

"go.uber.org/zap"

"github.com/status-im/status-go/services/ens"
"github.com/status-im/status-go/sqlite"

Expand Down Expand Up @@ -1305,13 +1307,12 @@ func (b *GethStatusBackend) GetKeyUIDByMnemonic(mnemonic string) (string, error)
func (b *GethStatusBackend) generateOrImportAccount(mnemonic string, customizationColorClock uint64, fetchBackup bool, request *requests.CreateAccount, opts ...params.Option) (*multiaccounts.Account, error) {
keystoreDir := keystoreRelativePath

b.UpdateRootDataDir(request.BackupDisabledDataDir)
b.UpdateRootDataDir(request.RootDataDir)
err := b.OpenAccounts()
if err != nil {
b.log.Error("failed open accounts", err)
return nil, err
}

accountGenerator := b.accountManager.AccountsGenerator()

var info generator.GeneratedAccountInfo
Expand Down Expand Up @@ -1751,13 +1752,20 @@ func (b *GethStatusBackend) loadNodeConfig(inputNodeCfg *params.NodeConfig) erro
conf.Version = params.Version
conf.RootDataDir = b.rootDataDir
conf.DataDir = filepath.Join(b.rootDataDir, conf.DataDir)
conf.ShhextConfig.BackupDisabledDataDir = filepath.Join(b.rootDataDir, conf.ShhextConfig.BackupDisabledDataDir)
conf.KeyStoreDir = filepath.Join(b.rootDataDir, conf.KeyStoreDir)

if _, err = os.Stat(conf.RootDataDir); os.IsNotExist(err) {
if err := os.MkdirAll(conf.RootDataDir, os.ModePerm); err != nil {
b.log.Warn("failed to create data directory", zap.Error(err))
return err
}
}

if len(conf.LogDir) == 0 {
conf.LogFile = filepath.Join(b.rootDataDir, conf.LogFile)
} else {
conf.LogFile = filepath.Join(conf.LogDir, conf.LogFile)
}
conf.KeyStoreDir = filepath.Join(b.rootDataDir, conf.KeyStoreDir)

b.config = conf

Expand Down
4 changes: 2 additions & 2 deletions api/messenger_raw_message_resend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (s *MessengerRawMessageResendTest) createBackendAndLogin(backend **GethStat
s.Require().NoError(err)
}

func (s *MessengerRawMessageResendTest) setCreateAccountRequest(displayName, backupDisabledDataDir, logFilePath string) *requests.CreateAccount {
func (s *MessengerRawMessageResendTest) setCreateAccountRequest(displayName, rootDataDir, logFilePath string) *requests.CreateAccount {
nameServer := "1.1.1.1"
verifyENSContractAddress := "0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e"
verifyTransactionChainID := int64(1)
Expand All @@ -187,7 +187,7 @@ func (s *MessengerRawMessageResendTest) setCreateAccountRequest(displayName, bac
UpstreamConfig: verifyURL,
WakuV2Nameserver: &nameServer,
VerifyENSContractAddress: &verifyENSContractAddress,
BackupDisabledDataDir: backupDisabledDataDir,
RootDataDir: rootDataDir,
Password: password,
DisplayName: displayName,
LogEnabled: true,
Expand Down
1 change: 0 additions & 1 deletion appdatabase/node_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ func randomNodeConfig() *params.NodeConfig {
},
ShhextConfig: params.ShhextConfig{
PFSEnabled: randomBool(),
BackupDisabledDataDir: randomString(),
InstallationID: randomString(),
MailServerConfirmations: randomBool(),
EnableConnectionManager: randomBool(),
Expand Down
1 change: 0 additions & 1 deletion cmd/ping-community/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,6 @@ func defaultNodeConfig(installationID string) (*params.NodeConfig, error) {
}

nodeConfig.ShhextConfig = params.ShhextConfig{
BackupDisabledDataDir: "",
InstallationID: installationID,
MaxMessageDeliveryAttempts: api.DefaultMaxMessageDeliveryAttempts,
MailServerConfirmations: true,
Expand Down
1 change: 0 additions & 1 deletion cmd/populate-db/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,6 @@ func defaultNodeConfig(installationID string) (*params.NodeConfig, error) {
}

nodeConfig.ShhextConfig = params.ShhextConfig{
BackupDisabledDataDir: "",
InstallationID: installationID,
MaxMessageDeliveryAttempts: api.DefaultMaxMessageDeliveryAttempts,
MailServerConfirmations: true,
Expand Down
1 change: 0 additions & 1 deletion cmd/spiff-workflow/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,6 @@ func defaultNodeConfig(installationID string) (*params.NodeConfig, error) {
}

nodeConfig.ShhextConfig = params.ShhextConfig{
BackupDisabledDataDir: "",
InstallationID: installationID,
MaxMessageDeliveryAttempts: api.DefaultMaxMessageDeliveryAttempts,
MailServerConfirmations: true,
Expand Down
12 changes: 6 additions & 6 deletions cmd/status-cli/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ func start(cCtx *cli.Context, name string, port int, apiModules string) (*Status
backend := api.NewGethStatusBackend()

createAccountRequest := &requests.CreateAccount{
DisplayName: name,
CustomizationColor: "#ffffff",
Emoji: "some",
Password: "some-password",
BackupDisabledDataDir: fmt.Sprintf("./test-%s", strings.ToLower(name)),
LogFilePath: "log",
DisplayName: name,
CustomizationColor: "#ffffff",
Emoji: "some",
Password: "some-password",
RootDataDir: fmt.Sprintf("./test-%s", strings.ToLower(name)),
LogFilePath: "log",
APIConfig: &requests.APIConfig{
APIModules: apiModules,
HTTPHost: "127.0.0.1",
Expand Down
10 changes: 5 additions & 5 deletions nodecfg/node_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,13 @@ func insertLightETHConfig(tx *sql.Tx, c *params.NodeConfig) error {
func insertShhExtConfig(tx *sql.Tx, c *params.NodeConfig) error {
_, err := tx.Exec(`
INSERT OR REPLACE INTO shhext_config (
pfs_enabled, backup_disabled_data_dir, installation_id, mailserver_confirmations, enable_connection_manager,
pfs_enabled, installation_id, mailserver_confirmations, enable_connection_manager,
enable_last_used_monitor, connection_target, request_delay, max_server_failures, max_message_delivery_attempts,
whisper_cache_dir, disable_generic_discovery_topic, send_v1_messages, data_sync_enabled, verify_transaction_url,
verify_ens_url, verify_ens_contract_address, verify_transaction_chain_id, anon_metrics_server_enabled,
anon_metrics_send_id, anon_metrics_server_postgres_uri, bandwidth_stats_enabled, synthetic_id
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'id')`,
c.ShhextConfig.PFSEnabled, c.ShhextConfig.BackupDisabledDataDir, c.ShhextConfig.InstallationID, c.ShhextConfig.MailServerConfirmations, c.ShhextConfig.EnableConnectionManager,
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'id')`,
c.ShhextConfig.PFSEnabled, c.ShhextConfig.InstallationID, c.ShhextConfig.MailServerConfirmations, c.ShhextConfig.EnableConnectionManager,
c.ShhextConfig.EnableLastUsedMonitor, c.ShhextConfig.ConnectionTarget, c.ShhextConfig.RequestsDelay, c.ShhextConfig.MaxServerFailures, c.ShhextConfig.MaxMessageDeliveryAttempts,
c.ShhextConfig.WhisperCacheDir, c.ShhextConfig.DisableGenericDiscoveryTopic, c.ShhextConfig.SendV1Messages, c.ShhextConfig.DataSyncEnabled, c.ShhextConfig.VerifyTransactionURL,
c.ShhextConfig.VerifyENSURL, c.ShhextConfig.VerifyENSContractAddress, c.ShhextConfig.VerifyTransactionChainID, c.ShhextConfig.AnonMetricsServerEnabled,
Expand Down Expand Up @@ -627,13 +627,13 @@ func loadNodeConfig(tx *sql.Tx) (*params.NodeConfig, error) {
}

err = tx.QueryRow(`
SELECT pfs_enabled, backup_disabled_data_dir, installation_id, mailserver_confirmations, enable_connection_manager,
SELECT pfs_enabled, installation_id, mailserver_confirmations, enable_connection_manager,
enable_last_used_monitor, connection_target, request_delay, max_server_failures, max_message_delivery_attempts,
whisper_cache_dir, disable_generic_discovery_topic, send_v1_messages, data_sync_enabled, verify_transaction_url,
verify_ens_url, verify_ens_contract_address, verify_transaction_chain_id, anon_metrics_server_enabled,
anon_metrics_send_id, anon_metrics_server_postgres_uri, bandwidth_stats_enabled FROM shhext_config WHERE synthetic_id = 'id'
`).Scan(
&nodecfg.ShhextConfig.PFSEnabled, &nodecfg.ShhextConfig.BackupDisabledDataDir, &nodecfg.ShhextConfig.InstallationID, &nodecfg.ShhextConfig.MailServerConfirmations, &nodecfg.ShhextConfig.EnableConnectionManager,
&nodecfg.ShhextConfig.PFSEnabled, &nodecfg.ShhextConfig.InstallationID, &nodecfg.ShhextConfig.MailServerConfirmations, &nodecfg.ShhextConfig.EnableConnectionManager,
&nodecfg.ShhextConfig.EnableLastUsedMonitor, &nodecfg.ShhextConfig.ConnectionTarget, &nodecfg.ShhextConfig.RequestsDelay, &nodecfg.ShhextConfig.MaxServerFailures, &nodecfg.ShhextConfig.MaxMessageDeliveryAttempts,
&nodecfg.ShhextConfig.WhisperCacheDir, &nodecfg.ShhextConfig.DisableGenericDiscoveryTopic, &nodecfg.ShhextConfig.SendV1Messages, &nodecfg.ShhextConfig.DataSyncEnabled, &nodecfg.ShhextConfig.VerifyTransactionURL,
&nodecfg.ShhextConfig.VerifyENSURL, &nodecfg.ShhextConfig.VerifyENSContractAddress, &nodecfg.ShhextConfig.VerifyTransactionChainID, &nodecfg.ShhextConfig.AnonMetricsServerEnabled,
Expand Down
13 changes: 3 additions & 10 deletions params/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"crypto/ecdsa"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net/url"
Expand Down Expand Up @@ -603,8 +602,6 @@ type PushNotificationServerConfig struct {
// ShhextConfig defines options used by shhext service.
type ShhextConfig struct {
PFSEnabled bool
// BackupDisabledDataDir is the file system folder the node should use for any data storage needs that it doesn't want backed up.
BackupDisabledDataDir string
// InstallationId id of the current installation
InstallationID string
// MailServerConfirmations should be true if client wants to receive confirmatons only from a selected mail servers.
Expand Down Expand Up @@ -683,9 +680,6 @@ func (c *ShhextConfig) Validate(validate *validator.Validate) error {
if err := validate.Struct(c); err != nil {
return err
}
if c.PFSEnabled && len(c.BackupDisabledDataDir) == 0 {
return errors.New("field BackupDisabledDataDir is required if PFSEnabled is true")
}
return nil
}

Expand Down Expand Up @@ -873,6 +867,7 @@ func NewNodeConfig(dataDir string, networkID uint64) (*NodeConfig, error) {

config := &NodeConfig{
NetworkID: networkID,
RootDataDir: dataDir,
DataDir: dataDir,
KeyStoreDir: keyStoreDir,
KeycardPairingDataFile: keycardPairingDataFile,
Expand Down Expand Up @@ -907,10 +902,8 @@ func NewNodeConfig(dataDir string, networkID uint64) (*NodeConfig, error) {
DataDir: wakuV2Dir,
MaxMessageSize: wakuv2common.DefaultMaxMessageSize,
},
ShhextConfig: ShhextConfig{
BackupDisabledDataDir: dataDir,
},
SwarmConfig: SwarmConfig{},
ShhextConfig: ShhextConfig{},
SwarmConfig: SwarmConfig{},
TorrentConfig: TorrentConfig{
Enabled: false,
Port: 9025,
Expand Down
Loading

0 comments on commit a4f9d16

Please sign in to comment.