diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index a32b4320..2d77ed90 100644 --- a/CHANGELOG_PENDING.md +++ b/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 diff --git a/benchmarks/codec_test.go b/benchmarks/codec_test.go index 09487563..c0e13d16 100644 --- a/benchmarks/codec_test.go +++ b/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() diff --git a/docs/spec/p2p/peer.md b/docs/spec/p2p/peer.md index 116fec4f..eb344c29 100644 --- a/docs/spec/p2p/peer.md +++ b/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 } ``` diff --git a/node/node.go b/node/node.go index cb3d7d67..7f288fe4 100644 --- a/node/node.go +++ b/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 diff --git a/p2p/node_info.go b/p2p/node_info.go index fa1333b2..c0718dee 100644 --- a/p2p/node_info.go +++ b/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 { diff --git a/p2p/test_util.go b/p2p/test_util.go index 90bcba4f..b88dfb06 100644 --- a/p2p/test_util.go +++ b/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) diff --git a/rpc/core/status.go b/rpc/core/status.go index e34f5244..de8d69cc 100644 --- a/rpc/core/status.go +++ b/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 diff --git a/rpc/core/types/responses.go b/rpc/core/types/responses.go index dbb50ff6..77672910 100644 --- a/rpc/core/types/responses.go +++ b/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 diff --git a/rpc/core/types/responses_test.go b/rpc/core/types/responses_test.go index e410d47a..c6c86e1f 100644 --- a/rpc/core/types/responses_test.go +++ b/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()) } }