Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix_: restore node config #6270

Draft
wants to merge 1 commit into
base: fix/v1_upgrading
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions api/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,6 @@ func DefaultNodeConfig(installationID string, request *requests.CreateAccount, o
nodeConfig.ShhextConfig.VerifyENSContractAddress = *request.VerifyENSContractAddress
}

if request.NetworkID != nil {
nodeConfig.NetworkID = *request.NetworkID
}

nodeConfig.TorrentConfig = params.TorrentConfig{
Enabled: false,
Port: 0,
Expand Down
95 changes: 95 additions & 0 deletions api/geth_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/status-im/status-go/appdatabase"
"github.com/status-im/status-go/centralizedmetrics"
centralizedmetricscommon "github.com/status-im/status-go/centralizedmetrics/common"
d_common "github.com/status-im/status-go/common"
"github.com/status-im/status-go/common/dbsetup"
"github.com/status-im/status-go/connection"
"github.com/status-im/status-go/eth-node/crypto"
Expand Down Expand Up @@ -579,6 +580,95 @@ func (b *GethStatusBackend) LoginAccount(request *requests.Login) error {
return b.LoggedIn(request.KeyUID, err)
}

func (b *GethStatusBackend) workaroundToFixBadMigration(request *requests.Login) (err error) {
if !d_common.IsMobilePlatform() { // this issue only happens on mobile platform
return nil
}

b.mu.Lock()
defer b.mu.Unlock()

var (
currentConf *params.NodeConfig
defaultNodeConf *params.NodeConfig
)
currentConf, err = nodecfg.GetNodeConfigFromDB(b.appDB)
if err != nil {
return err
}

// check if we saved a empty node config because of node config migration failed
if currentConf.NetworkID == 0 &&
currentConf.KeyStoreDir == "" &&
currentConf.DataDir == "" &&
currentConf.NodeKey == "" {
Comment on lines +601 to +604
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem is that the node_config is empty in the database, but... why is this a problem? Can't we just use the default values, without writing them to the database?

Perhaps this is also related (and could be mitigated by) #5597. It seems that loadNodeConfig actually gives priority to DB values rather than the values passed with CreateAccount/LoginAccount/.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem is that the node_config is empty in the database, but... why is this a problem? Can't we just use the default values, without writing them to the database?

default values with backend are not enough, we don't have following values:

  • verifyTransactionURL
  • verifyENSURL
  • verifyENSContractAddress
  • verifyTransactionChainID
    They're needed to pass from frontend currently.

Perhaps this is also related (and could be mitigated by) #5597.

it's not related :)

It seems that loadNodeConfig actually gives priority to DB values rather than the values passed with CreateAccount/LoginAccount

loadNodeConfig support passing inputNodeCfg which will override values from DB

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default values with backend are not enough, we don't have following values:

  • verifyTransactionURL
  • verifyENSURL
  • verifyENSContractAddress
  • verifyTransactionChainID
  • They're needed to pass from frontend currently.

@friofry will we need these parameters to be passed from the client after merging #6178 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that these fields are related :) Initially I wanted to answer that my pr is related to another table in the database. But then I found this line working as a fallback:

nodeConfig.ShhextConfig.VerifyENSURL = mainnet(request.WalletSecretsConfig.StatusProxyStageName).FallbackURL

And this field will be removed soon, as well as Status Proxy will eventually be replaced by Smart Proxy. Which will always be in-memory and won't depend on the database.

also loadNodeConfig is really fragile and complex. We probably need to collect all the issues and rework them all.

// check if exist old node config
oldNodeConf := &params.NodeConfig{}
err = b.appDB.QueryRow("SELECT node_config FROM settings WHERE synthetic_id = 'id'").Scan(&sqlite.JSONBlob{Data: oldNodeConf})
if err != nil && err != sql.ErrNoRows {
return err
}
if err == sql.ErrNoRows {
return errors.New("failed to migrate node config as there's no data in settings")
}

// the createAccount contains all the fields that are needed to create the default node config
createAccount := b.convertLoginRequestToAccountRequest(request)
defaultNodeConf, err = DefaultNodeConfig(oldNodeConf.ShhextConfig.InstallationID, createAccount)
if err != nil {
return err
}

b.overridePartialWithOldNodeConfig(defaultNodeConf, oldNodeConf)
var tx *sql.Tx
tx, err = b.appDB.BeginTx(context.Background(), &sql.TxOptions{})
if err != nil {
return err
}
defer func() {
if err == nil {
err = tx.Commit()
return
}
// don't shadow original error
_ = tx.Rollback()
}()
err = nodecfg.SaveConfigWithTx(tx, defaultNodeConf)
}

return nil
}

func (b *GethStatusBackend) overridePartialWithOldNodeConfig(conf *params.NodeConfig, oldNodeConf *params.NodeConfig) {
// rootDataDir should be set by InitializeApplication or UpdateRootDataDir already
conf.RootDataDir = b.rootDataDir
conf.LogEnabled = oldNodeConf.LogEnabled
conf.LogFile = oldNodeConf.LogFile
conf.LogDir = oldNodeConf.LogDir
conf.LogLevel = oldNodeConf.LogLevel
conf.DataDir = oldNodeConf.DataDir
conf.KeyStoreDir = oldNodeConf.KeyStoreDir
conf.NodeKey = oldNodeConf.NodeKey
conf.RegisterTopics = oldNodeConf.RegisterTopics
conf.RequireTopics = oldNodeConf.RequireTopics
}

func (b *GethStatusBackend) convertLoginRequestToAccountRequest(loginRequest *requests.Login) *requests.CreateAccount {
createAccount := &requests.CreateAccount{}
createAccount.WalletSecretsConfig = loginRequest.WalletSecretsConfig
createAccount.StatusProxyEnabled = loginRequest.StatusProxyEnabled
createAccount.WakuV2Nameserver = &loginRequest.WakuV2Nameserver
createAccount.WakuV2LightClient = loginRequest.WakuV2LightClient
createAccount.WakuV2EnableMissingMessageVerification = loginRequest.WakuV2EnableMissingMessageVerification
createAccount.WakuV2EnableStoreConfirmationForMessagesSent = loginRequest.WakuV2EnableStoreConfirmationForMessagesSent
createAccount.TelemetryServerURL = loginRequest.TelemetryServerURL
createAccount.VerifyTransactionURL = loginRequest.VerifyTransactionURL
createAccount.VerifyENSURL = loginRequest.VerifyENSURL
createAccount.VerifyTransactionChainID = loginRequest.VerifyTransactionChainID
createAccount.VerifyENSContractAddress = loginRequest.VerifyENSContractAddress
return createAccount
}

func (b *GethStatusBackend) loginAccount(request *requests.Login) error {
if err := request.Validate(); err != nil {
return err
Expand Down Expand Up @@ -613,6 +703,11 @@ func (b *GethStatusBackend) loginAccount(request *requests.Login) error {
return errors.Wrap(err, "failed to open database")
}

//relate PR: https://github.com/status-im/status-go/pull/6248
if err := b.workaroundToFixBadMigration(request); err != nil {
return errors.Wrap(err, "failed to workaround bad migration")
}

defaultCfg := &params.NodeConfig{
// why we need this? relate PR: https://github.com/status-im/status-go/pull/4014
KeycardPairingDataFile: DefaultKeycardPairingDataFile,
Expand Down
42 changes: 35 additions & 7 deletions api/old_mobile_v1_user_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@ import (
"github.com/stretchr/testify/suite"
"go.uber.org/zap"

"github.com/status-im/status-go/common"
"github.com/status-im/status-go/protocol/requests"
"github.com/status-im/status-go/t/utils"
)

const (
v1_10_keyUID = "0x88f310d80e3d5821c00714c52bf4fae15f571ba5abae6d804b1e8a9723136a9c"
v1_10_passwd = "0x20756cad9b728c8225fd8cedb6badaf8731e174506950219ea657cd54f35f46c" // #nosec G101
v1_10_srcFolder = "../static/test-mobile-release-1.10.1"
v1_10_keyUID = "0x88f310d80e3d5821c00714c52bf4fae15f571ba5abae6d804b1e8a9723136a9c"
v1_10_passwd = "0x20756cad9b728c8225fd8cedb6badaf8731e174506950219ea657cd54f35f46c" // #nosec G101
v1_10_BeforeUpgradeFolder = "../static/test-mobile-release-1.10.1/before-upgrade-to-v2.30.0"
// This folder is used to test the case where the node config migration failed
// and the user logs in again after the failure
v1_10_AfterUpgradeFolder = "../static/test-mobile-release-1.10.1/after-upgrade-to-v2.30.0"
)

type OldMobileV1_10_UserLoginTest struct {
Expand All @@ -24,9 +28,6 @@ type OldMobileV1_10_UserLoginTest struct {

func (s *OldMobileV1_10_UserLoginTest) SetupTest() {
utils.Init()
s.tmpdir = s.T().TempDir()
copyDir(v1_10_srcFolder, s.tmpdir, s.T())

var err error
s.logger, err = zap.NewDevelopment()
s.Require().NoError(err)
Expand All @@ -36,7 +37,10 @@ func TestOldMobileV1_10_UserLogin(t *testing.T) {
suite.Run(t, new(OldMobileV1_10_UserLoginTest))
}

func (s *OldMobileV1_10_UserLoginTest) TestLogin() {
func (s *OldMobileV1_10_UserLoginTest) TestLoginWithSuccessNodeConfigMigration() {
s.tmpdir = s.T().TempDir()
copyDir(v1_10_BeforeUpgradeFolder, s.tmpdir, s.T())

b := NewGethStatusBackend(s.logger)
b.UpdateRootDataDir(s.tmpdir)
s.Require().NoError(b.OpenAccounts())
Expand All @@ -47,3 +51,27 @@ func (s *OldMobileV1_10_UserLoginTest) TestLogin() {
s.Require().NoError(b.LoginAccount(loginRequest))
s.Require().NoError(b.Logout())
}

// without workaroundToFixBadMigration, this test would login fail
func (s *OldMobileV1_10_UserLoginTest) TestLoginWithFailNodeConfigMigration() {
bkFunc := common.IsMobilePlatform
common.IsMobilePlatform = func() bool {
return true
}
defer func() {
common.IsMobilePlatform = bkFunc
}()

s.tmpdir = s.T().TempDir()
copyDir(v1_10_AfterUpgradeFolder, s.tmpdir, s.T())

b := NewGethStatusBackend(s.logger)
b.UpdateRootDataDir(s.tmpdir)
s.Require().NoError(b.OpenAccounts())
loginRequest := &requests.Login{
KeyUID: v1_10_keyUID,
Password: v1_10_passwd,
}
err := b.LoginAccount(loginRequest)
s.Require().NoError(err)
}
10 changes: 10 additions & 0 deletions protocol/requests/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ type Login struct {

APIConfig *APIConfig `json:"apiConfig"`
StatusProxyEnabled bool `json:"statusProxyEnabled"`

// Following fields are used for migration from old node config to new one
WakuV2LightClient bool `json:"wakuV2LightClient"`
WakuV2EnableStoreConfirmationForMessagesSent bool `json:"wakuV2EnableStoreConfirmationForMessagesSent"`
WakuV2EnableMissingMessageVerification bool `json:"wakuV2EnableMissingMessageVerification"`
TelemetryServerURL string `json:"telemetryServerURL"`
VerifyTransactionURL *string `json:"verifyTransactionURL"`
VerifyENSURL *string `json:"verifyENSURL"`
VerifyENSContractAddress *string `json:"verifyENSContractAddress"`
VerifyTransactionChainID *int64 `json:"verifyTransactionChainID"`
}

func (c *Login) Validate() error {
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The account data files in this directory are generated with android release apk v1.10.1.
It's used to test whether older status-go accounts can log in with the latest version of status-go.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
37d8e2a10dc12253a48ca9fe8af4edd3176b76563b2ce2a17eaa3c65e2b15a35
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
MANIFEST-000000
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=============== Jan 10, 2025 (UTC) ===============
08:28:04.874809 log@legend F·NumFile S·FileSize N·Entry C·BadEntry B·BadBlock Ke·KeyError D·DroppedEntry L·Level Q·SeqNum T·TimeElapsed
08:28:04.876561 db@open opening
08:28:04.877163 version@stat F·[] S·0B[] Sc·[]
08:28:04.881631 db@janitor F·2 G·0
08:28:04.881703 db@open done T·5.131458ms
08:45:33.946633 db@close closing
08:45:33.947813 db@close done T·1.201375ms
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
MANIFEST-000000
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=============== Jan 10, 2025 (UTC) ===============
08:28:04.869465 log@legend F·NumFile S·FileSize N·Entry C·BadEntry B·BadBlock Ke·KeyError D·DroppedEntry L·Level Q·SeqNum T·TimeElapsed
08:28:04.871618 db@open opening
08:28:04.872157 version@stat F·[] S·0B[] Sc·[]
08:28:04.873259 db@janitor F·2 G·0
08:28:04.873299 db@open done T·1.661375ms
08:45:33.947909 db@close closing
08:45:33.948202 db@close done T·290µs
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"address":"a64e3fa6f903bcecb15626b86e7ebb23a1f51637","crypto":{"cipher":"aes-128-ctr","ciphertext":"2be636220812a32406bd788693186e4cc10389a6dd0c9219f19c0d8549df29fe","cipherparams":{"iv":"b9bde28266ec3c6cbdd261fa291fc6c7"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"24d55064449be0b55db1540fd617db259ba157633d564929305d32844732e265"},"mac":"97fe060748cc119a9bacb877d614043ee587e672ad3b03243a012e2f508d431a"},"id":"079a7e5f-b2a5-4db5-9609-90559bd4e76d","version":3,"extendedkey":{"cipher":"aes-128-ctr","ciphertext":"9d5b24438b27be2efd7bf124d8a171b7936d1c36ab70ded50e912d8e40719b895538875391963948eaee256c92c4fa78df31b8dc119365698a8a0567c01f218a3cf1959fc4139af53279563816921d9665f7846de14272d3ec31306ffccc6fa9c894843578f7808b0379031572a958","cipherparams":{"iv":"1736cb5848feb3276c1086a6b0ed3a8c"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"5661a7fed8196676e860c67ea8cbd98b23bb43601d4b3145ec8c3272fbb09666"},"mac":"d47edca606e668ddb92135cd14bac6eda65a0207837a2435d621562cd6075f6b"},"subaccountindex":0}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"address":"33ec2bc1dafd01868ef846747cc639a22f9f9ac8","crypto":{"cipher":"aes-128-ctr","ciphertext":"3d70719bd93c15c1617c81cacbc50cb2d968b0ec19494711d3a9716493589a06","cipherparams":{"iv":"ad346e2abd8e18877868a27202db56b8"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"e7ec638ee76bd15a8407385f0e0db09e2f543d8fe51224a914d2be05739b53da"},"mac":"eb9c2442ace5cd28b51bf066e6780ab98750a34ad6736f0154ef660ed45ef084"},"id":"7fa6c3fb-c79a-453c-a387-51e44be5ff73","version":3,"extendedkey":{"cipher":"aes-128-ctr","ciphertext":"50fd960987eccef995ab5dba9515254c565690452cae6fc2d4d36fa7ef79487b6e49b6c8b4fd940306c32e47c18beb7b4cf9f43c223808fcc580ec2bc6048397e03d014569e88ea17cc32009761c73d96e1faf2a80388ae4ca6fdd3c39135d42de3e23809e83fc87b8150294ef9ea7","cipherparams":{"iv":"da011707bdfe431903bf2d42ad2a5440"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"b3f7656df7e7bef77f002b5d31a0d0549ffa4139aba108ff855aa66b1301bffd"},"mac":"7444f44a3c9a50c0bf52dba24b1b516c790e6f7b28b718d14540f1dece477862"},"subaccountindex":0}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"address":"0e90e6bbe03c68b570c3a12bf327e31d590c262a","crypto":{"cipher":"aes-128-ctr","ciphertext":"c5fedc26b5a36a77dc4406799de1188dd7da6ff7dbda232d312ab098342fdeac","cipherparams":{"iv":"9436848e6e9431f5381a16b059ef674c"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"a88217ef4e823cdbcd08c773dca37a0ac3989729ff75916526a41120a16f81c8"},"mac":"ef9729843b99edbc762c2f19f4af52ea55c7083884ba93ba7727f71f677c4f55"},"id":"14dba39a-2ffe-4f2b-973b-4b7dfcfa33ef","version":3,"extendedkey":{"cipher":"aes-128-ctr","ciphertext":"87e76dd157cd81e134692df6c62159e9c290647fc7cf634b7f7dacff3da0f766c2fe08f96533dd124ed7e9e50ba033b3d4e7f236b95f95a01839a3654b32e562414dc70c006cda5c4f7394bc3b9b3dbde2d36b52a21f77c0de590e22292c310fff1f91d0abdd8453b85ff913f76b99","cipherparams":{"iv":"7699daf7f3f3e924217cabbd454055c6"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"18a0192018134313395f590f2f8e715f54d6680187af59a9de1644a2593b4cba"},"mac":"cb7db464595b921b2f0fb4b158e1aceedef8df0acce51b70b5ea5c4032b4b56a"},"subaccountindex":0}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"address":"38fb3454a695aa1ba434604b550d9a439f764346","crypto":{"cipher":"aes-128-ctr","ciphertext":"7a5671cade118faf184a1a162ab906c260f63e7f45ba1a678bec3626b56ac10d","cipherparams":{"iv":"ae5c7a380054eef5ddad4046ca1ade9b"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"b825bebb3977671dd42844dbe6d81984e0e721cb7dd23d444ce392411a2172c7"},"mac":"ad55f72fc4702af1473d7da13e8dd0f69added33412e867818bd289aef503569"},"id":"b49c1e46-cb86-451b-bde7-bb10481c6919","version":3,"extendedkey":{"cipher":"aes-128-ctr","ciphertext":"eb5029c62a9d66bbf15b3c3f12e5d76f676c92cf3a1b33a1413cba72dbfcf4dd093b96eabbf2d8c07fb95a530dc3e78ebef9e0786b9c2fd9bd9d0dbe24c5010d868904b74f8d02e7cd34a37f56195c85cea1566f0e95aaabf24d83ad7b606bce6b0fbabae84898fd3b468bdf0dd067","cipherparams":{"iv":"42bbdebb5b7bb8f6c598ec24f189ab43"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":4096,"p":6,"r":8,"salt":"49271401aedc577b17ef864340782727c642589cb6003421364520d068dd6b0f"},"mac":"490e845666b3e32af69ffd7cd1eb212120f6ee2dfb809f2b679d5415d53d2efc"},"subaccountindex":0}
Empty file.
Loading