NodeConfig, make sure that absolute paths are saved
Problem
in some places DataDir is expected to be relative path
loadNodeConfig:
conf.RootDataDir = b.rootDataDir
conf.DataDir = filepath.Join(b.rootDataDir, conf.DataDir)
conf.KeyStoreDir = filepath.Join(b.rootDataDir, conf.KeyStoreDir)
or
func defaultNodeConfig(installationID string) (*params.NodeConfig, error) {
...
nodeConfig.DataDir = api.DefaultDataDir
}
and other palces expect absolute paths:
NewNodeConfig:
config := &NodeConfig{
NetworkID: networkID,
RootDataDir: dataDir,
DataDir: dataDir,
We need to make sure that all paths in NodeConfig are stored in absolute form (or if that's not possible, relative, but it should be consistent).
And the code that concatenates absolute paths should be fixed conf.DataDir = filepath.Join(b.rootDataDir, conf.DataDir)
(it was found while I was debugging tests in https://github.com/status-im/status-go/pull/5527)
Implementation
DataDir, KeyStoreDir, RootDataDir, LogDir, LogFile should be absolute
Functions like getDefaultDataDir should be used correctly. And names of fields and methods should reflect the path type (perhaps it makes sense to introduce a wrapper type for FilePath to support both forms).
Acceptance Criteria
See status-im/status-react#9942
[status-im/status-react#9942] Upgradable paths in configs
Storing absolute path for different configs breaks compatibility on iOS
as app's dir is changed after upgrade. The solution is to store relative
paths and to concatenate it with `backend.rootDataDir`. The only
exception is `LogFile` as it is stored outside `backend.rootDataDir` on
Android. `LogDir` config was added to allow adding of custom dir for log
file.
Configs concerned:
`DataDir`
`LogDir`
`LogFile`
`KeystoreDir`
`BackupDisabledDataDir`
We need to make sure that mobile handles this correctly.
[status-im/status-react#9942] Upgradable paths in configs
Storing absolute path for different configs breaks compatibility on iOS
as app's dir is changed after upgrade. The solution is to store relative
paths and to concatenate it with `backend.rootDataDir`. The only
exception is `LogFile` as it is stored outside `backend.rootDataDir` on
Android. `LogDir` config was added to allow adding of custom dir for log
file.
Configs concerned:
`DataDir`
`LogDir`
`LogFile`
`KeystoreDir`
`BackupDisabledDataDir`
thanks for the info, so I think our only solution would be make sure that all paths in NodeConfig are stored in relative path? and this was also my old understanding, but I didn't expect it was not consistent 🙂 @friofry
I stumbled upon the same issue today. NodeConfig handling is far from ideal. I came up with an ad-hoc solution for my problem, which I believe will work for this issue as well: store only relative paths and add methods that return concatenated paths. Example from my changes:
diff --git a/api/geth_backend.go b/api/geth_backend.go
index b0580a0de..6873ad77d 100644
--- a/api/geth_backend.go
+++ b/api/geth_backend.go
@@ -2079,12 +2079,6 @@ func (b *GethStatusBackend) loadNodeConfig(inputNodeCfg *params.NodeConfig) erro
}
}
- if len(conf.LogDir) == 0 {
- conf.LogFile = filepath.Join(b.rootDataDir, conf.LogFile)
- } else {
- conf.LogFile = filepath.Join(conf.LogDir, conf.LogFile)
- }
-
b.config = conf
if inputNodeCfg != nil && inputNodeCfg.RuntimeLogLevel != "" {
diff --git a/params/config.go b/params/config.go
index ca3ee5bf9..77d0a1b92 100644
--- a/params/config.go
+++ b/params/config.go
@@ -1196,12 +1196,19 @@ func LesTopic(netid int) string {
}
}
+func (c *NodeConfig) LogFilePath() string {
+ if len(c.LogDir) == 0 {
+ return filepath.Join(c.RootDataDir, c.LogFile)
+ }
+ return filepath.Join(c.LogDir, c.LogFile)
+}
+
func (c *NodeConfig) LogSettings() logutils.LogSettings {
return logutils.LogSettings{
Enabled: c.LogEnabled,
Level: c.LogLevel,
Namespaces: c.LogNamespaces,
- File: c.LogFile,
+ File: c.LogFilePath(),
MaxSize: c.LogMaxSize,
MaxBackups: c.LogMaxBackups,
CompressRotated: c.LogCompressRotated,