cmd/puppeth: reorganize stats reports to make it readable

13 files changed, 205 insertions, 136 deletions

diff --git a/cmd/puppeth/module_dashboard.go b/cmd/puppeth/module_dashboard.go
index 1cf6cab79..7d01f6f0a 100644
--- a/cmd/puppeth/module_dashboard.go
+++ b/cmd/puppeth/module_dashboard.go
@@ -22,6 +22,7 @@ import (
 	"html/template"
 	"math/rand"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"github.com/ethereum/go-ethereum/log"
@@ -499,9 +500,13 @@ type dashboardInfos struct {
 	port int
 }
 
-// String implements the stringer interface.
-func (info *dashboardInfos) String() string {
-	return fmt.Sprintf("host=%s, port=%d", info.host, info.port)
+// Report converts the typed struct into a plain string->string map, cotnaining
+// most - but not all - fields for reporting to the user.
+func (info *dashboardInfos) Report() map[string]string {
+	return map[string]string{
+		"Website address":       info.host,
+		"Website listener port": strconv.Itoa(info.port),
+	}
 }
 
 // checkDashboard does a health-check against a dashboard container to verify if
diff --git a/cmd/puppeth/module_ethstats.go b/cmd/puppeth/module_ethstats.go
index 6ce662f65..2e83e366e 100644
--- a/cmd/puppeth/module_ethstats.go
+++ b/cmd/puppeth/module_ethstats.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"math/rand"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"text/template"
 
@@ -123,9 +124,15 @@ type ethstatsInfos struct {
 	banned []string
 }
 
-// String implements the stringer interface.
-func (info *ethstatsInfos) String() string {
-	return fmt.Sprintf("host=%s, port=%d, secret=%s, banned=%v", info.host, info.port, info.secret, info.banned)
+// Report converts the typed struct into a plain string->string map, cotnaining
+// most - but not all - fields for reporting to the user.
+func (info *ethstatsInfos) Report() map[string]string {
+	return map[string]string{
+		"Website address":       info.host,
+		"Website listener port": strconv.Itoa(info.port),
+		"Login secret":          info.secret,
+		"Banned addresses":      fmt.Sprintf("%v", info.banned),
+	}
 }
 
 // checkEthstats does a health-check against an ethstats server to verify whether
diff --git a/cmd/puppeth/module_faucet.go b/cmd/puppeth/module_faucet.go
index 3c1296bdd..238aa115f 100644
--- a/cmd/puppeth/module_faucet.go
+++ b/cmd/puppeth/module_faucet.go
@@ -18,6 +18,7 @@ package main
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
 	"html/template"
 	"math/rand"
@@ -25,6 +26,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/log"
 )
 
@@ -162,9 +164,31 @@ type faucetInfos struct {
 	captchaSecret string
 }
 
-// String implements the stringer interface.
-func (info *faucetInfos) String() string {
-	return fmt.Sprintf("host=%s, api=%d, eth=%d, amount=%d, minutes=%d, tiers=%d, github=%s, captcha=%v, ethstats=%s", info.host, info.port, info.node.portFull, info.amount, info.minutes, info.tiers, info.githubUser, info.captchaToken != "", info.node.ethstats)
+// Report converts the typed struct into a plain string->string map, cotnaining
+// most - but not all - fields for reporting to the user.
+func (info *faucetInfos) Report() map[string]string {
+	report := map[string]string{
+		"Website address":              info.host,
+		"Website listener port":        strconv.Itoa(info.port),
+		"Ethereum listener port":       strconv.Itoa(info.node.portFull),
+		"Funding amount (base tier)":   fmt.Sprintf("%d Ethers", info.amount),
+		"Funding cooldown (base tier)": fmt.Sprintf("%d mins", info.minutes),
+		"Funding tiers":                strconv.Itoa(info.tiers),
+		"Captha protection":            fmt.Sprintf("%v", info.captchaToken != ""),
+		"Ethstats username":            info.node.ethstats,
+		"GitHub authentication":        info.githubUser,
+	}
+	if info.node.keyJSON != "" {
+		var key struct {
+			Address string `json:"address"`
+		}
+		if err := json.Unmarshal([]byte(info.node.keyJSON), &key); err == nil {
+			report["Funding account"] = common.HexToAddress(key.Address).Hex()
+		} else {
+			log.Error("Failed to retrieve signer address", "err", err)
+		}
+	}
+	return report
 }
 
 // checkFaucet does a health-check against an faucet server to verify whether
diff --git a/cmd/puppeth/module_nginx.go b/cmd/puppeth/module_nginx.go
index fd6d1d74e..67084c80a 100644
--- a/cmd/puppeth/module_nginx.go
+++ b/cmd/puppeth/module_nginx.go
@@ -22,6 +22,7 @@ import (
 	"html/template"
 	"math/rand"
 	"path/filepath"
+	"strconv"
 
 	"github.com/ethereum/go-ethereum/log"
 )
@@ -88,9 +89,12 @@ type nginxInfos struct {
 	port int
 }
 
-// String implements the stringer interface.
-func (info *nginxInfos) String() string {
-	return fmt.Sprintf("port=%d", info.port)
+// Report converts the typed struct into a plain string->string map, cotnaining
+// most - but not all - fields for reporting to the user.
+func (info *nginxInfos) Report() map[string]string {
+	return map[string]string{
+		"Shared listener port": strconv.Itoa(info.port),
+	}
 }
 
 // checkNginx does a health-check against an nginx reverse-proxy to verify whether
diff --git a/cmd/puppeth/module_node.go b/cmd/puppeth/module_node.go
index 375e3e646..ad50cd80a 100644
--- a/cmd/puppeth/module_node.go
+++ b/cmd/puppeth/module_node.go
@@ -18,6 +18,7 @@ package main
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
 	"math/rand"
 	"path/filepath"
@@ -25,6 +26,7 @@ import (
 	"strings"
 	"text/template"
 
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/log"
 )
 
@@ -164,14 +166,37 @@ type nodeInfos struct {
 	gasPrice   float64
 }
 
-// String implements the stringer interface.
-func (info *nodeInfos) String() string {
-	discv5 := ""
+// Report converts the typed struct into a plain string->string map, cotnaining
+// most - but not all - fields for reporting to the user.
+func (info *nodeInfos) Report() map[string]string {
+	report := map[string]string{
+		"Data directory":             info.datadir,
+		"Listener port (full nodes)": strconv.Itoa(info.portFull),
+		"Peer count (all total)":     strconv.Itoa(info.peersTotal),
+		"Peer count (light nodes)":   strconv.Itoa(info.peersLight),
+		"Ethstats username":          info.ethstats,
+	}
 	if info.peersLight > 0 {
-		discv5 = fmt.Sprintf(", portv5=%d", info.portLight)
+		report["Listener port (light nodes)"] = strconv.Itoa(info.portLight)
+	}
+	if info.gasTarget > 0 {
+		report["Gas limit (baseline target)"] = fmt.Sprintf("%0.3f MGas", info.gasTarget)
+		report["Gas price (minimum accepted)"] = fmt.Sprintf("%0.3f GWei", info.gasPrice)
+	}
+	if info.etherbase != "" {
+		report["Miner account"] = info.etherbase
+	}
+	if info.keyJSON != "" {
+		var key struct {
+			Address string `json:"address"`
+		}
+		if err := json.Unmarshal([]byte(info.keyJSON), &key); err == nil {
+			report["Signer account"] = common.HexToAddress(key.Address).Hex()
+		} else {
+			log.Error("Failed to retrieve signer address", "err", err)
+		}
 	}
-	return fmt.Sprintf("port=%d%s, datadir=%s, peers=%d, lights=%d, ethstats=%s, gastarget=%0.3f MGas, gasprice=%0.3f GWei",
-		info.portFull, discv5, info.datadir, info.peersTotal, info.peersLight, info.ethstats, info.gasTarget, info.gasPrice)
+	return report
 }
 
 // checkNode does a health-check against an boot or seal node server to verify
diff --git a/cmd/puppeth/puppeth.go b/cmd/puppeth/puppeth.go
index f783a7981..26382dac1 100644
--- a/cmd/puppeth/puppeth.go
+++ b/cmd/puppeth/puppeth.go
@@ -38,7 +38,7 @@ func main() {
 		},
 		cli.IntFlag{
 			Name:  "loglevel",
-			Value: 4,
+			Value: 3,
 			Usage: "log level to emit to the screen",
 		},
 	}
diff --git a/cmd/puppeth/wizard_dashboard.go b/cmd/puppeth/wizard_dashboard.go
index 53a28a535..3f68c93a4 100644
--- a/cmd/puppeth/wizard_dashboard.go
+++ b/cmd/puppeth/wizard_dashboard.go
@@ -128,5 +128,5 @@ func (w *wizard) deployDashboard() {
 		return
 	}
 	// All ok, run a network scan to pick any changes up
-	w.networkStats(false)
+	w.networkStats()
 }
diff --git a/cmd/puppeth/wizard_ethstats.go b/cmd/puppeth/wizard_ethstats.go
index 8bfa1d6e5..ff75a9d5d 100644
--- a/cmd/puppeth/wizard_ethstats.go
+++ b/cmd/puppeth/wizard_ethstats.go
@@ -112,5 +112,5 @@ func (w *wizard) deployEthstats() {
 		return
 	}
 	// All ok, run a network scan to pick any changes up
-	w.networkStats(false)
+	w.networkStats()
 }
diff --git a/cmd/puppeth/wizard_faucet.go b/cmd/puppeth/wizard_faucet.go
index 51c4e2f7f..08e471ef8 100644
--- a/cmd/puppeth/wizard_faucet.go
+++ b/cmd/puppeth/wizard_faucet.go
@@ -198,5 +198,5 @@ func (w *wizard) deployFaucet() {
 		return
 	}
 	// All ok, run a network scan to pick any changes up
-	w.networkStats(false)
+	w.networkStats()
 }
diff --git a/cmd/puppeth/wizard_intro.go b/cmd/puppeth/wizard_intro.go
index 2d9a097ee..a5fea6f85 100644
--- a/cmd/puppeth/wizard_intro.go
+++ b/cmd/puppeth/wizard_intro.go
@@ -88,7 +88,7 @@ func (w *wizard) run() {
 			}
 			w.servers[server] = client
 		}
-		w.networkStats(false)
+		w.networkStats()
 	}
 	// Basics done, loop ad infinitum about what to do
 	for {
@@ -110,12 +110,11 @@ func (w *wizard) run() {
 		} else {
 			fmt.Println(" 4. Manage network components")
 		}
-		//fmt.Println(" 5. ProTips for common usecases")
 
 		choice := w.read()
 		switch {
 		case choice == "" || choice == "1":
-			w.networkStats(false)
+			w.networkStats()
 
 		case choice == "2":
 			if w.conf.genesis == nil {
@@ -126,7 +125,7 @@ func (w *wizard) run() {
 		case choice == "3":
 			if len(w.servers) == 0 {
 				if w.makeServer() != "" {
-					w.networkStats(false)
+					w.networkStats()
 				}
 			} else {
 				w.manageServers()
@@ -138,9 +137,6 @@ func (w *wizard) run() {
 				w.manageComponents()
 			}
 
-		case choice == "5":
-			w.networkStats(true)
-
 		default:
 			log.Error("That's not something I can do")
 		}
diff --git a/cmd/puppeth/wizard_netstats.go b/cmd/puppeth/wizard_netstats.go
index c06972198..7d8e84242 100644
--- a/cmd/puppeth/wizard_netstats.go
+++ b/cmd/puppeth/wizard_netstats.go
@@ -18,8 +18,8 @@ package main
 
 import (
 	"encoding/json"
-	"fmt"
 	"os"
+	"sort"
 	"strings"
 
 	"github.com/ethereum/go-ethereum/core"
@@ -29,7 +29,7 @@ import (
 
 // networkStats verifies the status of network components and generates a protip
 // configuration set to give users hints on how to do various tasks.
-func (w *wizard) networkStats(tips bool) {
+func (w *wizard) networkStats() {
 	if len(w.servers) == 0 {
 		log.Error("No remote machines to gather stats from")
 		return
@@ -37,51 +37,53 @@ func (w *wizard) networkStats(tips bool) {
 	protips := new(protips)
 
 	// Iterate over all the specified hosts and check their status
-	stats := tablewriter.NewWriter(os.Stdout)
-	stats.SetHeader([]string{"Server", "IP", "Status", "Service", "Details"})
-	stats.SetColWidth(100)
+	stats := make(serverStats)
 
 	for server, pubkey := range w.conf.Servers {
 		client := w.servers[server]
 		logger := log.New("server", server)
 		logger.Info("Starting remote server health-check")
 
-		// If the server is not connected, try to connect again
+		stat := &serverStat{
+			address:  client.address,
+			services: make(map[string]map[string]string),
+		}
+		stats[client.server] = stat
+
 		if client == nil {
 			conn, err := dial(server, pubkey)
 			if err != nil {
 				logger.Error("Failed to establish remote connection", "err", err)
-				stats.Append([]string{server, "", err.Error(), "", ""})
+				stat.failure = err.Error()
 				continue
 			}
 			client = conn
 		}
 		// Client connected one way or another, run health-checks
-		services := make(map[string]string)
 		logger.Debug("Checking for nginx availability")
 		if infos, err := checkNginx(client, w.network); err != nil {
 			if err != ErrServiceUnknown {
-				services["nginx"] = err.Error()
+				stat.services["nginx"] = map[string]string{"offline": err.Error()}
 			}
 		} else {
-			services["nginx"] = infos.String()
+			stat.services["nginx"] = infos.Report()
 		}
 		logger.Debug("Checking for ethstats availability")
 		if infos, err := checkEthstats(client, w.network); err != nil {
 			if err != ErrServiceUnknown {
-				services["ethstats"] = err.Error()
+				stat.services["ethstats"] = map[string]string{"offline": err.Error()}
 			}
 		} else {
-			services["ethstats"] = infos.String()
+			stat.services["ethstats"] = infos.Report()
 			protips.ethstats = infos.config
 		}
 		logger.Debug("Checking for bootnode availability")
 		if infos, err := checkNode(client, w.network, true); err != nil {
 			if err != ErrServiceUnknown {
-				services["bootnode"] = err.Error()
+				stat.services["bootnode"] = map[string]string{"offline": err.Error()}
 			}
 		} else {
-			services["bootnode"] = infos.String()
+			stat.services["bootnode"] = infos.Report()
 
 			protips.genesis = string(infos.genesis)
 			protips.bootFull = append(protips.bootFull, infos.enodeFull)
@@ -92,41 +94,33 @@ func (w *wizard) networkStats(tips bool) {
 		logger.Debug("Checking for sealnode availability")
 		if infos, err := checkNode(client, w.network, false); err != nil {
 			if err != ErrServiceUnknown {
-				services["sealnode"] = err.Error()
+				stat.services["sealnode"] = map[string]string{"offline": err.Error()}
 			}
 		} else {
-			services["sealnode"] = infos.String()
+			stat.services["sealnode"] = infos.Report()
 			protips.genesis = string(infos.genesis)
 		}
 		logger.Debug("Checking for faucet availability")
 		if infos, err := checkFaucet(client, w.network); err != nil {
 			if err != ErrServiceUnknown {
-				services["faucet"] = err.Error()
+				stat.services["faucet"] = map[string]string{"offline": err.Error()}
 			}
 		} else {
-			services["faucet"] = infos.String()
+			stat.services["faucet"] = infos.Report()
 		}
 		logger.Debug("Checking for dashboard availability")
 		if infos, err := checkDashboard(client, w.network); err != nil {
 			if err != ErrServiceUnknown {
-				services["dashboard"] = err.Error()
+				stat.services["dashboard"] = map[string]string{"offline": err.Error()}
 			}
 		} else {
-			services["dashboard"] = infos.String()
+			stat.services["dashboard"] = infos.Report()
 		}
 		// All status checks complete, report and check next server
 		delete(w.services, server)
-		for service := range services {
+		for service := range stat.services {
 			w.services[server] = append(w.services[server], service)
 		}
-		server, address := client.server, client.address
-		for service, status := range services {
-			stats.Append([]string{server, address, "online", service, status})
-			server, address = "", ""
-		}
-		if len(services) == 0 {
-			stats.Append([]string{server, address, "online", "", ""})
-		}
 	}
 	// If a genesis block was found, load it into our configs
 	if protips.genesis != "" && w.conf.genesis == nil {
@@ -145,91 +139,105 @@ func (w *wizard) networkStats(tips bool) {
 	w.conf.bootLight = protips.bootLight
 
 	// Print any collected stats and return
-	if !tips {
-		stats.Render()
-	} else {
-		protips.print(w.network)
-	}
+	stats.render()
 }
 
-// protips contains a collection of network infos to report pro-tips
-// based on.
-type protips struct {
-	genesis   string
-	network   int64
-	bootFull  []string
-	bootLight []string
-	ethstats  string
+// serverStat is a collection of service configuration parameters and health
+// check reports to print to the user.
+type serverStat struct {
+	address  string
+	failure  string
+	services map[string]map[string]string
 }
 
-// print analyzes the network information available and prints a collection of
-// pro tips for the user's consideration.
-func (p *protips) print(network string) {
-	// If a known genesis block is available, display it and prepend an init command
-	fullinit, lightinit := "", ""
-	if p.genesis != "" {
-		fullinit = fmt.Sprintf("geth --datadir=$HOME/.%s init %s.json && ", network, network)
-		lightinit = fmt.Sprintf("geth --datadir=$HOME/.%s --light init %s.json && ", network, network)
-	}
-	// If an ethstats server is available, add the ethstats flag
-	statsflag := ""
-	if p.ethstats != "" {
-		if strings.Contains(p.ethstats, " ") {
-			statsflag = fmt.Sprintf(` --ethstats="yournode:%s"`, p.ethstats)
-		} else {
-			statsflag = fmt.Sprintf(` --ethstats=yournode:%s`, p.ethstats)
-		}
-	}
-	// If bootnodes have been specified, add the bootnode flag
-	bootflagFull := ""
-	if len(p.bootFull) > 0 {
-		bootflagFull = fmt.Sprintf(` --bootnodes %s`, strings.Join(p.bootFull, ","))
-	}
-	bootflagLight := ""
-	if len(p.bootLight) > 0 {
-		bootflagLight = fmt.Sprintf(` --bootnodes %s`, strings.Join(p.bootLight, ","))
-	}
-	// Assemble all the known pro-tips
-	var tasks, tips []string
-
-	tasks = append(tasks, "Run an archive node with historical data")
-	tips = append(tips, fmt.Sprintf("%sgeth --networkid=%d --datadir=$HOME/.%s --cache=1024%s%s", fullinit, p.network, network, statsflag, bootflagFull))
+// serverStats is a collection of server stats for multiple hosts.
+type serverStats map[string]*serverStat
 
-	tasks = append(tasks, "Run a full node with recent data only")
-	tips = append(tips, fmt.Sprintf("%sgeth --networkid=%d --datadir=$HOME/.%s --cache=512 --fast%s%s", fullinit, p.network, network, statsflag, bootflagFull))
+// render converts the gathered statistics into a user friendly tabular report
+// and prints it to the standard output.
+func (stats serverStats) render() {
+	// Start gathering service statistics and config parameters
+	table := tablewriter.NewWriter(os.Stdout)
 
-	tasks = append(tasks, "Run a light node with on demand retrievals")
-	tips = append(tips, fmt.Sprintf("%sgeth --networkid=%d --datadir=$HOME/.%s --light%s%s", lightinit, p.network, network, statsflag, bootflagLight))
+	table.SetHeader([]string{"Server", "Address", "Service", "Config", "Value"})
+	table.SetAlignment(tablewriter.ALIGN_LEFT)
+	table.SetColWidth(100)
 
-	tasks = append(tasks, "Run an embedded node with constrained memory")
-	tips = append(tips, fmt.Sprintf("%sgeth --networkid=%d --datadir=$HOME/.%s --cache=32 --light%s%s", lightinit, p.network, network, statsflag, bootflagLight))
-
-	// If the tips are short, display in a table
-	short := true
-	for _, tip := range tips {
-		if len(tip) > 100 {
-			short = false
-			break
+	// Find the longest lines for all columns for the hacked separator
+	separator := make([]string, 5)
+	for server, stat := range stats {
+		if len(server) > len(separator[0]) {
+			separator[0] = strings.Repeat("-", len(server))
+		}
+		if len(stat.address) > len(separator[1]) {
+			separator[1] = strings.Repeat("-", len(stat.address))
+		}
+		for service, configs := range stat.services {
+			if len(service) > len(separator[2]) {
+				separator[2] = strings.Repeat("-", len(service))
+			}
+			for config, value := range configs {
+				if len(config) > len(separator[3]) {
+					separator[3] = strings.Repeat("-", len(config))
+				}
+				if len(value) > len(separator[4]) {
+					separator[4] = strings.Repeat("-", len(value))
+				}
+			}
 		}
 	}
-	fmt.Println()
-	if short {
-		howto := tablewriter.NewWriter(os.Stdout)
-		howto.SetHeader([]string{"Fun tasks for you", "Tips on how to"})
-		howto.SetColWidth(100)
+	// Fill up the server report in alphabetical order
+	servers := make([]string, 0, len(stats))
+	for server := range stats {
+		servers = append(servers, server)
+	}
+	sort.Strings(servers)
 
-		for i := 0; i < len(tasks); i++ {
-			howto.Append([]string{tasks[i], tips[i]})
+	for i, server := range servers {
+		// Add a separator between all servers
+		if i > 0 {
+			table.Append(separator)
+		}
+		// Fill up the service report in alphabetical order
+		services := make([]string, 0, len(stats[server].services))
+		for service := range stats[server].services {
+			services = append(services, service)
+		}
+		sort.Strings(services)
+
+		for j, service := range services {
+			// Add an empty line between all services
+			if j > 0 {
+				table.Append([]string{"", "", "", separator[3], separator[4]})
+			}
+			// Fill up the config report in alphabetical order
+			configs := make([]string, 0, len(stats[server].services[service]))
+			for service := range stats[server].services[service] {
+				configs = append(configs, service)
+			}
+			sort.Strings(configs)
+
+			for k, config := range configs {
+				switch {
+				case j == 0 && k == 0:
+					table.Append([]string{server, stats[server].address, service, config, stats[server].services[service][config]})
+				case k == 0:
+					table.Append([]string{"", "", service, config, stats[server].services[service][config]})
+				default:
+					table.Append([]string{"", "", "", config, stats[server].services[service][config]})
+				}
+			}
 		}
-		howto.Render()
-		return
-	}
-	// Meh, tips got ugly, split into many lines
-	for i := 0; i < len(tasks); i++ {
-		fmt.Println(tasks[i])
-		fmt.Println(strings.Repeat("-", len(tasks[i])))
-		fmt.Println(tips[i])
-		fmt.Println()
-		fmt.Println()
 	}
+	table.Render()
+}
+
+// protips contains a collection of network infos to report pro-tips
+// based on.
+type protips struct {
+	genesis   string
+	network   int64
+	bootFull  []string
+	bootLight []string
+	ethstats  string
 }
diff --git a/cmd/puppeth/wizard_network.go b/cmd/puppeth/wizard_network.go
index c20e31fab..bf8248e4b 100644
--- a/cmd/puppeth/wizard_network.go
+++ b/cmd/puppeth/wizard_network.go
@@ -53,12 +53,12 @@ func (w *wizard) manageServers() {
 		w.conf.flush()
 
 		log.Info("Disconnected existing server", "server", server)
-		w.networkStats(false)
+		w.networkStats()
 		return
 	}
 	// If the user requested connecting a new server, do it
 	if w.makeServer() != "" {
-		w.networkStats(false)
+		w.networkStats()
 	}
 }
 
diff --git a/cmd/puppeth/wizard_node.go b/cmd/puppeth/wizard_node.go
index 05232486b..69d1715a4 100644
--- a/cmd/puppeth/wizard_node.go
+++ b/cmd/puppeth/wizard_node.go
@@ -156,5 +156,5 @@ func (w *wizard) deployNode(boot bool) {
 	log.Info("Waiting for node to finish booting")
 	time.Sleep(3 * time.Second)
 
-	w.networkStats(false)
+	w.networkStats()
 }