Skip to content

node: warn when using deprecated config/resource files #18199

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 7, 2018
Merged

node: warn when using deprecated config/resource files #18199

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 35 additions & 9 deletions node/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"path/filepath"
"runtime"
"strings"
"sync"

"github.com/ethereum/go-ethereum/accounts"
"github.com/ethereum/go-ethereum/accounts/keystore"
Expand Down Expand Up @@ -152,6 +153,10 @@ type Config struct {

// Logger is a custom logger to use with the p2p.Server.
Logger log.Logger `toml:",omitempty"`

staticNodesWarning bool
trustedNodesWarning bool
oldGethResourceWarning bool
Copy link
Member

Choose a reason for hiding this comment

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

Why not use sync.Once for these? Would be cleaner imho and would get rid of the global lock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this, but it's not a good idea because some code shallow-copies Config structs. That doesn't work with sync.Once, it contains sync.Mutex.

}

// IPCEndpoint resolves an IPC endpoint based on a configured value, taking into
Expand Down Expand Up @@ -263,8 +268,8 @@ var isOldGethResource = map[string]bool{
"chaindata": true,
"nodes": true,
"nodekey": true,
"static-nodes.json": true,
"trusted-nodes.json": true,
"static-nodes.json": false, // no warning for these because they have their
"trusted-nodes.json": false, // own separate warning.
}

// ResolvePath resolves path in the instance directory.
Expand All @@ -277,13 +282,15 @@ func (c *Config) ResolvePath(path string) string {
}
// Backwards-compatibility: ensure that data directory files created
// by geth 1.4 are used if they exist.
if c.name() == "geth" && isOldGethResource[path] {
if warn, isOld := isOldGethResource[path]; isOld {
oldpath := ""
if c.Name == "geth" {
if c.name() == "geth" {
oldpath = filepath.Join(c.DataDir, path)
}
if oldpath != "" && common.FileExist(oldpath) {
// TODO: print warning
if warn {
c.warnOnce(&c.oldGethResourceWarning, "Using deprecated resource file %s, please move this file to the 'geth' subdirectory of datadir.", oldpath)
}
return oldpath
}
}
Expand Down Expand Up @@ -337,28 +344,30 @@ func (c *Config) NodeKey() *ecdsa.PrivateKey {

// StaticNodes returns a list of node enode URLs configured as static nodes.
func (c *Config) StaticNodes() []*enode.Node {
return c.parsePersistentNodes(c.ResolvePath(datadirStaticNodes))
return c.parsePersistentNodes(&c.staticNodesWarning, c.ResolvePath(datadirStaticNodes))
}

// TrustedNodes returns a list of node enode URLs configured as trusted nodes.
func (c *Config) TrustedNodes() []*enode.Node {
return c.parsePersistentNodes(c.ResolvePath(datadirTrustedNodes))
return c.parsePersistentNodes(&c.trustedNodesWarning, c.ResolvePath(datadirTrustedNodes))
}

// parsePersistentNodes parses a list of discovery node URLs loaded from a .json
// file from within the data directory.
func (c *Config) parsePersistentNodes(path string) []*enode.Node {
func (c *Config) parsePersistentNodes(w *bool, path string) []*enode.Node {
// Short circuit if no node config is present
if c.DataDir == "" {
return nil
}
if _, err := os.Stat(path); err != nil {
return nil
}
c.warnOnce(w, "Found deprecated node list file %s, please use the TOML config file instead.", path)

// Load the nodes from the config file.
var nodelist []string
if err := common.LoadJSON(path, &nodelist); err != nil {
log.Error(fmt.Sprintf("Can't load node file %s: %v", path, err))
log.Error(fmt.Sprintf("Can't load node list file: %v", err))
return nil
}
// Interpret the list as a discovery node array
Expand Down Expand Up @@ -440,3 +449,20 @@ func makeAccountManager(conf *Config) (*accounts.Manager, string, error) {
}
return accounts.NewManager(backends...), ephemeral, nil
}

var warnLock sync.Mutex

func (c *Config) warnOnce(w *bool, format string, args ...interface{}) {
warnLock.Lock()
defer warnLock.Unlock()

if *w {
return
}
l := c.Logger
if l == nil {
l = log.Root()
}
l.Warn(fmt.Sprintf(format, args...))
*w = true
}