diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 5fcbbb7b1..f4858d953 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -14,6 +14,8 @@ BREAKING CHANGES: * [privval] \#2459 Split `SocketPVMsg`s implementations into Request and Response, where the Response may contain a error message (returned by the remote signer) * [state] \#2644 Add Version field to State, breaking the format of State as encoded on disk. + * [rpc] \#2654 Remove all `node_info.other.*_version` fields in `/status` and + `/net_info` * Apps * [abci] \#2298 ResponseQuery.Proof is now a structured merkle.Proof, not just @@ -43,6 +45,9 @@ BREAKING CHANGES: * [state] \#2644 Require block.Version to match state.Version * P2P Protocol + * [p2p] \#2654 Add `ProtocolVersion` struct with protocol versions to top of + DefaultNodeInfo and require `ProtocolVersion.Block` to match during peer handshake + FEATURES: - [crypto/merkle] \#2298 General Merkle Proof scheme for chaining various types of Merkle trees together diff --git a/benchmarks/codec_test.go b/benchmarks/codec_test.go index 71d7a83b2..2be1db156 100644 --- a/benchmarks/codec_test.go +++ b/benchmarks/codec_test.go @@ -14,14 +14,15 @@ import ( func testNodeInfo(id p2p.ID) p2p.DefaultNodeInfo { return p2p.DefaultNodeInfo{ - ID_: id, - Moniker: "SOMENAME", - Network: "SOMENAME", - ListenAddr: "SOMEADDR", - Version: "SOMEVER", + ProtocolVersion: p2p.InitProtocolVersion, + ID_: id, + Moniker: "SOMENAME", + Network: "SOMENAME", + ListenAddr: "SOMEADDR", + Version: "SOMEVER", Other: p2p.DefaultNodeInfoOther{ - AminoVersion: "SOMESTRING", - P2PVersion: "OTHERSTRING", + TxIndex: "on", + RPCAddress: "0.0.0.0:26657", }, } } diff --git a/consensus/version.go b/consensus/version.go deleted file mode 100644 index c04d2ac7d..000000000 --- a/consensus/version.go +++ /dev/null @@ -1,11 +0,0 @@ -package consensus - -import "fmt" - -// kind of arbitrary -var Spec = "1" // async -var Major = "0" // -var Minor = "2" // replay refactor -var Revision = "2" // validation -> commit - -var Version = fmt.Sprintf("v%s/%s.%s.%s", Spec, Major, Minor, Revision) diff --git a/docs/spec/p2p/peer.md b/docs/spec/p2p/peer.md index a1ff25d8b..f5c2e7bf2 100644 --- a/docs/spec/p2p/peer.md +++ b/docs/spec/p2p/peer.md @@ -75,22 +75,25 @@ The Tendermint Version Handshake allows the peers to exchange their NodeInfo: ```golang type NodeInfo struct { + Version p2p.Version ID p2p.ID ListenAddr string Network string - Version string + SoftwareVersion string Channels []int8 Moniker string Other NodeInfoOther } +type Version struct { + P2P uint64 + Block uint64 + App uint64 +} + type NodeInfoOther struct { - AminoVersion string - P2PVersion string - ConsensusVersion string - RPCVersion string TxIndex string RPCAddress string } @@ -99,8 +102,7 @@ type NodeInfoOther struct { The connection is disconnected if: - `peer.NodeInfo.ID` is not equal `peerConn.ID` -- `peer.NodeInfo.Version` is not formatted as `X.X.X` where X are integers known as Major, Minor, and Revision -- `peer.NodeInfo.Version` Major is not the same as ours +- `peer.NodeInfo.Version.Block` does not match ours - `peer.NodeInfo.Network` is not the same as ours - `peer.Channels` does not intersect with our known Channels. - `peer.NodeInfo.ListenAddr` is malformed or is a DNS host that cannot be diff --git a/node/node.go b/node/node.go index 12e0b8e67..97de24736 100644 --- a/node/node.go +++ b/node/node.go @@ -32,7 +32,6 @@ import ( rpccore "github.com/tendermint/tendermint/rpc/core" ctypes "github.com/tendermint/tendermint/rpc/core/types" grpccore "github.com/tendermint/tendermint/rpc/grpc" - "github.com/tendermint/tendermint/rpc/lib" "github.com/tendermint/tendermint/rpc/lib/server" sm "github.com/tendermint/tendermint/state" "github.com/tendermint/tendermint/state/txindex" @@ -771,9 +770,10 @@ func makeNodeInfo( txIndexerStatus = "off" } nodeInfo := p2p.DefaultNodeInfo{ - ID_: nodeID, - Network: chainID, - Version: version.Version, + ProtocolVersion: p2p.InitProtocolVersion, + ID_: nodeID, + Network: chainID, + Version: version.TMCoreSemVer, Channels: []byte{ bc.BlockchainChannel, cs.StateChannel, cs.DataChannel, cs.VoteChannel, cs.VoteSetBitsChannel, @@ -782,12 +782,8 @@ func makeNodeInfo( }, Moniker: config.Moniker, Other: p2p.DefaultNodeInfoOther{ - AminoVersion: amino.Version, - P2PVersion: p2p.Version, - ConsensusVersion: cs.Version, - RPCVersion: fmt.Sprintf("%v/%v", rpc.Version, rpccore.Version), - TxIndex: txIndexerStatus, - RPCAddress: config.RPC.ListenAddress, + TxIndex: txIndexerStatus, + RPCAddress: config.RPC.ListenAddress, }, } diff --git a/p2p/node_info.go b/p2p/node_info.go index a468443d1..5874dc857 100644 --- a/p2p/node_info.go +++ b/p2p/node_info.go @@ -3,9 +3,9 @@ package p2p import ( "fmt" "reflect" - "strings" cmn "github.com/tendermint/tendermint/libs/common" + "github.com/tendermint/tendermint/version" ) const ( @@ -18,8 +18,10 @@ func MaxNodeInfoSize() int { return maxNodeInfoSize } +//------------------------------------------------------------- + // NodeInfo exposes basic info of a node -// and determines if we're compatible +// and determines if we're compatible. type NodeInfo interface { nodeInfoAddress nodeInfoTransport @@ -31,16 +33,38 @@ type nodeInfoAddress interface { NetAddress() *NetAddress } -// nodeInfoTransport is validates a nodeInfo and checks +// nodeInfoTransport validates a nodeInfo and checks // our compatibility with it. It's for use in the handshake. type nodeInfoTransport interface { ValidateBasic() error CompatibleWith(other NodeInfo) error } +//------------------------------------------------------------- + +// ProtocolVersion contains the protocol versions for the software. +type ProtocolVersion struct { + P2P version.Protocol `json:"p2p"` + Block version.Protocol `json:"block"` + App version.Protocol `json:"app"` +} + +var InitProtocolVersion = ProtocolVersion{ + P2P: version.P2PProtocol, + Block: version.BlockProtocol, + App: 0, +} + +//------------------------------------------------------------- + +// Assert DefaultNodeInfo satisfies NodeInfo +var _ NodeInfo = DefaultNodeInfo{} + // DefaultNodeInfo is the basic node information exchanged // between two peers during the Tendermint P2P handshake. type DefaultNodeInfo struct { + ProtocolVersion ProtocolVersion `json:"protocol_version"` + // Authenticate // TODO: replace with NetAddress ID_ ID `json:"id"` // authenticated identifier @@ -59,12 +83,8 @@ type DefaultNodeInfo struct { // DefaultNodeInfoOther is the misc. applcation specific data type DefaultNodeInfoOther 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"` + TxIndex string `json:"tx_index"` + RPCAddress string `json:"rpc_address"` } // ID returns the node's peer ID. @@ -86,35 +106,28 @@ func (info DefaultNodeInfo) ID() ID { // url-encoding), and we just need to be careful with how we handle that in our // clients. (e.g. off by default). func (info DefaultNodeInfo) ValidateBasic() error { - if len(info.Channels) > maxNumChannels { - return fmt.Errorf("info.Channels is too long (%v). Max is %v", len(info.Channels), maxNumChannels) - } - // 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) - } + // ID is already validated. - // Sanitize versions - // XXX: Should we be more strict about version and address formats? - other := info.Other - versions := []string{ - 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) + // Validate ListenAddr. + _, err := NewNetAddressString(IDAddressString(info.ID(), info.ListenAddr)) + if err != nil { + return err } - 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) + + // Network is validated in CompatibleWith. + + // Validate Version + if len(info.Version) > 0 && + (!cmn.IsASCIIText(info.Version) || cmn.ASCIITrim(info.Version) == "") { + + return fmt.Errorf("info.Version must be valid ASCII text without tabs, but got %v", info.Version) } + // Validate Channels - ensure max and check for duplicates. + if len(info.Channels) > maxNumChannels { + return fmt.Errorf("info.Channels is too long (%v). Max is %v", len(info.Channels), maxNumChannels) + } channels := make(map[byte]struct{}) for _, ch := range info.Channels { _, ok := channels[ch] @@ -124,13 +137,30 @@ func (info DefaultNodeInfo) ValidateBasic() error { channels[ch] = struct{}{} } - // ensure ListenAddr is good - _, err := NewNetAddressString(IDAddressString(info.ID(), info.ListenAddr)) - return err + // Validate Moniker. + 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) + } + + // Validate Other. + other := info.Other + txIndex := other.TxIndex + switch txIndex { + case "", "on", "off": + default: + return fmt.Errorf("info.Other.TxIndex should be either 'on' or 'off', got '%v'", txIndex) + } + // XXX: Should we be more strict about address formats? + rpcAddr := other.RPCAddress + if len(rpcAddr) > 0 && (!cmn.IsASCIIText(rpcAddr) || cmn.ASCIITrim(rpcAddr) == "") { + return fmt.Errorf("info.Other.RPCAddress=%v must be valid ASCII text without tabs", rpcAddr) + } + + return nil } // CompatibleWith checks if two DefaultNodeInfo are compatible with eachother. -// CONTRACT: two nodes are compatible if the major version matches and network match +// CONTRACT: two nodes are compatible if the Block version and network match // and they have at least one channel in common. func (info DefaultNodeInfo) CompatibleWith(other_ NodeInfo) error { other, ok := other_.(DefaultNodeInfo) @@ -138,22 +168,9 @@ func (info DefaultNodeInfo) CompatibleWith(other_ NodeInfo) error { return fmt.Errorf("wrong NodeInfo type. Expected DefaultNodeInfo, got %v", reflect.TypeOf(other_)) } - iMajor, _, _, iErr := splitVersion(info.Version) - oMajor, _, _, oErr := splitVersion(other.Version) - - // if our own version number is not formatted right, we messed up - if iErr != nil { - return iErr - } - - // version number must be formatted correctly ("x.x.x") - if oErr != nil { - return oErr - } - - // major version must match - if iMajor != oMajor { - return fmt.Errorf("Peer is on a different major version. Got %v, expected %v", oMajor, iMajor) + if info.ProtocolVersion.Block != other.ProtocolVersion.Block { + return fmt.Errorf("Peer is on a different Block version. Got %v, expected %v", + other.ProtocolVersion.Block, info.ProtocolVersion.Block) } // nodes must be on the same network @@ -201,11 +218,3 @@ func (info DefaultNodeInfo) NetAddress() *NetAddress { } return netAddr } - -func splitVersion(version string) (string, string, string, error) { - spl := strings.Split(version, ".") - if len(spl) != 3 { - return "", "", "", fmt.Errorf("Invalid version format %v", version) - } - return spl[0], spl[1], spl[2], nil -} diff --git a/p2p/node_info_test.go b/p2p/node_info_test.go new file mode 100644 index 000000000..c9a72dbc2 --- /dev/null +++ b/p2p/node_info_test.go @@ -0,0 +1,123 @@ +package p2p + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/tendermint/tendermint/crypto/ed25519" +) + +func TestNodeInfoValidate(t *testing.T) { + + // empty fails + ni := DefaultNodeInfo{} + assert.Error(t, ni.ValidateBasic()) + + channels := make([]byte, maxNumChannels) + for i := 0; i < maxNumChannels; i++ { + channels[i] = byte(i) + } + dupChannels := make([]byte, 5) + copy(dupChannels[:], channels[:5]) + dupChannels = append(dupChannels, testCh) + + nonAscii := "¢§µ" + emptyTab := fmt.Sprintf("\t") + emptySpace := fmt.Sprintf(" ") + + testCases := []struct { + testName string + malleateNodeInfo func(*DefaultNodeInfo) + expectErr bool + }{ + {"Too Many Channels", func(ni *DefaultNodeInfo) { ni.Channels = append(channels, byte(maxNumChannels)) }, true}, + {"Duplicate Channel", func(ni *DefaultNodeInfo) { ni.Channels = dupChannels }, true}, + {"Good Channels", func(ni *DefaultNodeInfo) { ni.Channels = ni.Channels[:5] }, false}, + + {"Invalid NetAddress", func(ni *DefaultNodeInfo) { ni.ListenAddr = "not-an-address" }, true}, + {"Good NetAddress", func(ni *DefaultNodeInfo) { ni.ListenAddr = "0.0.0.0:26656" }, false}, + + {"Non-ASCII Version", func(ni *DefaultNodeInfo) { ni.Version = nonAscii }, true}, + {"Empty tab Version", func(ni *DefaultNodeInfo) { ni.Version = emptyTab }, true}, + {"Empty space Version", func(ni *DefaultNodeInfo) { ni.Version = emptySpace }, true}, + {"Empty Version", func(ni *DefaultNodeInfo) { ni.Version = "" }, false}, + + {"Non-ASCII Moniker", func(ni *DefaultNodeInfo) { ni.Moniker = nonAscii }, true}, + {"Empty tab Moniker", func(ni *DefaultNodeInfo) { ni.Moniker = emptyTab }, true}, + {"Empty space Moniker", func(ni *DefaultNodeInfo) { ni.Moniker = emptySpace }, true}, + {"Empty Moniker", func(ni *DefaultNodeInfo) { ni.Moniker = "" }, true}, + {"Good Moniker", func(ni *DefaultNodeInfo) { ni.Moniker = "hey its me" }, false}, + + {"Non-ASCII TxIndex", func(ni *DefaultNodeInfo) { ni.Other.TxIndex = nonAscii }, true}, + {"Empty tab TxIndex", func(ni *DefaultNodeInfo) { ni.Other.TxIndex = emptyTab }, true}, + {"Empty space TxIndex", func(ni *DefaultNodeInfo) { ni.Other.TxIndex = emptySpace }, true}, + {"Empty TxIndex", func(ni *DefaultNodeInfo) { ni.Other.TxIndex = "" }, false}, + {"Off TxIndex", func(ni *DefaultNodeInfo) { ni.Other.TxIndex = "off" }, false}, + + {"Non-ASCII RPCAddress", func(ni *DefaultNodeInfo) { ni.Other.RPCAddress = nonAscii }, true}, + {"Empty tab RPCAddress", func(ni *DefaultNodeInfo) { ni.Other.RPCAddress = emptyTab }, true}, + {"Empty space RPCAddress", func(ni *DefaultNodeInfo) { ni.Other.RPCAddress = emptySpace }, true}, + {"Empty RPCAddress", func(ni *DefaultNodeInfo) { ni.Other.RPCAddress = "" }, false}, + {"Good RPCAddress", func(ni *DefaultNodeInfo) { ni.Other.RPCAddress = "0.0.0.0:26657" }, false}, + } + + nodeKey := NodeKey{PrivKey: ed25519.GenPrivKey()} + name := "testing" + + // test case passes + ni = testNodeInfo(nodeKey.ID(), name).(DefaultNodeInfo) + ni.Channels = channels + assert.NoError(t, ni.ValidateBasic()) + + for _, tc := range testCases { + ni := testNodeInfo(nodeKey.ID(), name).(DefaultNodeInfo) + ni.Channels = channels + tc.malleateNodeInfo(&ni) + err := ni.ValidateBasic() + if tc.expectErr { + assert.Error(t, err, tc.testName) + } else { + assert.NoError(t, err, tc.testName) + } + } + +} + +func TestNodeInfoCompatible(t *testing.T) { + + nodeKey1 := NodeKey{PrivKey: ed25519.GenPrivKey()} + nodeKey2 := NodeKey{PrivKey: ed25519.GenPrivKey()} + name := "testing" + + var newTestChannel byte = 0x2 + + // test NodeInfo is compatible + ni1 := testNodeInfo(nodeKey1.ID(), name).(DefaultNodeInfo) + ni2 := testNodeInfo(nodeKey2.ID(), name).(DefaultNodeInfo) + assert.NoError(t, ni1.CompatibleWith(ni2)) + + // add another channel; still compatible + ni2.Channels = []byte{newTestChannel, testCh} + assert.NoError(t, ni1.CompatibleWith(ni2)) + + // wrong NodeInfo type is not compatible + _, netAddr := CreateRoutableAddr() + ni3 := mockNodeInfo{netAddr} + assert.Error(t, ni1.CompatibleWith(ni3)) + + testCases := []struct { + testName string + malleateNodeInfo func(*DefaultNodeInfo) + }{ + {"Wrong block version", func(ni *DefaultNodeInfo) { ni.ProtocolVersion.Block += 1 }}, + {"Wrong network", func(ni *DefaultNodeInfo) { ni.Network += "-wrong" }}, + {"No common channels", func(ni *DefaultNodeInfo) { ni.Channels = []byte{newTestChannel} }}, + } + + for _, tc := range testCases { + ni := testNodeInfo(nodeKey2.ID(), name).(DefaultNodeInfo) + tc.malleateNodeInfo(&ni) + assert.Error(t, ni1.CompatibleWith(ni)) + } +} diff --git a/p2p/peer_test.go b/p2p/peer_test.go index fecf7f1cc..9c330ee52 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -207,11 +207,12 @@ func (rp *remotePeer) accept(l net.Listener) { func (rp *remotePeer) nodeInfo(l net.Listener) NodeInfo { return DefaultNodeInfo{ - ID_: rp.Addr().ID, - Moniker: "remote_peer", - Network: "testing", - Version: "123.123.123", - ListenAddr: l.Addr().String(), - Channels: rp.channels, + ProtocolVersion: InitProtocolVersion, + ID_: rp.Addr().ID, + ListenAddr: l.Addr().String(), + Network: "testing", + Version: "1.2.3-rc0-deadbeef", + Channels: rp.channels, + Moniker: "remote_peer", } } diff --git a/p2p/test_util.go b/p2p/test_util.go index 2859dc645..4d43175bb 100644 --- a/p2p/test_util.go +++ b/p2p/test_util.go @@ -247,11 +247,16 @@ func testNodeInfo(id ID, name string) NodeInfo { func testNodeInfoWithNetwork(id ID, name, network string) NodeInfo { return DefaultNodeInfo{ - ID_: id, - ListenAddr: fmt.Sprintf("127.0.0.1:%d", cmn.RandIntn(64512)+1023), - Moniker: name, - Network: network, - Version: "123.123.123", - Channels: []byte{testCh}, + ProtocolVersion: InitProtocolVersion, + ID_: id, + ListenAddr: fmt.Sprintf("127.0.0.1:%d", cmn.RandIntn(64512)+1023), + Network: network, + Version: "1.2.3-rc0-deadbeef", + Channels: []byte{testCh}, + Moniker: name, + Other: DefaultNodeInfoOther{ + TxIndex: "on", + RPCAddress: fmt.Sprintf("127.0.0.1:%d", cmn.RandIntn(64512)+1023), + }, } } diff --git a/p2p/version.go b/p2p/version.go deleted file mode 100644 index 9a4c7bbaf..000000000 --- a/p2p/version.go +++ /dev/null @@ -1,3 +0,0 @@ -package p2p - -const Version = "0.5.0" diff --git a/rpc/core/status.go b/rpc/core/status.go index c26b06b8a..793e1ade7 100644 --- a/rpc/core/status.go +++ b/rpc/core/status.go @@ -31,6 +31,11 @@ import ( // "id": "", // "result": { // "node_info": { +// "protocol_version": { +// "p2p": "4", +// "block": "7", +// "app": "0" +// }, // "id": "53729852020041b956e86685e24394e0bee4373f", // "listen_addr": "10.0.2.15:26656", // "network": "test-chain-Y1OHx6", @@ -38,10 +43,6 @@ import ( // "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" // } diff --git a/rpc/core/version.go b/rpc/core/version.go deleted file mode 100644 index e283de479..000000000 --- a/rpc/core/version.go +++ /dev/null @@ -1,5 +0,0 @@ -package core - -// a single integer is sufficient here - -const Version = "3" // rpc routes for profiling, setting config diff --git a/rpc/lib/version.go b/rpc/lib/version.go deleted file mode 100644 index 8828f260b..000000000 --- a/rpc/lib/version.go +++ /dev/null @@ -1,7 +0,0 @@ -package rpc - -const Maj = "0" -const Min = "7" -const Fix = "0" - -const Version = Maj + "." + Min + "." + Fix