rpc: Transform /status result.node_info.other into map (#2417)

* [rpc] transform /status result.node_info.other into map
* amino does not support maps, duh

Refs #2391

CHANGELOG_PENDING.md

@@ -7,6 +7,7 @@ BREAKING CHANGES:
 * CLI/RPC/Config
 
 * Apps
+  [rpc] /status `result.node_info.other` became a map #[2391](https://github.com/tendermint/tendermint/issues/2391)
 
 * Go API
   * \#2310 Mempool.ReapMaxBytes -> Mempool.ReapMaxBytesMaxGas

benchmarks/codec_test.go

@@ -24,7 +24,10 @@ func BenchmarkEncodeStatusWire(b *testing.B) {
 			Network:    "SOMENAME",
 			ListenAddr: "SOMEADDR",
 			Version:    "SOMEVER",
-			Other:      []string{"SOMESTRING", "OTHERSTRING"},
+			Other: p2p.NodeInfoOther{
+				AminoVersion: "SOMESTRING",
+				P2PVersion:   "OTHERSTRING",
+			},
 		},
 		SyncInfo: ctypes.SyncInfo{
 			LatestBlockHash:   []byte("SOMEBYTES"),
@@ -59,7 +62,10 @@ func BenchmarkEncodeNodeInfoWire(b *testing.B) {
 		Network:    "SOMENAME",
 		ListenAddr: "SOMEADDR",
 		Version:    "SOMEVER",
-		Other:      []string{"SOMESTRING", "OTHERSTRING"},
+		Other: p2p.NodeInfoOther{
+			AminoVersion: "SOMESTRING",
+			P2PVersion:   "OTHERSTRING",
+		},
 	}
 	b.StartTimer()
 
@@ -84,7 +90,10 @@ func BenchmarkEncodeNodeInfoBinary(b *testing.B) {
 		Network:    "SOMENAME",
 		ListenAddr: "SOMEADDR",
 		Version:    "SOMEVER",
-		Other:      []string{"SOMESTRING", "OTHERSTRING"},
+		Other: p2p.NodeInfoOther{
+			AminoVersion: "SOMESTRING",
+			P2PVersion:   "OTHERSTRING",
+		},
 	}
 	b.StartTimer()
 

docs/spec/p2p/peer.md

@@ -83,7 +83,15 @@ type NodeInfo struct {
   Channels   []int8
 
   Moniker    string
-  Other      []string
+  Other      NodeInfoOther
+}
+
+type NodeInfoOther struct {
+	AminoVersion     string
+	P2PVersion       string
+	ConsensusVersion string
+	RPCVersion       string
+	TxIndex          string
 }
 ```
 

node/node.go

@@ -690,12 +690,12 @@ func (n *Node) makeNodeInfo(nodeID p2p.ID) p2p.NodeInfo {
 			evidence.EvidenceChannel,
 		},
 		Moniker: n.config.Moniker,
-		Other: []string{
-			fmt.Sprintf("amino_version=%v", amino.Version),
-			fmt.Sprintf("p2p_version=%v", p2p.Version),
-			fmt.Sprintf("consensus_version=%v", cs.Version),
-			fmt.Sprintf("rpc_version=%v/%v", rpc.Version, rpccore.Version),
-			fmt.Sprintf("tx_index=%v", txIndexerStatus),
+		Other: p2p.NodeInfoOther{
+			AminoVersion:     amino.Version,
+			P2PVersion:       p2p.Version,
+			ConsensusVersion: cs.Version,
+			RPCVersion:       fmt.Sprintf("%v/%v", rpc.Version, rpccore.Version),
+			TxIndex:          txIndexerStatus,
 		},
 	}
 
@@ -704,7 +704,7 @@ func (n *Node) makeNodeInfo(nodeID p2p.ID) p2p.NodeInfo {
 	}
 
 	rpcListenAddr := n.config.RPC.ListenAddress
-	nodeInfo.Other = append(nodeInfo.Other, fmt.Sprintf("rpc_addr=%v", rpcListenAddr))
+	nodeInfo.Other.RPCAddress = rpcListenAddr
 
 	if !n.sw.IsListening() {
 		return nodeInfo

p2p/node_info.go

@@ -32,8 +32,29 @@ type NodeInfo struct {
 	Channels cmn.HexBytes `json:"channels"` // channels this node knows about
 
 	// ASCIIText fields
-	Moniker string   `json:"moniker"` // arbitrary moniker
-	Other   []string `json:"other"`   // other application specific data
+	Moniker string        `json:"moniker"` // arbitrary moniker
+	Other   NodeInfoOther `json:"other"`   // other application specific data
+}
+
+// NodeInfoOther is the misc. applcation specific data
+type NodeInfoOther struct {
+	AminoVersion     string `json:"amino_version"`
+	P2PVersion       string `json:"p2p_version"`
+	ConsensusVersion string `json:"consensus_version"`
+	RPCVersion       string `json:"rpc_version"`
+	TxIndex          string `json:"tx_index"`
+	RPCAddress       string `json:"rpc_address"`
+}
+
+func (o NodeInfoOther) String() string {
+	return fmt.Sprintf(
+		"{amino_version: %v, p2p_version: %v, consensus_version: %v, rpc_version: %v, tx_index: %v}",
+		o.AminoVersion,
+		o.P2PVersion,
+		o.ConsensusVersion,
+		o.RPCVersion,
+		o.TxIndex,
+	)
 }
 
 // Validate checks the self-reported NodeInfo is safe.
@@ -56,13 +77,28 @@ func (info NodeInfo) Validate() error {
 
 	// Sanitize ASCII text fields.
 	if !cmn.IsASCIIText(info.Moniker) || cmn.ASCIITrim(info.Moniker) == "" {
-		return fmt.Errorf("info.Moniker must be valid non-empty ASCII text without tabs, but got %v.", info.Moniker)
+		return fmt.Errorf("info.Moniker must be valid non-empty ASCII text without tabs, but got %v", info.Moniker)
 	}
-	for i, s := range info.Other {
-		if !cmn.IsASCIIText(s) || cmn.ASCIITrim(s) == "" {
-			return fmt.Errorf("info.Other[%v] must be valid non-empty ASCII text without tabs, but got %v.", i, s)
+
+	// Sanitize versions
+	// XXX: Should we be more strict about version and address formats?
+	other := info.Other
+	versions := []string{other.AminoVersion,
+		other.AminoVersion,
+		other.P2PVersion,
+		other.ConsensusVersion,
+		other.RPCVersion}
+	for i, v := range versions {
+		if cmn.ASCIITrim(v) != "" && !cmn.IsASCIIText(v) {
+			return fmt.Errorf("info.Other[%d]=%v must be valid non-empty ASCII text without tabs", i, v)
 		}
 	}
+	if cmn.ASCIITrim(other.TxIndex) != "" && (other.TxIndex != "on" && other.TxIndex != "off") {
+		return fmt.Errorf("info.Other.TxIndex should be either 'on' or 'off', got '%v'", other.TxIndex)
+	}
+	if cmn.ASCIITrim(other.RPCAddress) != "" && !cmn.IsASCIIText(other.RPCAddress) {
+		return fmt.Errorf("info.Other.RPCAddress=%v must be valid non-empty ASCII text without tabs", other.RPCAddress)
+	}
 
 	channels := make(map[byte]struct{})
 	for _, ch := range info.Channels {

p2p/test_util.go

@@ -140,12 +140,21 @@ func MakeSwitch(cfg *config.P2PConfig, i int, network, version string, initSwitc
 	sw := NewSwitch(cfg)
 	sw.SetLogger(log.TestingLogger())
 	sw = initSwitch(i, sw)
+	addr := fmt.Sprintf("127.0.0.1:%d", cmn.RandIntn(64512)+1023)
 	ni := NodeInfo{
 		ID:         nodeKey.ID(),
 		Moniker:    fmt.Sprintf("switch%d", i),
 		Network:    network,
 		Version:    version,
-		ListenAddr: fmt.Sprintf("127.0.0.1:%d", cmn.RandIntn(64512)+1023),
+		ListenAddr: addr,
+		Other: NodeInfoOther{
+			AminoVersion:     "1.0",
+			P2PVersion:       "1.0",
+			ConsensusVersion: "1.0",
+			RPCVersion:       "1.0",
+			TxIndex:          "off",
+			RPCAddress:       addr,
+		},
 	}
 	for ch := range sw.reactorsByCh {
 		ni.Channels = append(ni.Channels, ch)

rpc/core/status.go

@@ -25,43 +25,43 @@ import (
 // > The above command returns JSON structured like this:
 //
 // ```json
-//{
-//  "jsonrpc": "2.0",
-//  "id": "",
-//  "result": {
-//    "node_info": {
-//      "id": "562dd7f579f0ecee8c94a11a3c1e378c1876f433",
-//      "listen_addr": "192.168.1.2:26656",
-//      "network": "test-chain-I6zScH",
-//      "version": "0.19.0",
-//      "channels": "4020212223303800",
-//      "moniker": "Ethans-MacBook-Pro.local",
-//      "other": [
-//        "amino_version=0.9.8",
-//        "p2p_version=0.5.0",
-//        "consensus_version=v1/0.2.2",
-//        "rpc_version=0.7.0/3",
-//        "tx_index=on",
-//        "rpc_addr=tcp://0.0.0.0:26657"
-//      ]
-//    },
-//    "sync_info": {
-//      "latest_block_hash": "2D4D7055BE685E3CB2410603C92AD37AE557AC59",
-//      "latest_app_hash": "0000000000000000",
-//      "latest_block_height": 231,
-//      "latest_block_time": "2018-04-27T23:18:08.459766485-04:00",
-//      "catching_up": false
-//    },
-//    "validator_info": {
-//      "address": "5875562FF0FFDECC895C20E32FC14988952E99E7",
-//      "pub_key": {
-//        "type": "tendermint/PubKeyEd25519",
-//        "value": "PpDJRUrLG2RgFqYYjawfn/AcAgacSXpLFrmfYYQnuzE="
-//      },
-//      "voting_power": 10
-//    }
-//  }
-//}
+// {
+// "jsonrpc": "2.0",
+// "id": "",
+// "result": {
+//   "node_info": {
+//   		"id": "53729852020041b956e86685e24394e0bee4373f",
+//   		"listen_addr": "10.0.2.15:26656",
+//   		"network": "test-chain-Y1OHx6",
+//   		"version": "0.24.0-2ce1abc2",
+//   		"channels": "4020212223303800",
+//   		"moniker": "ubuntu-xenial",
+//   		"other": {
+//   			"amino_version": "0.12.0",
+//   			"p2p_version": "0.5.0",
+//   			"consensus_version": "v1/0.2.2",
+//   			"rpc_version": "0.7.0/3",
+//   			"tx_index": "on",
+//   			"rpc_addr": "tcp://0.0.0.0:26657"
+//   		}
+//   	},
+//   	"sync_info": {
+//   		"latest_block_hash": "F51538DA498299F4C57AC8162AAFA0254CE08286",
+//   		"latest_app_hash": "0000000000000000",
+//   		"latest_block_height": "18",
+//   		"latest_block_time": "2018-09-17T11:42:19.149920551Z",
+//   		"catching_up": false
+//   	},
+//   	"validator_info": {
+//   		"address": "D9F56456D7C5793815D0E9AF07C3A355D0FC64FD",
+//   		"pub_key": {
+//   			"type": "tendermint/PubKeyEd25519",
+//   			"value": "wVxKNtEsJmR4vvh651LrVoRguPs+6yJJ9Bz174gw9DM="
+//   		},
+//   		"voting_power": "10"
+//   	}
+//   }
+// }
 // ```
 func Status() (*ctypes.ResultStatus, error) {
 	var latestHeight int64 = -1

rpc/core/types/responses.go

@@ -2,7 +2,6 @@ package core_types
 
 import (
 	"encoding/json"
-	"strings"
 	"time"
 
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -85,13 +84,7 @@ func (s *ResultStatus) TxIndexEnabled() bool {
 	if s == nil {
 		return false
 	}
-	for _, s := range s.NodeInfo.Other {
-		info := strings.Split(s, "=")
-		if len(info) == 2 && info[0] == "tx_index" {
-			return info[1] == "on"
-		}
-	}
-	return false
+	return s.NodeInfo.Other.TxIndex == "on"
 }
 
 // Info about peer connections

rpc/core/types/responses_test.go

@@ -9,31 +9,27 @@ import (
 )
 
 func TestStatusIndexer(t *testing.T) {
-	assert := assert.New(t)
-
 	var status *ResultStatus
-	assert.False(status.TxIndexEnabled())
+	assert.False(t, status.TxIndexEnabled())
 
 	status = &ResultStatus{}
-	assert.False(status.TxIndexEnabled())
+	assert.False(t, status.TxIndexEnabled())
 
 	status.NodeInfo = p2p.NodeInfo{}
-	assert.False(status.TxIndexEnabled())
+	assert.False(t, status.TxIndexEnabled())
 
 	cases := []struct {
 		expected bool
-		other    []string
+		other    p2p.NodeInfoOther
 	}{
-		{false, nil},
-		{false, []string{}},
-		{false, []string{"a=b"}},
-		{false, []string{"tx_indexiskv", "some=dood"}},
-		{true, []string{"tx_index=on", "tx_index=other"}},
-		{true, []string{"^(*^(", "tx_index=on", "a=n=b=d="}},
+		{false, p2p.NodeInfoOther{}},
+		{false, p2p.NodeInfoOther{TxIndex: "aa"}},
+		{false, p2p.NodeInfoOther{TxIndex: "off"}},
+		{true, p2p.NodeInfoOther{TxIndex: "on"}},
 	}
 
 	for _, tc := range cases {
 		status.NodeInfo.Other = tc.other
-		assert.Equal(tc.expected, status.TxIndexEnabled())
+		assert.Equal(t, tc.expected, status.TxIndexEnabled())
 	}
 }