From 4eb277d1bba3a13f0458fac6e220092754f7d9da Mon Sep 17 00:00:00 2001 From: Dmitrii Golubev Date: Tue, 4 Jan 2022 08:18:29 +0100 Subject: [PATCH 01/13] feat: use validators from init chain response or genesis to establish connection (#228) * feat: use validator list from init-chain response or genesis file to establish connection with them in inter-validator component * refactor: move the logger from required argument into optional parameter, by defailt is used NopLogger * fix: check a size of persisted peers before deleting * feat: add "island" manifest file where every validator is isolated from each other * feat: add a task into Makefile to run "island" e2e test * chore: remove debug logging --- dash/quorum/validator_conn_executor.go | 45 ++++++++++-- dash/quorum/validator_conn_executor_test.go | 6 +- node/node.go | 9 ++- p2p/switch.go | 3 + test/e2e/Makefile | 3 + test/e2e/networks/island.toml | 81 +++++++++++++++++++++ 6 files changed, 138 insertions(+), 9 deletions(-) create mode 100644 test/e2e/networks/island.toml diff --git a/dash/quorum/validator_conn_executor.go b/dash/quorum/validator_conn_executor.go index 25a4eaa9cf..ad8212e2c0 100644 --- a/dash/quorum/validator_conn_executor.go +++ b/dash/quorum/validator_conn_executor.go @@ -37,6 +37,8 @@ type Switch interface { StopPeerGracefully(p2p.Peer) } +type optionFunc func(vc *ValidatorConnExecutor) error + // ValidatorConnExecutor retrieves validator update events and establishes new validator connections // within the ValidatorSet. // If it's already connected to a member of current validator set, it will keep that connection. @@ -74,7 +76,8 @@ func NewValidatorConnExecutor( nodeID p2p.ID, eventBus *types.EventBus, sw Switch, - logger log.Logger) *ValidatorConnExecutor { + opts ...optionFunc, +) (*ValidatorConnExecutor, error) { vc := &ValidatorConnExecutor{ nodeID: nodeID, eventBus: eventBus, @@ -84,11 +87,39 @@ func NewValidatorConnExecutor( connectedValidators: validatorMap{}, quorumHash: make(tmbytes.HexBytes, crypto.QuorumHashSize), } - - baseService := service.NewBaseService(logger, validatorConnExecutorName, vc) + baseService := service.NewBaseService(log.NewNopLogger(), validatorConnExecutorName, vc) vc.BaseService = *baseService - return vc + for _, opt := range opts { + err := opt(vc) + if err != nil { + return nil, err + } + } + return vc, nil +} + +// WithValidatorsSet sets the validators and quorum-hash as default values +func WithValidatorsSet(valSet *types.ValidatorSet) func(vc *ValidatorConnExecutor) error { + return func(vc *ValidatorConnExecutor) error { + if len(valSet.Validators) == 0 { + return nil + } + err := vc.setQuorumHash(valSet.QuorumHash) + if err != nil { + return err + } + vc.validatorSetMembers = newValidatorMap(valSet.Validators) + return nil + } +} + +// WithLogger sets a logger +func WithLogger(logger log.Logger) func(vc *ValidatorConnExecutor) error { + return func(vc *ValidatorConnExecutor) error { + vc.Logger = logger.With("module", "ValidatorConnExecutor") + return nil + } } // OnStart implements Service to subscribe to Validator Update events @@ -96,7 +127,11 @@ func (vc *ValidatorConnExecutor) OnStart() error { if err := vc.subscribe(); err != nil { return err } - + // establish connection with validators retrieves from genesis or init-chain + err := vc.updateConnections() + if err != nil { + return err + } go func() { var err error for err == nil { diff --git a/dash/quorum/validator_conn_executor_test.go b/dash/quorum/validator_conn_executor_test.go index 16bc9f2328..5f5734627b 100644 --- a/dash/quorum/validator_conn_executor_test.go +++ b/dash/quorum/validator_conn_executor_test.go @@ -320,7 +320,8 @@ func TestEndBlock(t *testing.T) { // setup ValidatorConnExecutor sw := mock.NewMockSwitch() nodeID := newVals.Validators[0].NodeAddress.NodeID - vc := NewValidatorConnExecutor(nodeID, eventBus, sw, log.TestingLogger()) + vc, err := NewValidatorConnExecutor(nodeID, eventBus, sw) + require.NoError(t, err) err = vc.Start() require.NoError(t, err) defer func() { err := vc.Stop(); require.NoError(t, err) }() @@ -474,7 +475,8 @@ func setup( sw = mock.NewMockSwitch() nodeID := me.NodeAddress.NodeID - vc = NewValidatorConnExecutor(nodeID, eventBus, sw, log.TestingLogger()) + vc, err = NewValidatorConnExecutor(nodeID, eventBus, sw) + require.NoError(t, err) err = vc.Start() require.NoError(t, err) diff --git a/node/node.go b/node/node.go index 137c698def..6601982fff 100644 --- a/node/node.go +++ b/node/node.go @@ -1044,11 +1044,16 @@ func NewNode(config *cfg.Config, } // Initialize ValidatorConnExecutor - validatorConnExecutor := dashquorum.NewValidatorConnExecutor( + validatorConnExecutor, err := dashquorum.NewValidatorConnExecutor( nodeInfo.ID(), eventBus, sw, - logger.With("module", "ValidatorConnExecutor")) + dashquorum.WithLogger(logger), + dashquorum.WithValidatorsSet(state.Validators), + ) + if err != nil { + return nil, err + } node := &Node{ config: config, diff --git a/p2p/switch.go b/p2p/switch.go index ddf067989c..7ccdd0dba5 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -582,6 +582,9 @@ func (sw *Switch) AddPersistentPeers(addrs []string) error { // It ignores ErrNetAddressLookup. However, if there are other errors, first encounter is // returned. func (sw *Switch) RemovePersistentPeer(addr string) error { + if len(sw.persistentPeersAddrs) == 0 { + return nil + } sw.Logger.Info("Removing persistent peer", "addr", addr) toDelete, err := NewNetAddressString(addr) if err != nil { diff --git a/test/e2e/Makefile b/test/e2e/Makefile index d2b22fd686..7b9184d7c7 100644 --- a/test/e2e/Makefile +++ b/test/e2e/Makefile @@ -20,6 +20,9 @@ runner/dashcore: runner e2e/app/compile runner/rotate: runner e2e/app/compile ./build/runner -f networks/rotate.toml +runner/island: runner e2e/app/compile + ./build/runner -f networks/island.toml + # We need to build support for database backends into the app in # order to build a binary with a Tenderdash node in it (for built-in # ABCI testing). diff --git a/test/e2e/networks/island.toml b/test/e2e/networks/island.toml new file mode 100644 index 0000000000..68312a7909 --- /dev/null +++ b/test/e2e/networks/island.toml @@ -0,0 +1,81 @@ +initial_height = 1000 +initial_state = { initial01 = "a", initial02 = "b", initial03 = "c" } +initial_core_chain_locked_height = 3400 + +[chainlock_updates] +1000 = 3450 +1004 = 3451 +1009 = 3452 +1020 = 3454 +1040 = 3500 + +[validator_update.0] +validator01 = 100 +validator02 = 100 +validator03 = 100 +validator04 = 100 + +[validator_update.1010] +validator02 = 100 +validator04 = 100 +validator06 = 100 +validator07 = 100 + +[validator_update.1020] +validator01 = 100 +validator03 = 100 +validator05 = 100 +validator07 = 100 + +[validator_update.1030] +validator01 = 100 +validator02 = 100 +validator03 = 100 +validator04 = 100 +validator05 = 100 +validator06 = 100 +validator07 = 100 + +[node.validator01] +snapshot_interval = 5 +perturb = ["disconnect"] +privval_protocol = "dashcore" + +[node.validator02] +database = "boltdb" +abci_protocol = "tcp" +privval_protocol = "dashcore" +persist_interval = 0 +perturb = ["restart"] + +[node.validator03] +database = "badgerdb" +privval_protocol = "dashcore" +persist_interval = 3 +retain_blocks = 3 +perturb = ["kill"] + +[node.validator04] +database = "rocksdb" +abci_protocol = "builtin" +privval_protocol = "dashcore" +perturb = ["pause"] + +[node.validator05] +start_at = 1005 +database = "cleveldb" +fast_sync = "v0" +privval_protocol = "dashcore" +perturb = ["kill", "pause", "disconnect", "restart"] + +[node.validator06] +database = "rocksdb" +fast_sync = "v0" +privval_protocol = "dashcore" + +[node.validator07] +start_at = 1005 +database = "cleveldb" +fast_sync = "v0" +privval_protocol = "dashcore" +perturb = ["pause"] From 9f0b34add55176465a33407ec0bec4526cb3c12c Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 10 Nov 2021 11:11:58 +0100 Subject: [PATCH 02/13] feat: autodetect NodeID in ValdatorUpdate --- abci/types/pubkey.go | 12 ++++- abci/types/types.pb.go | 82 +++++++++++++++---------------- dash/quorum/mock/mock_switch.go | 5 +- p2p/address.go | 34 +++++++++++-- proto/tendermint/abci/types.proto | 2 +- state/execution.go | 11 ++--- state/execution_test.go | 1 + types/validator.go | 29 ++++++----- types/validator_test.go | 23 +++++++++ 9 files changed, 133 insertions(+), 66 deletions(-) diff --git a/abci/types/pubkey.go b/abci/types/pubkey.go index 3ddbdea5b7..4c20f19cac 100644 --- a/abci/types/pubkey.go +++ b/abci/types/pubkey.go @@ -4,6 +4,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" cryptoenc "github.com/tendermint/tendermint/crypto/encoding" + "github.com/tendermint/tendermint/p2p" crypto2 "github.com/tendermint/tendermint/proto/tendermint/crypto" ) @@ -18,14 +19,23 @@ func UpdateValidator(proTxHash crypto.ProTxHash, NodeAddress: nodeAddress, } + var pke crypto.PubKey if len(pubkeyBytes) > 0 { - pke := bls12381.PubKey(pubkeyBytes) + pke = bls12381.PubKey(pubkeyBytes) pkp, err := cryptoenc.PubKeyToProto(pke) if err != nil { panic(err) } valUpdate.PubKey = &pkp } + + if nodeAddress != "" { + addr, err := p2p.ParseNodeAddressWithPubkey(nodeAddress, pke) + if err != nil { + panic("cannot parse node address: " + err.Error()) + } + valUpdate.NodeAddress = addr.String() + } return valUpdate } diff --git a/abci/types/types.pb.go b/abci/types/types.pb.go index ceea029b2d..0b7ecbd356 100644 --- a/abci/types/types.pb.go +++ b/abci/types/types.pb.go @@ -3425,12 +3425,12 @@ func init() { func init() { proto.RegisterFile("tendermint/abci/types.proto", fileDescriptor_252557cfdd89a31a) } var fileDescriptor_252557cfdd89a31a = []byte{ - // 3026 bytes of a gzipped FileDescriptorProto + // 3028 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe4, 0x5a, 0xcd, 0x93, 0x23, 0xc5, 0xb1, 0x57, 0xeb, 0x5b, 0xa9, 0xd1, 0xc7, 0xd4, 0x0e, 0xbb, 0x5a, 0xed, 0xee, 0xcc, 0xd2, 0x04, 0xb0, 0x2c, 0x30, 0xf3, 0x98, 0x0d, 0xbe, 0xde, 0xe3, 0x3d, 0x90, 0x84, 0x16, 0x0d, 0x3b, 0xcc, 0x0c, 0x3d, 0xda, 0xe5, 0xd9, 0x98, 0x6d, 0x5a, 0xea, 0x1a, 0xa9, 0x59, 0xa9, 0xbb, 0xe9, 0x2e, - 0x0d, 0x33, 0x5c, 0xb1, 0x2f, 0x9c, 0xf0, 0xcd, 0x3e, 0x70, 0xf6, 0x9f, 0xe0, 0x83, 0x23, 0x7c, + 0x0d, 0x33, 0x5c, 0xb1, 0x2f, 0x9c, 0xf0, 0xcd, 0x3e, 0xf0, 0x27, 0xf8, 0xec, 0x83, 0x23, 0x7c, 0xe6, 0xc8, 0xd1, 0x27, 0x4c, 0x40, 0xf8, 0xe2, 0xa3, 0x2f, 0x8e, 0x70, 0x84, 0xc3, 0x8e, 0xfa, 0xe8, 0x2f, 0x49, 0x2d, 0x69, 0xd8, 0xa3, 0x6f, 0x5d, 0x59, 0x99, 0xd9, 0x55, 0xd5, 0x55, 0xbf, 0xfc, 0x65, 0x76, 0xc1, 0x35, 0x82, 0x4d, 0x1d, 0x3b, 0x63, 0xc3, 0x24, 0x3b, 0x5a, 0xaf, 0x6f, @@ -3577,45 +3577,45 @@ var fileDescriptor_252557cfdd89a31a = []byte{ 0xee, 0x19, 0x2b, 0x8f, 0xfd, 0x4e, 0x82, 0x8a, 0xef, 0x43, 0xc4, 0xa6, 0xff, 0x81, 0x9c, 0x3d, 0xe9, 0xa9, 0xde, 0x2a, 0x4d, 0x1d, 0x40, 0x8f, 0x8f, 0x4e, 0x7a, 0x23, 0xa3, 0x7f, 0x0f, 0x9f, 0x8b, 0x38, 0x94, 0xb5, 0x27, 0xbd, 0x7b, 0x7c, 0x31, 0xf9, 0x30, 0x92, 0x0b, 0x86, 0x91, 0x9a, - 0x1a, 0x06, 0x7a, 0x12, 0xd6, 0x4c, 0x4b, 0xc7, 0xaa, 0xa6, 0xeb, 0x0e, 0x76, 0x5d, 0x41, 0x73, - 0x8a, 0x54, 0xd6, 0xe0, 0x22, 0xf9, 0x7b, 0x09, 0xd0, 0x6c, 0x14, 0x44, 0xc7, 0xb0, 0x1e, 0x04, - 0x52, 0x8f, 0x45, 0xf0, 0x78, 0x74, 0x33, 0x3e, 0x8a, 0x46, 0xd2, 0x8f, 0xea, 0x69, 0x54, 0xec, - 0xa2, 0x2e, 0x6c, 0x90, 0xa1, 0x83, 0xdd, 0xa1, 0x35, 0xd2, 0x55, 0x9b, 0xcd, 0x94, 0x2d, 0x47, - 0x72, 0xc5, 0xe5, 0x48, 0x28, 0xc8, 0xb7, 0xf7, 0x7b, 0x96, 0x1e, 0x3d, 0xd9, 0x86, 0x5a, 0x77, - 0xc6, 0x4c, 0xcc, 0x33, 0x6e, 0x48, 0xd2, 0xe3, 0x0c, 0x49, 0xbe, 0x03, 0xd5, 0xf7, 0xfd, 0xf7, - 0x8b, 0x37, 0x4d, 0x0d, 0x53, 0x9a, 0x19, 0xe6, 0x29, 0xe4, 0x1f, 0x58, 0x84, 0x27, 0xef, 0xff, - 0x17, 0x06, 0x4e, 0xef, 0xbf, 0x4b, 0xec, 0xb2, 0x8b, 0x91, 0x84, 0x70, 0xf3, 0x36, 0xac, 0x53, - 0xf8, 0xc0, 0xba, 0x1a, 0x24, 0xe2, 0x6c, 0x99, 0xf3, 0x4a, 0x85, 0x77, 0xec, 0x7b, 0x59, 0xb8, - 0xfc, 0x4f, 0x09, 0xf2, 0x1e, 0x82, 0xa3, 0x97, 0x42, 0x58, 0x52, 0x9e, 0x53, 0x56, 0xf4, 0x14, - 0x83, 0x62, 0x77, 0x74, 0xac, 0xc9, 0x8b, 0x8f, 0x35, 0xee, 0xaf, 0x85, 0xf7, 0xdb, 0x29, 0x7d, - 0xe1, 0xdf, 0x4e, 0x2f, 0x00, 0x22, 0x16, 0xd1, 0x46, 0xea, 0xa9, 0x45, 0x0c, 0x73, 0xa0, 0xf2, - 0x93, 0xc3, 0x99, 0x7c, 0x95, 0xf5, 0x3c, 0x60, 0x1d, 0x47, 0x54, 0x2e, 0xff, 0x41, 0x82, 0xbc, - 0x4f, 0x96, 0x2e, 0x5a, 0xbb, 0xbe, 0x0c, 0x59, 0xc1, 0x07, 0x78, 0xf1, 0x5a, 0xb4, 0xfc, 0xdf, - 0x28, 0xe9, 0xd0, 0x6f, 0x94, 0x3a, 0xe4, 0xc7, 0x98, 0x68, 0x8c, 0x31, 0x72, 0x48, 0xf7, 0xdb, - 0xe8, 0x55, 0xa8, 0x2d, 0x29, 0x7f, 0x3c, 0xd1, 0x9f, 0x57, 0xfa, 0xb8, 0xfd, 0x3a, 0x14, 0x43, - 0xff, 0x1f, 0x28, 0x0c, 0x1f, 0xb4, 0x3f, 0xa8, 0x26, 0xea, 0xb9, 0x2f, 0xbf, 0xbe, 0x99, 0x3a, - 0xc0, 0x9f, 0xa1, 0x1a, 0xe4, 0x94, 0x76, 0xab, 0xd3, 0x6e, 0xdd, 0xab, 0x4a, 0xf5, 0xe2, 0x97, - 0x5f, 0xdf, 0xcc, 0x29, 0x98, 0x95, 0x31, 0x6f, 0x77, 0x60, 0x2d, 0xfc, 0x39, 0xa3, 0x5c, 0x04, - 0x41, 0xf9, 0xed, 0xfb, 0x47, 0xfb, 0x7b, 0xad, 0x46, 0xb7, 0xad, 0x3e, 0x38, 0xec, 0xb6, 0xab, - 0x12, 0xba, 0x02, 0x97, 0xf6, 0xf7, 0xde, 0xe9, 0x74, 0xd5, 0xd6, 0xfe, 0x5e, 0xfb, 0xa0, 0xab, - 0x36, 0xba, 0xdd, 0x46, 0xeb, 0x5e, 0x35, 0xb9, 0xfb, 0x05, 0x40, 0xa5, 0xd1, 0x6c, 0xed, 0x51, - 0x1e, 0x65, 0xf4, 0x35, 0x51, 0x26, 0x4e, 0xb3, 0x1a, 0xd6, 0xc2, 0x0b, 0x13, 0xf5, 0xc5, 0x55, - 0x72, 0x74, 0x17, 0x32, 0xac, 0xbc, 0x85, 0x16, 0xdf, 0xa0, 0xa8, 0x2f, 0x29, 0x9b, 0xd3, 0xc1, - 0xb0, 0x73, 0xb5, 0xf0, 0x4a, 0x45, 0x7d, 0x71, 0x15, 0x1d, 0x29, 0x50, 0x08, 0xaa, 0x4b, 0xcb, - 0xaf, 0x58, 0xd4, 0x57, 0xa8, 0xac, 0x53, 0x9f, 0x41, 0x1e, 0xbb, 0xfc, 0xca, 0x41, 0x7d, 0x85, - 0x68, 0x86, 0xf6, 0x21, 0xe7, 0x55, 0x08, 0x96, 0x5d, 0x82, 0xa8, 0x2f, 0xad, 0x7a, 0xd3, 0x4f, - 0xc0, 0x2b, 0x39, 0x8b, 0x6f, 0x74, 0xd4, 0x97, 0x94, 0xf0, 0xd1, 0x1e, 0x64, 0x45, 0xd6, 0xb4, - 0xe4, 0x62, 0x43, 0x7d, 0x59, 0x15, 0x9b, 0x2e, 0x5a, 0x50, 0x96, 0x5b, 0x7e, 0x4f, 0xa5, 0xbe, - 0xc2, 0xdf, 0x09, 0x74, 0x1f, 0x20, 0x54, 0xb7, 0x59, 0xe1, 0x02, 0x4a, 0x7d, 0x95, 0xbf, 0x0e, - 0xe8, 0x10, 0xf2, 0x7e, 0x7e, 0xbe, 0xf4, 0x3a, 0x48, 0x7d, 0x79, 0xf9, 0x1f, 0x3d, 0x84, 0x52, - 0x34, 0x63, 0x5c, 0xed, 0x92, 0x47, 0x7d, 0xc5, 0xba, 0x3e, 0xf5, 0x1f, 0x4d, 0x1f, 0x57, 0xbb, - 0xf4, 0x51, 0x5f, 0xb1, 0xcc, 0x8f, 0x3e, 0x81, 0xf5, 0xd9, 0xf4, 0x6e, 0xf5, 0x3b, 0x20, 0xf5, - 0x0b, 0x14, 0xfe, 0xd1, 0x18, 0xd0, 0x9c, 0xb4, 0xf0, 0x02, 0x57, 0x42, 0xea, 0x17, 0xf9, 0x0f, - 0xd0, 0x6c, 0x7f, 0xf3, 0xc3, 0xa6, 0xf4, 0xed, 0x0f, 0x9b, 0xd2, 0xf7, 0x3f, 0x6c, 0x4a, 0x5f, - 0xfd, 0xb8, 0x99, 0xf8, 0xf6, 0xc7, 0xcd, 0xc4, 0x9f, 0x7e, 0xdc, 0x4c, 0xfc, 0xfc, 0xf9, 0x81, - 0x41, 0x86, 0x93, 0xde, 0x76, 0xdf, 0x1a, 0xef, 0x84, 0xef, 0xab, 0xcd, 0xbb, 0x43, 0xd7, 0xcb, - 0xb2, 0x08, 0x77, 0xe7, 0xdf, 0x01, 0x00, 0x00, 0xff, 0xff, 0x30, 0x49, 0xe8, 0x7c, 0x63, 0x27, - 0x00, 0x00, + 0x1a, 0x06, 0x7a, 0x16, 0xd6, 0x4c, 0x4b, 0xc7, 0xaa, 0xa6, 0xeb, 0x0e, 0x76, 0x5d, 0x4e, 0x73, + 0x84, 0xe7, 0x22, 0xed, 0x69, 0xf0, 0x0e, 0xf9, 0x7b, 0x09, 0xd0, 0x6c, 0x2c, 0x44, 0xc7, 0xb0, + 0x1e, 0x84, 0x53, 0x8f, 0x4b, 0xf0, 0xa8, 0x74, 0x33, 0x3e, 0x96, 0x46, 0x92, 0x90, 0xea, 0x69, + 0x54, 0xec, 0xa2, 0x2e, 0x6c, 0x90, 0xa1, 0x83, 0xdd, 0xa1, 0x35, 0xd2, 0x55, 0x9b, 0xcd, 0x97, + 0x2d, 0x4a, 0x72, 0xc5, 0x45, 0x49, 0x28, 0xc8, 0xb7, 0xf7, 0x7b, 0x96, 0x1e, 0x40, 0xd9, 0x86, + 0x5a, 0x77, 0xc6, 0x4c, 0xcc, 0x33, 0x6e, 0x48, 0xd2, 0xe3, 0x0c, 0x49, 0xbe, 0x03, 0xd5, 0xf7, + 0xfd, 0xf7, 0x8b, 0x37, 0x4d, 0x0d, 0x53, 0x9a, 0x19, 0xe6, 0x29, 0xe4, 0x1f, 0x58, 0x84, 0xa7, + 0xf0, 0xff, 0x17, 0x86, 0x4f, 0xef, 0xef, 0x4b, 0xec, 0xb2, 0x8b, 0x91, 0x84, 0xd0, 0xf3, 0x36, + 0xac, 0x53, 0x10, 0xc1, 0xba, 0x1a, 0xa4, 0xe3, 0x6c, 0x99, 0xf3, 0x4a, 0x85, 0x77, 0xec, 0x7b, + 0xb9, 0xb8, 0xfc, 0x4f, 0x09, 0xf2, 0x1e, 0x8e, 0xa3, 0x97, 0x42, 0x88, 0x52, 0x9e, 0x53, 0x5c, + 0xf4, 0x14, 0x83, 0x92, 0x77, 0x74, 0xac, 0xc9, 0x8b, 0x8f, 0x35, 0xee, 0xdf, 0x85, 0xf7, 0xf3, + 0x29, 0x7d, 0xe1, 0x9f, 0x4f, 0x2f, 0x00, 0x22, 0x16, 0xd1, 0x46, 0xea, 0xa9, 0x45, 0x0c, 0x73, + 0xa0, 0xf2, 0xf3, 0xc3, 0xf9, 0x7c, 0x95, 0xf5, 0x3c, 0x60, 0x1d, 0x47, 0x54, 0x2e, 0xff, 0x41, + 0x82, 0xbc, 0x4f, 0x99, 0x2e, 0x5a, 0xc1, 0xbe, 0x0c, 0x59, 0xc1, 0x0a, 0x78, 0x09, 0x5b, 0xb4, + 0xfc, 0x9f, 0x29, 0xe9, 0xd0, 0xcf, 0x94, 0x3a, 0xe4, 0xc7, 0x98, 0x68, 0x8c, 0x37, 0x72, 0x60, + 0xf7, 0xdb, 0xe8, 0x55, 0xa8, 0x2d, 0x29, 0x82, 0x3c, 0xd1, 0x9f, 0x57, 0x00, 0xb9, 0xfd, 0x3a, + 0x14, 0x43, 0x7f, 0x21, 0x28, 0x18, 0x1f, 0xb4, 0x3f, 0xa8, 0x26, 0xea, 0xb9, 0x2f, 0xbf, 0xbe, + 0x99, 0x3a, 0xc0, 0x9f, 0xa1, 0x1a, 0xe4, 0x94, 0x76, 0xab, 0xd3, 0x6e, 0xdd, 0xab, 0x4a, 0xf5, + 0xe2, 0x97, 0x5f, 0xdf, 0xcc, 0x29, 0x98, 0x15, 0x33, 0x6f, 0x77, 0x60, 0x2d, 0xfc, 0x39, 0xa3, + 0x8c, 0x04, 0x41, 0xf9, 0xed, 0xfb, 0x47, 0xfb, 0x7b, 0xad, 0x46, 0xb7, 0xad, 0x3e, 0x38, 0xec, + 0xb6, 0xab, 0x12, 0xba, 0x02, 0x97, 0xf6, 0xf7, 0xde, 0xe9, 0x74, 0xd5, 0xd6, 0xfe, 0x5e, 0xfb, + 0xa0, 0xab, 0x36, 0xba, 0xdd, 0x46, 0xeb, 0x5e, 0x35, 0xb9, 0xfb, 0x05, 0x40, 0xa5, 0xd1, 0x6c, + 0xed, 0x51, 0x36, 0x65, 0xf4, 0x35, 0x51, 0x2c, 0x4e, 0xb3, 0x4a, 0xd6, 0xc2, 0x6b, 0x13, 0xf5, + 0xc5, 0xb5, 0x72, 0x74, 0x17, 0x32, 0xac, 0xc8, 0x85, 0x16, 0xdf, 0xa3, 0xa8, 0x2f, 0x29, 0x9e, + 0xd3, 0xc1, 0xb0, 0x73, 0xb5, 0xf0, 0x62, 0x45, 0x7d, 0x71, 0x2d, 0x1d, 0x29, 0x50, 0x08, 0x6a, + 0x4c, 0xcb, 0x2f, 0x5a, 0xd4, 0x57, 0xa8, 0xaf, 0x53, 0x9f, 0x41, 0x36, 0xbb, 0xfc, 0xe2, 0x41, + 0x7d, 0x85, 0x98, 0x86, 0xf6, 0x21, 0xe7, 0xd5, 0x09, 0x96, 0x5d, 0x85, 0xa8, 0x2f, 0xad, 0x7d, + 0xd3, 0x4f, 0xc0, 0xeb, 0x39, 0x8b, 0xef, 0x75, 0xd4, 0x97, 0x14, 0xf2, 0xd1, 0x1e, 0x64, 0x45, + 0xee, 0xb4, 0xe4, 0x7a, 0x43, 0x7d, 0x59, 0x2d, 0x9b, 0x2e, 0x5a, 0x50, 0x9c, 0x5b, 0x7e, 0x5b, + 0xa5, 0xbe, 0xc2, 0x3f, 0x0a, 0x74, 0x1f, 0x20, 0x54, 0xbd, 0x59, 0xe1, 0x1a, 0x4a, 0x7d, 0x95, + 0x7f, 0x0f, 0xe8, 0x10, 0xf2, 0x7e, 0x96, 0xbe, 0xf4, 0x52, 0x48, 0x7d, 0xf9, 0x4f, 0x00, 0xf4, + 0x10, 0x4a, 0xd1, 0xbc, 0x71, 0xb5, 0xab, 0x1e, 0xf5, 0x15, 0xab, 0xfb, 0xd4, 0x7f, 0x34, 0x89, + 0x5c, 0xed, 0xea, 0x47, 0x7d, 0xc5, 0x62, 0x3f, 0xfa, 0x04, 0xd6, 0x67, 0x93, 0xbc, 0xd5, 0x6f, + 0x82, 0xd4, 0x2f, 0x50, 0xfe, 0x47, 0x63, 0x40, 0x73, 0x92, 0xc3, 0x0b, 0x5c, 0x0c, 0xa9, 0x5f, + 0xe4, 0x6f, 0x40, 0xb3, 0xfd, 0xcd, 0x0f, 0x9b, 0xd2, 0xb7, 0x3f, 0x6c, 0x4a, 0xdf, 0xff, 0xb0, + 0x29, 0x7d, 0xf5, 0xe3, 0x66, 0xe2, 0xdb, 0x1f, 0x37, 0x13, 0x7f, 0xfa, 0x71, 0x33, 0xf1, 0xf3, + 0xe7, 0x07, 0x06, 0x19, 0x4e, 0x7a, 0xdb, 0x7d, 0x6b, 0xbc, 0x13, 0xbe, 0xb5, 0x36, 0xef, 0x26, + 0x5d, 0x2f, 0xcb, 0x22, 0xdc, 0x9d, 0x7f, 0x07, 0x00, 0x00, 0xff, 0xff, 0x80, 0x31, 0xd9, 0x0b, + 0x69, 0x27, 0x00, 0x00, } // Reference imports to suppress errors if they are not otherwise used. diff --git a/dash/quorum/mock/mock_switch.go b/dash/quorum/mock/mock_switch.go index d3e0f8f104..0eeb7e2966 100644 --- a/dash/quorum/mock/mock_switch.go +++ b/dash/quorum/mock/mock_switch.go @@ -74,7 +74,10 @@ func (sw Switch) RemovePersistentPeer(addr string) error { func (sw *Switch) DialPeersAsync(addrs []string) error { for _, addr := range addrs { peer := &mocks.Peer{} - parsed, _ := p2p.ParseNodeAddress(addr) + parsed, err := p2p.ParseNodeAddress(addr) + if err != nil { + return err + } peer.On("ID").Return(parsed.NodeID) peer.On("String").Return(addr) diff --git a/p2p/address.go b/p2p/address.go index 9ac16d0ada..286afccc8e 100644 --- a/p2p/address.go +++ b/p2p/address.go @@ -5,8 +5,9 @@ package p2p // This file was downloaded from Tendermint master, revision bc1a20dbb86e4fa2120b2c8a9de88814471f4a2c: // https://raw.githubusercontent.com/tendermint/tendermint/bc1a20dbb86e4fa2120b2c8a9de88814471f4a2c/internal/p2p/address.go // and refactored to workaround some dependencies. -// When backporting upstream, you can replace this file. - +// Changed functions: +// 1. ParseNodeAddress divided into 2 functions +// 2. ParseNodeAddressWithPubkey added import ( "context" "errors" @@ -18,6 +19,7 @@ import ( "strconv" "strings" + tmcrypto "github.com/tendermint/tendermint/crypto" tmrand "github.com/tendermint/tendermint/libs/rand" ) @@ -46,9 +48,32 @@ type NodeAddress struct { Path string } +// ParseNodeAddressWithPubkey parses a node address URL into a NodeAddress, normalizing +// and validating it. If the node address does not contain Node ID, it will be generated +// based on provided public key +func ParseNodeAddressWithPubkey(urlString string, pubKey tmcrypto.PubKey) (NodeAddress, error) { + if urlString == "" { + return NodeAddress{}, fmt.Errorf("empty node address") + } + address, err := parseNodeAddressWithoutValidation(urlString) + if err != nil { + return address, err + } + if address.NodeID == "" && pubKey != nil { + address.NodeID = PubKeyToID(pubKey) + } + return address, address.Validate() +} + // ParseNodeAddress parses a node address URL into a NodeAddress, normalizing // and validating it. func ParseNodeAddress(urlString string) (NodeAddress, error) { + return ParseNodeAddressWithPubkey(urlString, nil) +} + +// parseNodeAddressWithoutValidation parses a node address URL into a NodeAddress, normalizing it. +// It does NOT validate parsed address +func parseNodeAddressWithoutValidation(urlString string) (NodeAddress, error) { // url.Parse requires a scheme, so if it fails to parse a scheme-less URL // we try to apply a default scheme. url, err := url.Parse(urlString) @@ -67,7 +92,7 @@ func ParseNodeAddress(urlString string) (NodeAddress, error) { // Opaque URLs are expected to contain only a node ID. if url.Opaque != "" { address.NodeID = ID(url.Opaque) - return address, address.Validate() + return address, nil } // Otherwise, just parse a normal networked URL. @@ -100,7 +125,7 @@ func ParseNodeAddress(urlString string) (NodeAddress, error) { } } - return address, address.Validate() + return address, nil } // Resolve resolves a NodeAddress into a set of Endpoints, by expanding @@ -199,7 +224,6 @@ func (a NodeAddress) NetAddress() (*NetAddress, error) { // p2p.RandNodeAddress generates a random validator address func RandNodeAddress() NodeAddress { - nodeID := tmrand.Bytes(20) port := (tmrand.Int() % 65535) + 1 addr, err := ParseNodeAddress(fmt.Sprintf("tcp://%x@127.0.0.1:%d", nodeID, port)) diff --git a/proto/tendermint/abci/types.proto b/proto/tendermint/abci/types.proto index 1021b8ce75..940b142c5f 100644 --- a/proto/tendermint/abci/types.proto +++ b/proto/tendermint/abci/types.proto @@ -352,7 +352,7 @@ message ValidatorUpdate { tendermint.crypto.PublicKey pub_key = 1 [(gogoproto.nullable) = true]; int64 power = 2; bytes pro_tx_hash = 3; - string node_address = 4; // address of the Validator, correct URI (RFC 3986) + string node_address = 4 [(gogoproto.nullable) = true]; // address of the Validator, correct URI } message ValidatorSetUpdate { diff --git a/state/execution.go b/state/execution.go index 721614ecf2..d6ef87cb02 100644 --- a/state/execution.go +++ b/state/execution.go @@ -490,8 +490,10 @@ func validateValidatorUpdates(abciUpdates []abci.ValidatorUpdate, } // Check if validator's pubkey matches an ABCI type in the consensus params + var pk crypto.PubKey // needed for NodeAddress if valUpdate.PubKey != nil { - pk, err := cryptoenc.PubKeyFromProto(*valUpdate.PubKey) + var err error + pk, err = cryptoenc.PubKeyFromProto(*valUpdate.PubKey) if err != nil { return err } @@ -530,15 +532,12 @@ func validateValidatorUpdates(abciUpdates []abci.ValidatorUpdate, ) } - // Validate endpoint address + // Validate endpoint address; note that we need to support case where node ID is not set if valUpdate.NodeAddress != "" { - addr, err := p2p.ParseNodeAddress(valUpdate.NodeAddress) + _, err := p2p.ParseNodeAddressWithPubkey(valUpdate.NodeAddress, pk) if err != nil { return fmt.Errorf("cannot parse validator address %s: %w", valUpdate.NodeAddress, err) } - if err = addr.Validate(); err != nil { - return fmt.Errorf("validator address %s is invalid: %w", valUpdate.NodeAddress, err) - } } } return nil diff --git a/state/execution_test.go b/state/execution_test.go index 12365cab34..ebeb99e704 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -156,6 +156,7 @@ func TestValidateValidatorUpdates(t *testing.T) { } addr := p2p.RandNodeAddress() + addr.NodeID = "" testCases := []struct { name string diff --git a/types/validator.go b/types/validator.go index a2ec93dffa..15fc726402 100644 --- a/types/validator.go +++ b/types/validator.go @@ -49,22 +49,23 @@ func NewValidatorDefaultVotingPower(pubKey crypto.PubKey, proTxHash []byte) *Val // NewValidator returns a new validator with the given pubkey and voting power. func NewValidator(pubKey crypto.PubKey, votingPower int64, proTxHash []byte, address string) *Validator { - val := &Validator{ + var ( + addr p2p.NodeAddress + err error + ) + if address != "" { + addr, err = p2p.ParseNodeAddressWithPubkey(address, pubKey) + if err != nil { + panic(err.Error()) + } + } + return &Validator{ PubKey: pubKey, VotingPower: votingPower, ProposerPriority: 0, ProTxHash: proTxHash, + NodeAddress: addr, } - - if address != "" { - addr, err := p2p.ParseNodeAddress(address) - if err != nil { - panic(fmt.Sprintf("cannot parse validator address %s: %s", address, err)) - } - val.NodeAddress = addr - } - - return val } // ValidateBasic performs basic validation. @@ -85,6 +86,12 @@ func (v *Validator) ValidateBasic() error { return fmt.Errorf("validator proTxHash is the wrong size: %v", len(v.ProTxHash)) } + if !v.NodeAddress.Zero() { + if err := v.NodeAddress.Validate(); err != nil { + return fmt.Errorf("validator node address is invalid: %w", err) + } + } + return nil } diff --git a/types/validator_test.go b/types/validator_test.go index ceda591af4..a7a1cb58a7 100644 --- a/types/validator_test.go +++ b/types/validator_test.go @@ -1,9 +1,11 @@ package types import ( + "fmt" "testing" "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/p2p" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -107,3 +109,24 @@ func TestValidatorValidateBasic(t *testing.T) { }) } } + +func TestNewValidator(t *testing.T) { + quorumHash := crypto.RandQuorumHash() + priv := NewMockPVForQuorum(quorumHash) + pubKey, err := priv.GetPubKey(quorumHash) + nodeID := p2p.PubKeyToID(pubKey) + proTxHash := crypto.RandProTxHash() + require.NoError(t, err) + + validator := NewValidator(pubKey, DefaultDashVotingPower, proTxHash, + fmt.Sprintf("tcp://%s@127.0.0.1:12345", nodeID)) + require.NotNil(t, validator) + assert.Equal(t, nodeID, validator.NodeAddress.NodeID) + + validator = NewValidator(pubKey, DefaultDashVotingPower, proTxHash, "127.0.0.1:23456") + require.NotNil(t, validator) + assert.EqualValues(t, "127.0.0.1", validator.NodeAddress.Hostname) + assert.EqualValues(t, 23456, validator.NodeAddress.Port) + assert.EqualValues(t, "tcp", validator.NodeAddress.Protocol) + assert.EqualValues(t, nodeID, validator.NodeAddress.NodeID) +} From bb3839aad062650a0665f45703b9c9adedb071cf Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 17 Nov 2021 18:20:50 +0100 Subject: [PATCH 03/13] feat(dash/quorum): handle validators without address --- dash/quorum/validator_conn_executor.go | 2 +- dash/quorum/validator_conn_executor_test.go | 37 +++++++++++++++++-- dash/quorum/validator_map.go | 5 ++- .../adr-d001-inter-validator-set-messaging.md | 11 ++++-- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/dash/quorum/validator_conn_executor.go b/dash/quorum/validator_conn_executor.go index ad8212e2c0..dc3fe171ff 100644 --- a/dash/quorum/validator_conn_executor.go +++ b/dash/quorum/validator_conn_executor.go @@ -233,7 +233,7 @@ func (vc *ValidatorConnExecutor) setQuorumHash(newQuorumHash tmbytes.HexBytes) e } // selectValidators selects `count` validators from current ValidatorSet. -// It uses algorithm described in DIP-6 (`SelectValidatorsDIP6()`). +// It uses algorithm described in DIP-6. // Returns map indexed by validator address. func (vc *ValidatorConnExecutor) selectValidators() (validatorMap, error) { activeValidators := vc.validatorSetMembers diff --git a/dash/quorum/validator_conn_executor_test.go b/dash/quorum/validator_conn_executor_test.go index 5f5734627b..cc2d38d1e7 100644 --- a/dash/quorum/validator_conn_executor_test.go +++ b/dash/quorum/validator_conn_executor_test.go @@ -64,14 +64,19 @@ func TestValidatorConnExecutor_WrongAddress(t *testing.T) { me := mock.NewValidator(65535) addr1, err := p2p.ParseNodeAddress("http://john@www.google.com:80") - val1 := mock.NewValidator(5) + require.NoError(t, err) + + val1 := mock.NewValidator(100) val1.NodeAddress = addr1 - // val1 := &types.Validator{NodeAddress: addr1} - require.NoError(t, err) + valsWithoutAddress := make([]*types.Validator, 5) + for i := 0; i < len(valsWithoutAddress); i++ { + valsWithoutAddress[i] = mock.NewValidator(uint64(200 + i)) + valsWithoutAddress[i].NodeAddress = p2p.NodeAddress{} + } + tc := testCase{ me: me, - // quorumHash: , validatorUpdates: []validatorUpdate{ 0: { validators: []*types.Validator{ @@ -94,6 +99,30 @@ func TestValidatorConnExecutor_WrongAddress(t *testing.T) { // {Operation: mock.OpStopOne}, }, }, + 2: { + validators: []*types.Validator{ + me, + valsWithoutAddress[0], + mock.NewValidator(1), + mock.NewValidator(2), + mock.NewValidator(3), + mock.NewValidator(4), + mock.NewValidator(5), + }, + expectedHistory: []mock.SwitchHistoryEvent{ + {Operation: mock.OpDialMany, Params: []string{ + mock.NewNodeAddress(2), + mock.NewNodeAddress(5), + }}, + }, + }, + 3: { // this should disconnect everyone because none of the validators has correct address + validators: append([]*types.Validator{me}, valsWithoutAddress...), + expectedHistory: []mock.SwitchHistoryEvent{ + {Operation: mock.OpStopOne}, + {Operation: mock.OpStopOne}, + }, + }, }, } executeTestCase(t, tc) diff --git a/dash/quorum/validator_map.go b/dash/quorum/validator_map.go index 5d64f25873..7df0e7716c 100644 --- a/dash/quorum/validator_map.go +++ b/dash/quorum/validator_map.go @@ -12,9 +12,10 @@ type validatorMap map[p2p.ID]types.Validator func newValidatorMap(validators []*types.Validator) validatorMap { newMap := make(validatorMap, len(validators)) for _, validator := range validators { - newMap[validator.NodeAddress.NodeID] = *validator + if !validator.NodeAddress.Zero() && validator.NodeAddress.NodeID != "" { + newMap[validator.NodeAddress.NodeID] = *validator + } } - return newMap } diff --git a/docs/architecture/adr-d001-inter-validator-set-messaging.md b/docs/architecture/adr-d001-inter-validator-set-messaging.md index a8a5644be4..99213020e4 100644 --- a/docs/architecture/adr-d001-inter-validator-set-messaging.md +++ b/docs/architecture/adr-d001-inter-validator-set-messaging.md @@ -123,11 +123,11 @@ through another Validator) to any other Validator which is a member of the same ### What systems will be affected? * Tenderdash -* ABCI App +* ABCI App (Dash Drive) ### What new data structures are needed, what data structures need changes? -1. Additional configuration parameter: number of validator connections +None ### What new APIs will be needed, what APIs will change? @@ -151,10 +151,15 @@ message ValidatorUpdate { tendermint.crypto.PublicKey pub_key = 1 [(gogoproto.nullable) = true]; int64 power = 2; bytes pro_tx_hash = 3; - string node_address = 4; // address of the Validator, correct URI (RFC 3986) + string node_address = 4 [(gogoproto.nullable) = true]; // address of the Validator } ``` + +If ABCI app cannot determine node address, it can leave it empty. + +Note that the Dash Drive as an ABCI app should be able to determine node address if it is member of the active Validator Set. If the node is NOT a member of active Validator Set, the `node_address` should be empty. + ### What are the efficiency considerations (time/space)? 1. This change will increase the amount of data sent by the ABCI application to Tenderdash. The connection between them is local. Therefore the change should not have any noticeable performance or bandwidth impact. From da4e2ab634f677e7504250fdf3f9660909f92e44 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Fri, 26 Nov 2021 13:00:32 +0100 Subject: [PATCH 04/13] feat(quorum): Autodetect node ID when connecting to quorum members --- abci/example/kvstore/helpers.go | 6 +- abci/types/pubkey.go | 12 +-- dash/dashtypes/validator_address.go | 114 ++++++++++++++++++++ dash/dashtypes/validator_address_test.go | 61 +++++++++++ dash/quorum/mock/mock_switch.go | 5 +- dash/quorum/mock/test_helpers.go | 8 +- dash/quorum/validator_conn_executor.go | 26 +++-- dash/quorum/validator_conn_executor_test.go | 16 +-- dash/quorum/validator_map.go | 17 ++- node/node.go | 23 ++-- node/node_test.go | 8 ++ p2p/address.go | 45 ++------ p2p/conn/secret_connection.go | 40 ++++--- p2p/conn/secret_connection_test.go | 29 +++-- state/execution.go | 9 +- state/execution_test.go | 7 +- state/state_test.go | 6 +- test/e2e/tests/validator_test.go | 4 +- types/protobuf.go | 9 +- types/protobuf_test.go | 13 ++- types/validator.go | 26 ++--- types/validator_set.go | 8 +- types/validator_test.go | 15 ++- 23 files changed, 350 insertions(+), 157 deletions(-) create mode 100644 dash/dashtypes/validator_address.go create mode 100644 dash/dashtypes/validator_address_test.go diff --git a/abci/example/kvstore/helpers.go b/abci/example/kvstore/helpers.go index f20cf90d7d..d9ce769090 100644 --- a/abci/example/kvstore/helpers.go +++ b/abci/example/kvstore/helpers.go @@ -5,11 +5,11 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" cryptoenc "github.com/tendermint/tendermint/crypto/encoding" - "github.com/tendermint/tendermint/p2p" + "github.com/tendermint/tendermint/dash/dashtypes" ) func ValUpdate( - pubKey crypto.PubKey, proTxHash crypto.ProTxHash, address p2p.NodeAddress) types.ValidatorUpdate { + pubKey crypto.PubKey, proTxHash crypto.ProTxHash, address dashtypes.ValidatorAddress) types.ValidatorUpdate { return types.UpdateValidator(proTxHash, pubKey.Bytes(), 100, address.String()) } @@ -21,7 +21,7 @@ func RandValidatorSetUpdate(cnt int) types.ValidatorSetUpdate { privKeys, proTxHashes, thresholdPublicKey := bls12381.CreatePrivLLMQDataDefaultThreshold(cnt) for i := 0; i < cnt; i++ { - res[i] = ValUpdate(privKeys[i].PubKey(), proTxHashes[i], p2p.RandNodeAddress()) + res[i] = ValUpdate(privKeys[i].PubKey(), proTxHashes[i], dashtypes.RandValidatorAddress()) } thresholdPublicKeyABCI, err := cryptoenc.PubKeyToProto(thresholdPublicKey) if err != nil { diff --git a/abci/types/pubkey.go b/abci/types/pubkey.go index 4c20f19cac..3ddbdea5b7 100644 --- a/abci/types/pubkey.go +++ b/abci/types/pubkey.go @@ -4,7 +4,6 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" cryptoenc "github.com/tendermint/tendermint/crypto/encoding" - "github.com/tendermint/tendermint/p2p" crypto2 "github.com/tendermint/tendermint/proto/tendermint/crypto" ) @@ -19,23 +18,14 @@ func UpdateValidator(proTxHash crypto.ProTxHash, NodeAddress: nodeAddress, } - var pke crypto.PubKey if len(pubkeyBytes) > 0 { - pke = bls12381.PubKey(pubkeyBytes) + pke := bls12381.PubKey(pubkeyBytes) pkp, err := cryptoenc.PubKeyToProto(pke) if err != nil { panic(err) } valUpdate.PubKey = &pkp } - - if nodeAddress != "" { - addr, err := p2p.ParseNodeAddressWithPubkey(nodeAddress, pke) - if err != nil { - panic("cannot parse node address: " + err.Error()) - } - valUpdate.NodeAddress = addr.String() - } return valUpdate } diff --git a/dash/dashtypes/validator_address.go b/dash/dashtypes/validator_address.go new file mode 100644 index 0000000000..bea1f54dff --- /dev/null +++ b/dash/dashtypes/validator_address.go @@ -0,0 +1,114 @@ +package dashtypes + +import ( + "errors" + "fmt" + "net" + "time" + + tmrand "github.com/tendermint/tendermint/libs/rand" + "github.com/tendermint/tendermint/p2p" + "github.com/tendermint/tendermint/p2p/conn" +) + +// ValidatorAddress is a NodeAddress that does not require node ID to be set +type ValidatorAddress struct { + p2p.NodeAddress +} + +// ParseValidatorAddress parses provided address, which should be in `proto://nodeID@host:port` form. +// `proto://` and `nodeID@` parts are optional. +func ParseValidatorAddress(address string) (ValidatorAddress, error) { + addr, err := p2p.ParseNodeAddressWithoutValidation(address) + if err != nil { + return ValidatorAddress{}, err + } + va := ValidatorAddress{NodeAddress: addr} + return va, va.Validate() +} + +// Validate ensures the validator address is correct. +// It ignores missing node IDs. +func (va ValidatorAddress) Validate() error { + if va.NodeAddress.Protocol == "" { + return errors.New("no protocol") + } + if va.NodeAddress.Hostname == "" { + return errors.New("no hostname") + } + if va.NodeAddress.Port <= 0 { + return errors.New("no port") + } + return nil +} + +// Hostname returns host name of this address +func (va ValidatorAddress) Hostname() string { + return va.NodeAddress.Hostname +} + +// Port returns port number of this address +func (va ValidatorAddress) Port() uint16 { + return va.NodeAddress.Port +} + +// Protocol returns protocl name of this address, like "tcp" +func (va ValidatorAddress) Protocol() string { + return va.NodeAddress.Protocol +} + +// NetAddress returns this ValidatorAddress as a *p2p.NetAddress that can be used to establish connection +func (va ValidatorAddress) NetAddress() (*p2p.NetAddress, error) { + if _, err := va.NodeID(); err != nil { + return nil, fmt.Errorf("cannot determine node id for address %s: %w", va.String(), err) + } + return va.NodeAddress.NetAddress() +} + +// NodeID() returns node ID. If it is not set, it will connect to remote node, retrieve its public key +// and calculate Node ID based on it. Noe this connection can be expensive. +func (va *ValidatorAddress) NodeID() (p2p.ID, error) { + if va.NodeAddress.NodeID == "" { + var err error + va.NodeAddress.NodeID, err = va.retrieveNodeID() + if err != nil { + return "", err + } + } + return va.NodeAddress.NodeID, nil +} + +// retrieveNodeID retrieves a node ID from remote node. +// Note that it is quite expensive, as it establishes secure connection to the other node +// which is dropped afterwards. +func (va ValidatorAddress) retrieveNodeID() (p2p.ID, error) { + dialer := net.Dialer{Timeout: 1000 * time.Millisecond} + connection, err := dialer.Dial("tcp4", fmt.Sprintf("%s:%d", va.NodeAddress.Hostname, va.NodeAddress.Port)) + if err != nil { + return "", fmt.Errorf("cannot lookup node ID: %w", err) + } + defer connection.Close() + if err := connection.SetDeadline(time.Now().Add(1 * time.Second)); err != nil { + return "", err + } + + sc, err := conn.MakeSecretConnection(connection, nil) + if err != nil { + return "", err + } + return p2p.PubKeyToID(sc.RemotePubKey()), nil +} + +// dashtypes.RandValidatorAddress generates a random validator address +func RandValidatorAddress() ValidatorAddress { + nodeID := tmrand.Bytes(20) + port := (tmrand.Int() % 65535) + 1 + addr, err := ParseValidatorAddress(fmt.Sprintf("tcp://%x@127.0.0.1:%d", nodeID, port)) + if err != nil { + panic(fmt.Sprintf("cannot generate random validator address: %s", err)) + } + if err := addr.Validate(); err != nil { + panic(fmt.Sprintf("randomly generated validator address %s is invalid: %s", addr.String(), err)) + } + return addr +} diff --git a/dash/dashtypes/validator_address_test.go b/dash/dashtypes/validator_address_test.go new file mode 100644 index 0000000000..76703f513c --- /dev/null +++ b/dash/dashtypes/validator_address_test.go @@ -0,0 +1,61 @@ +package dashtypes + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidatorAddress_String(t *testing.T) { + + tests := []struct { + uri string + want string + }{ + { + uri: "tcp://node@fqdn.address.com:1234", + want: "tcp://node@fqdn.address.com:1234", + }, + { + uri: "tcp://fqdn.address.com:1234", + want: "tcp://fqdn.address.com:1234", + }, + } + for _, tt := range tests { + t.Run(tt.uri, func(t *testing.T) { + va, err := ParseValidatorAddress(tt.uri) + assert.NoError(t, err) + got := va.String() + assert.EqualValues(t, tt.want, got) + }) + } +} + +// TestValidatorAddress_NodeID_fail checks if NodeID lookup fails when trying to connect to ssh port +// NOTE: Positive flow is tested as part of node_test.go TestNodeStartStop() +func TestValidatorAddress_NodeID_fail(t *testing.T) { + + tests := []struct { + uri string + want string + wantErr bool + }{ + { + uri: "tcp://node@fqdn.address.com:1234", + want: "node", + }, + { + uri: "tcp://127.0.0.1:22", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.uri, func(t *testing.T) { + va, err := ParseValidatorAddress(tt.uri) + assert.NoError(t, err) + got, err := va.NodeID() + assert.Equal(t, err != nil, tt.wantErr, "wantErr=%s, but err = %s", tt.wantErr, err) + assert.EqualValues(t, tt.want, got) + }) + } +} diff --git a/dash/quorum/mock/mock_switch.go b/dash/quorum/mock/mock_switch.go index 0eeb7e2966..9bec5f47f8 100644 --- a/dash/quorum/mock/mock_switch.go +++ b/dash/quorum/mock/mock_switch.go @@ -5,6 +5,7 @@ import ( "strings" "time" + "github.com/tendermint/tendermint/dash/dashtypes" "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/p2p/mocks" ) @@ -74,12 +75,12 @@ func (sw Switch) RemovePersistentPeer(addr string) error { func (sw *Switch) DialPeersAsync(addrs []string) error { for _, addr := range addrs { peer := &mocks.Peer{} - parsed, err := p2p.ParseNodeAddress(addr) + parsed, err := dashtypes.ParseValidatorAddress(addr) if err != nil { return err } - peer.On("ID").Return(parsed.NodeID) + peer.On("ID").Return(parsed.NodeID()) peer.On("String").Return(addr) if err := sw.PeerSet.Add(peer); err != nil { return err diff --git a/dash/quorum/mock/test_helpers.go b/dash/quorum/mock/test_helpers.go index 4c47b9052a..9a567c2f96 100644 --- a/dash/quorum/mock/test_helpers.go +++ b/dash/quorum/mock/test_helpers.go @@ -5,8 +5,8 @@ import ( "fmt" "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/dash/dashtypes" "github.com/tendermint/tendermint/libs/bytes" - "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/types" ) @@ -15,14 +15,16 @@ import ( func NewNodeAddress(n uint64) string { nodeID := make([]byte, 20) binary.LittleEndian.PutUint64(nodeID, n) - + if n == 0 { + n = 65535 + } return fmt.Sprintf("tcp://%x@127.0.0.1:%d", nodeID, uint16(n)) } // NewValidator generates a validator with only fields needed for node selection filled. // For the same `id`, mock validator will always have the same data (proTxHash, NodeID) func NewValidator(id uint64) *types.Validator { - address, err := p2p.ParseNodeAddress(NewNodeAddress(id)) + address, err := dashtypes.ParseValidatorAddress(NewNodeAddress(id)) if err != nil { panic(err) } diff --git a/dash/quorum/validator_conn_executor.go b/dash/quorum/validator_conn_executor.go index dc3fe171ff..e303a52027 100644 --- a/dash/quorum/validator_conn_executor.go +++ b/dash/quorum/validator_conn_executor.go @@ -49,7 +49,7 @@ type optionFunc func(vc *ValidatorConnExecutor) error // will retry the connection if it fails. type ValidatorConnExecutor struct { service.BaseService - nodeID p2p.ID + proTxHash types.ProTxHash eventBus *types.EventBus p2pSwitch Switch subscription types.Subscription @@ -73,13 +73,13 @@ type ValidatorConnExecutor struct { // NewValidatorConnExecutor creates a Service that connects to other validators within the same Validator Set. // Don't forget to Start() and Stop() the service. func NewValidatorConnExecutor( - nodeID p2p.ID, + proTxHash types.ProTxHash, eventBus *types.EventBus, sw Switch, opts ...optionFunc, ) (*ValidatorConnExecutor, error) { vc := &ValidatorConnExecutor{ - nodeID: nodeID, + proTxHash: proTxHash, eventBus: eventBus, p2pSwitch: sw, EventBusCapacity: defaultEventBusCapacity, @@ -232,18 +232,24 @@ func (vc *ValidatorConnExecutor) setQuorumHash(newQuorumHash tmbytes.HexBytes) e return nil } +// me returns current node's validator object, if any, or nil if not found +func (vc *ValidatorConnExecutor) me() *types.Validator { + me := vc.validatorSetMembers[validatorMapIndexType(vc.proTxHash.String())] + return &me +} + // selectValidators selects `count` validators from current ValidatorSet. // It uses algorithm described in DIP-6. // Returns map indexed by validator address. func (vc *ValidatorConnExecutor) selectValidators() (validatorMap, error) { activeValidators := vc.validatorSetMembers - me, ok := activeValidators[vc.nodeID] - if !ok { + me := vc.me() + if me == nil { return validatorMap{}, fmt.Errorf("current node is not member of active validator set") } selector := selectpeers.NewDIP6ValidatorSelector(vc.quorumHash) - selectedValidators, err := selector.SelectValidators(activeValidators.values(), &me) + selectedValidators, err := selector.SelectValidators(activeValidators.values(), me) if err != nil { return validatorMap{}, err } @@ -260,7 +266,10 @@ func (vc *ValidatorConnExecutor) disconnectValidator(validator types.Validator) return err } - id := validator.NodeAddress.NodeID + id, err := validator.NodeAddress.NodeID() + if err != nil { + return err + } peer := vc.p2pSwitch.Peers().Get(id) if peer == nil { return fmt.Errorf("cannot stop peer %s: not found", id) @@ -291,7 +300,7 @@ func (vc *ValidatorConnExecutor) disconnectValidators(exceptions validatorMap) e // to be established. It will also disconnect previous validators. func (vc *ValidatorConnExecutor) updateConnections() error { // We only do something if we are part of new ValidatorSet - if _, ok := vc.validatorSetMembers[vc.nodeID]; !ok { + if me := vc.me(); me == nil { vc.Logger.Debug("not a member of active ValidatorSet") // We need to disconnect connected validators. It needs to be done explicitly // because they are marked as persistent and will never disconnect themselves. @@ -355,6 +364,7 @@ func (vc *ValidatorConnExecutor) dial(vals validatorMap) error { vc.Logger.Error("cannot set validators as persistent", "peers", addresses, "err", err) return fmt.Errorf("cannot set validators as persistent: %w", err) } + // TODO in tendermint 0.35, we will use router.connectPeer instead of DialPeersAsync if err := vc.p2pSwitch.DialPeersAsync(addresses); err != nil { vc.Logger.Error("cannot dial validators", "peers", addresses, "err", err) return fmt.Errorf("cannot dial peers: %w", err) diff --git a/dash/quorum/validator_conn_executor_test.go b/dash/quorum/validator_conn_executor_test.go index cc2d38d1e7..2611968372 100644 --- a/dash/quorum/validator_conn_executor_test.go +++ b/dash/quorum/validator_conn_executor_test.go @@ -10,12 +10,12 @@ import ( "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/dash/dashtypes" "github.com/tendermint/tendermint/dash/quorum/mock" "github.com/tendermint/tendermint/dash/quorum/selectpeers" tmbytes "github.com/tendermint/tendermint/libs/bytes" "github.com/tendermint/tendermint/libs/log" mmock "github.com/tendermint/tendermint/mempool/mock" - "github.com/tendermint/tendermint/p2p" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" "github.com/tendermint/tendermint/proxy" sm "github.com/tendermint/tendermint/state" @@ -63,7 +63,7 @@ func TestValidatorConnExecutor_NotValidator(t *testing.T) { func TestValidatorConnExecutor_WrongAddress(t *testing.T) { me := mock.NewValidator(65535) - addr1, err := p2p.ParseNodeAddress("http://john@www.google.com:80") + addr1, err := dashtypes.ParseValidatorAddress("http://john@www.google.com:80") require.NoError(t, err) val1 := mock.NewValidator(100) @@ -72,7 +72,7 @@ func TestValidatorConnExecutor_WrongAddress(t *testing.T) { valsWithoutAddress := make([]*types.Validator, 5) for i := 0; i < len(valsWithoutAddress); i++ { valsWithoutAddress[i] = mock.NewValidator(uint64(200 + i)) - valsWithoutAddress[i].NodeAddress = p2p.NodeAddress{} + valsWithoutAddress[i].NodeAddress = dashtypes.ValidatorAddress{} } tc := testCase{ @@ -343,13 +343,13 @@ func TestEndBlock(t *testing.T) { // Ensure new validators have some IP addresses set for _, validator := range newVals.Validators { - validator.NodeAddress = p2p.RandNodeAddress() + validator.NodeAddress = dashtypes.RandValidatorAddress() } // setup ValidatorConnExecutor sw := mock.NewMockSwitch() - nodeID := newVals.Validators[0].NodeAddress.NodeID - vc, err := NewValidatorConnExecutor(nodeID, eventBus, sw) + proTxHash := newVals.Validators[0].ProTxHash + vc, err := NewValidatorConnExecutor(proTxHash, eventBus, sw) require.NoError(t, err) err = vc.Start() require.NoError(t, err) @@ -503,8 +503,8 @@ func setup( sw = mock.NewMockSwitch() - nodeID := me.NodeAddress.NodeID - vc, err = NewValidatorConnExecutor(nodeID, eventBus, sw) + proTxHash := me.ProTxHash + vc, err = NewValidatorConnExecutor(proTxHash, eventBus, sw) require.NoError(t, err) err = vc.Start() require.NoError(t, err) diff --git a/dash/quorum/validator_map.go b/dash/quorum/validator_map.go index 7df0e7716c..c660412b3f 100644 --- a/dash/quorum/validator_map.go +++ b/dash/quorum/validator_map.go @@ -1,19 +1,26 @@ package quorum import ( - "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/types" ) +// validatorMapIndexType represents data that is used to index `validatorMap` elements +type validatorMapIndexType string + // validatorMap maps validator ID to the validator -type validatorMap map[p2p.ID]types.Validator +type validatorMap map[validatorMapIndexType]types.Validator + +// validatorMapIndex returns index value to use inside validator map +func validatorMapIndex(v types.Validator) validatorMapIndexType { + return validatorMapIndexType(v.ProTxHash.String()) +} // newValidatorMap creates a new validatoMap based on a slice of Validators func newValidatorMap(validators []*types.Validator) validatorMap { newMap := make(validatorMap, len(validators)) for _, validator := range validators { - if !validator.NodeAddress.Zero() && validator.NodeAddress.NodeID != "" { - newMap[validator.NodeAddress.NodeID] = *validator + if !validator.NodeAddress.Zero() { + newMap[validatorMapIndex(*validator)] = *validator } } return newMap @@ -31,7 +38,7 @@ func (vm validatorMap) values() []*types.Validator { // contains returns true if the validatorMap contains `What`, false otherwise. // Items are compared using node ID. func (vm validatorMap) contains(what types.Validator) bool { - _, ok := vm[what.NodeAddress.NodeID] + _, ok := vm[validatorMapIndex(what)] return ok } diff --git a/node/node.go b/node/node.go index 6601982fff..930c7c276e 100644 --- a/node/node.go +++ b/node/node.go @@ -1043,16 +1043,19 @@ func NewNode(config *cfg.Config, }() } - // Initialize ValidatorConnExecutor - validatorConnExecutor, err := dashquorum.NewValidatorConnExecutor( - nodeInfo.ID(), - eventBus, - sw, - dashquorum.WithLogger(logger), - dashquorum.WithValidatorsSet(state.Validators), - ) - if err != nil { - return nil, err + // Initialize ValidatorConnExecutor (only on Validators) + var validatorConnExecutor *dashquorum.ValidatorConnExecutor + if proTxHashP != nil { + validatorConnExecutor, err = dashquorum.NewValidatorConnExecutor( + *proTxHashP, + eventBus, + sw, + dashquorum.WithLogger(logger), + dashquorum.WithValidatorsSet(state.Validators), + ) + if err != nil { + return nil, err + } } node := &Node{ diff --git a/node/node_test.go b/node/node_test.go index 92186a59ed..2f3981fef1 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/dash/dashtypes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -57,6 +58,13 @@ func TestNodeStartStop(t *testing.T) { t.Fatal("timed out waiting for the node to produce a block") } + // check if we can read node ID of this node + va, err := dashtypes.ParseValidatorAddress(config.P2P.ListenAddress) + assert.NoError(t, err) + id, err := va.NodeID() + assert.NoError(t, err) + assert.Equal(t, n.nodeInfo.ID(), id) + // stop the node go func() { err = n.Stop() diff --git a/p2p/address.go b/p2p/address.go index 286afccc8e..fb0f7a8c71 100644 --- a/p2p/address.go +++ b/p2p/address.go @@ -5,9 +5,9 @@ package p2p // This file was downloaded from Tendermint master, revision bc1a20dbb86e4fa2120b2c8a9de88814471f4a2c: // https://raw.githubusercontent.com/tendermint/tendermint/bc1a20dbb86e4fa2120b2c8a9de88814471f4a2c/internal/p2p/address.go // and refactored to workaround some dependencies. -// Changed functions: -// 1. ParseNodeAddress divided into 2 functions -// 2. ParseNodeAddressWithPubkey added +// Changes: +// 1. ParseNodeAddress divided into 2 functions (added ParseNodeAddressWithoutValidation) +// 2. Added some missing types at the end of file import ( "context" "errors" @@ -18,9 +18,6 @@ import ( "regexp" "strconv" "strings" - - tmcrypto "github.com/tendermint/tendermint/crypto" - tmrand "github.com/tendermint/tendermint/libs/rand" ) var ( @@ -48,32 +45,22 @@ type NodeAddress struct { Path string } -// ParseNodeAddressWithPubkey parses a node address URL into a NodeAddress, normalizing -// and validating it. If the node address does not contain Node ID, it will be generated -// based on provided public key -func ParseNodeAddressWithPubkey(urlString string, pubKey tmcrypto.PubKey) (NodeAddress, error) { +// ParseNodeAddress parses a node address URL into a NodeAddress, normalizing +// and validating it. +func ParseNodeAddress(urlString string) (NodeAddress, error) { if urlString == "" { return NodeAddress{}, fmt.Errorf("empty node address") } - address, err := parseNodeAddressWithoutValidation(urlString) + address, err := ParseNodeAddressWithoutValidation(urlString) if err != nil { return address, err } - if address.NodeID == "" && pubKey != nil { - address.NodeID = PubKeyToID(pubKey) - } return address, address.Validate() } -// ParseNodeAddress parses a node address URL into a NodeAddress, normalizing -// and validating it. -func ParseNodeAddress(urlString string) (NodeAddress, error) { - return ParseNodeAddressWithPubkey(urlString, nil) -} - -// parseNodeAddressWithoutValidation parses a node address URL into a NodeAddress, normalizing it. +// ParseNodeAddressWithoutValidation parses a node address URL into a NodeAddress, normalizing it. // It does NOT validate parsed address -func parseNodeAddressWithoutValidation(urlString string) (NodeAddress, error) { +func ParseNodeAddressWithoutValidation(urlString string) (NodeAddress, error) { // url.Parse requires a scheme, so if it fails to parse a scheme-less URL // we try to apply a default scheme. url, err := url.Parse(urlString) @@ -222,20 +209,6 @@ func (a NodeAddress) NetAddress() (*NetAddress, error) { return addr, nil } -// p2p.RandNodeAddress generates a random validator address -func RandNodeAddress() NodeAddress { - nodeID := tmrand.Bytes(20) - port := (tmrand.Int() % 65535) + 1 - addr, err := ParseNodeAddress(fmt.Sprintf("tcp://%x@127.0.0.1:%d", nodeID, port)) - if err != nil { - panic(fmt.Sprintf("cannot generate random validator address: %s", err)) - } - if err := addr.Validate(); err != nil { - panic(fmt.Sprintf("randomly generated validator address %s is invalid: %s", addr.String(), err)) - } - return addr -} - // Endpoint represents a transport connection endpoint, either local or remote. // // Endpoints are not necessarily networked (see e.g. MemoryTransport) but all diff --git a/p2p/conn/secret_connection.go b/p2p/conn/secret_connection.go index 6d43dee177..0165a26ed0 100644 --- a/p2p/conn/secret_connection.go +++ b/p2p/conn/secret_connection.go @@ -90,10 +90,6 @@ type SecretConnection struct { // Caller should call conn.Close() // See docs/sts-final.pdf for more information. func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*SecretConnection, error) { - var ( - locPubKey = locPrivKey.PubKey() - ) - // Generate ephemeral keys for perfect forward secrecy. locEphPub, locEphPriv := genEphKeys() @@ -157,10 +153,20 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (* sendAead: sendAead, } - // SignDigest the challenge bytes for authentication. - locSignature, err := signChallenge(&challenge, locPrivKey) - if err != nil { - return nil, fmt.Errorf("sign challenge: %v", err) + // Exchange public keys + var ( + locPubKey crypto.PubKey + locSignature []byte + ) + // Allow no local private key to just retrieve remote privkey + if locPrivKey != nil { + locPubKey = locPrivKey.PubKey() + + // SignDigest the challenge bytes for authentication. + locSignature, err = signChallenge(&challenge, locPrivKey) + if err != nil { + return nil, fmt.Errorf("sign challenge: %v", err) + } } // Share (in secret) each other's pubkey & challenge signature @@ -404,18 +410,22 @@ type authSigMessage struct { Sig []byte } +// shareAuthSignature exchanges public keys, authorized by the signed challenge. +// If `pubKey` is nil, it will just retrieve public key without sending ours to the other party. func shareAuthSignature(sc io.ReadWriter, pubKey crypto.PubKey, signature []byte) (recvMsg authSigMessage, err error) { // Send our info and receive theirs in tandem. var trs, _ = async.Parallel( func(_ int) (val interface{}, abort bool, err error) { - pbpk, err := cryptoenc.PubKeyToProto(pubKey) - if err != nil { - return nil, true, err - } - _, err = protoio.NewDelimitedWriter(sc).WriteMsg(&tmp2p.AuthSigMessage{PubKey: pbpk, Sig: signature}) - if err != nil { - return nil, true, err // abort + if pubKey != nil { + pbpk, err := cryptoenc.PubKeyToProto(pubKey) + if err != nil { + return nil, true, err + } + _, err = protoio.NewDelimitedWriter(sc).WriteMsg(&tmp2p.AuthSigMessage{PubKey: pbpk, Sig: signature}) + if err != nil { + return nil, true, err // abort + } } return nil, false, nil }, diff --git a/p2p/conn/secret_connection_test.go b/p2p/conn/secret_connection_test.go index ca20c433ea..d197e9f86f 100644 --- a/p2p/conn/secret_connection_test.go +++ b/p2p/conn/secret_connection_test.go @@ -258,19 +258,30 @@ func TestDeriveSecretsAndChallengeGolden(t *testing.T) { } } +// TestNilPubkey ensures that we can retrieve peer public key without revealing ours func TestNilPubkey(t *testing.T) { var fooConn, barConn = makeKVStoreConnPair() - defer fooConn.Close() - defer barConn.Close() var fooPrvKey = ed25519.GenPrivKey() - var barPrvKey = privKeyWithNilPubKey{ed25519.GenPrivKey()} - go MakeSecretConnection(fooConn, fooPrvKey) //nolint:errcheck // ignore for tests - - _, err := MakeSecretConnection(barConn, barPrvKey) - require.Error(t, err) - wantErr := "toproto: key type is not supported" - assert.Containsf(t, err.Error(), wantErr, "expected error containing %q, got %s", wantErr, err) + wg := sync.WaitGroup{} + wg.Add(2) + // Foo thread never receives peer's public key + go func() { + _, err := MakeSecretConnection(fooConn, fooPrvKey) + fooConn.Close() + assert.Error(t, err) + wg.Done() + }() + // Bar thread never sends its public key, but receives peers + go func() { + sc, err := MakeSecretConnection(barConn, nil) + barConn.Close() + require.NoError(t, err) + require.NotNil(t, sc) + assert.NotNil(t, sc.RemotePubKey()) + wg.Done() + }() + wg.Wait() } func writeLots(t *testing.T, wg *sync.WaitGroup, conn io.Writer, txt string, n int) { diff --git a/state/execution.go b/state/execution.go index d6ef87cb02..f83671f7c7 100644 --- a/state/execution.go +++ b/state/execution.go @@ -7,7 +7,7 @@ import ( "time" "github.com/tendermint/tendermint/crypto/bls12381" - "github.com/tendermint/tendermint/p2p" + "github.com/tendermint/tendermint/dash/dashtypes" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" @@ -490,10 +490,8 @@ func validateValidatorUpdates(abciUpdates []abci.ValidatorUpdate, } // Check if validator's pubkey matches an ABCI type in the consensus params - var pk crypto.PubKey // needed for NodeAddress if valUpdate.PubKey != nil { - var err error - pk, err = cryptoenc.PubKeyFromProto(*valUpdate.PubKey) + pk, err := cryptoenc.PubKeyFromProto(*valUpdate.PubKey) if err != nil { return err } @@ -532,9 +530,8 @@ func validateValidatorUpdates(abciUpdates []abci.ValidatorUpdate, ) } - // Validate endpoint address; note that we need to support case where node ID is not set if valUpdate.NodeAddress != "" { - _, err := p2p.ParseNodeAddressWithPubkey(valUpdate.NodeAddress, pk) + _, err := dashtypes.ParseValidatorAddress(valUpdate.NodeAddress) if err != nil { return fmt.Errorf("cannot parse validator address %s: %w", valUpdate.NodeAddress, err) } diff --git a/state/execution_test.go b/state/execution_test.go index ebeb99e704..d1633a1534 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -8,7 +8,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" - "github.com/tendermint/tendermint/p2p" + "github.com/tendermint/tendermint/dash/dashtypes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -155,8 +155,7 @@ func TestValidateValidatorUpdates(t *testing.T) { PubKeyTypes: []string{types.ABCIPubKeyTypeBLS12381}, } - addr := p2p.RandNodeAddress() - addr.NodeID = "" + addr := dashtypes.RandValidatorAddress() testCases := []struct { name string @@ -367,7 +366,7 @@ func TestEndBlockValidatorUpdates(t *testing.T) { // Ensure new validators have some IP addresses set for _, validator := range newVals.Validators { - validator.NodeAddress = p2p.RandNodeAddress() + validator.NodeAddress = dashtypes.RandValidatorAddress() } app.ValidatorSetUpdate = newVals.ABCIEquivalentValidatorUpdates() diff --git a/state/state_test.go b/state/state_test.go index e033e3ccc4..5fd1169111 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -16,8 +16,8 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" cryptoenc "github.com/tendermint/tendermint/crypto/encoding" + "github.com/tendermint/tendermint/dash/dashtypes" tmrand "github.com/tendermint/tendermint/libs/rand" - "github.com/tendermint/tendermint/p2p" tmstate "github.com/tendermint/tendermint/proto/tendermint/state" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" sm "github.com/tendermint/tendermint/state" @@ -114,7 +114,7 @@ func TestABCIResponsesSaveLoad1(t *testing.T) { abciPubKey, err := cryptoenc.PubKeyToProto(pubKey) require.NoError(t, err) - vu := types.TM2PB.NewValidatorUpdate(pubKey, 100, crypto.RandProTxHash(), p2p.RandNodeAddress()) + vu := types.TM2PB.NewValidatorUpdate(pubKey, 100, crypto.RandProTxHash(), dashtypes.RandValidatorAddress()) abciResponses.EndBlock = &abci.ResponseEndBlock{ValidatorSetUpdate: &abci.ValidatorSetUpdate{ ValidatorUpdates: []abci.ValidatorUpdate{vu}, ThresholdPublicKey: abciPubKey, @@ -846,7 +846,7 @@ func TestFourAddFourMinusOneGenesisValidators(t *testing.T) { abciValidatorUpdates := make([]abci.ValidatorUpdate, len(proTxHashes)) for j, proTxHash := range proTxHashes { abciValidatorUpdates[j] = abci.UpdateValidator(proTxHash, privateKeys3[j].PubKey().Bytes(), - types.DefaultDashVotingPower, p2p.RandNodeAddress().String()) + types.DefaultDashVotingPower, dashtypes.RandValidatorAddress().String()) } abciThresholdPublicKey3, err := cryptoenc.PubKeyToProto(thresholdPublicKey3) assert.NoError(t, err) diff --git a/test/e2e/tests/validator_test.go b/test/e2e/tests/validator_test.go index 8d751f1630..d32fe41dd7 100644 --- a/test/e2e/tests/validator_test.go +++ b/test/e2e/tests/validator_test.go @@ -7,7 +7,7 @@ import ( "github.com/dashevo/dashd-go/btcjson" "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/p2p" + "github.com/tendermint/tendermint/dash/dashtypes" "github.com/stretchr/testify/require" @@ -69,7 +69,7 @@ func TestValidator_Sets(t *testing.T) { valScheduleValidator.ProTxHash, h, valScheduleValidator.PubKey.Bytes(), validator.PubKey.Bytes()) // Validators in the schedule don't contain addresses - validator.NodeAddress = p2p.NodeAddress{} + validator.NodeAddress = dashtypes.ValidatorAddress{} } require.Equal(t, valSchedule.Set.Validators, validators, "incorrect validator set at height %v", h) diff --git a/types/protobuf.go b/types/protobuf.go index 6f5cf004e0..a19ca40c8f 100644 --- a/types/protobuf.go +++ b/types/protobuf.go @@ -5,7 +5,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" - "github.com/tendermint/tendermint/p2p" + "github.com/tendermint/tendermint/dash/dashtypes" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/ed25519" @@ -136,8 +136,11 @@ func (tm2pb) ConsensusParams(params *tmproto.ConsensusParams) *abci.ConsensusPar // XXX: panics on nil or unknown pubkey type func (tm2pb) NewValidatorUpdate( - pubkey crypto.PubKey, power int64, proTxHash []byte, address p2p.NodeAddress) abci.ValidatorUpdate { - + pubkey crypto.PubKey, + power int64, + proTxHash []byte, + address dashtypes.ValidatorAddress, +) abci.ValidatorUpdate { var pubkeyABCI *crypto2.PublicKey if pubkey != nil { pubkeyProto, err := cryptoenc.PubKeyToProto(pubkey) diff --git a/types/protobuf_test.go b/types/protobuf_test.go index 16122386bf..593bd4c41a 100644 --- a/types/protobuf_test.go +++ b/types/protobuf_test.go @@ -6,7 +6,7 @@ import ( "github.com/dashevo/dashd-go/btcjson" "github.com/tendermint/tendermint/crypto/bls12381" - "github.com/tendermint/tendermint/p2p" + "github.com/tendermint/tendermint/dash/dashtypes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -85,16 +85,21 @@ func (pubKeyBLS) TypeValue() crypto.KeyType { retu func TestABCIValidatorFromPubKeyAndPower(t *testing.T) { pubkey := bls12381.GenPrivKey().PubKey() - address := p2p.RandNodeAddress() + address := dashtypes.RandValidatorAddress() abciVal := TM2PB.NewValidatorUpdate(pubkey, DefaultDashVotingPower, crypto.RandProTxHash(), address) assert.Equal(t, DefaultDashVotingPower, abciVal.Power) assert.Equal(t, address.String(), abciVal.NodeAddress) assert.NotPanics(t, func() { - TM2PB.NewValidatorUpdate(nil, DefaultDashVotingPower, crypto.RandProTxHash(), p2p.RandNodeAddress()) + TM2PB.NewValidatorUpdate(nil, DefaultDashVotingPower, crypto.RandProTxHash(), dashtypes.RandValidatorAddress()) }) assert.Panics(t, func() { - TM2PB.NewValidatorUpdate(pubKeyBLS{}, DefaultDashVotingPower, crypto.RandProTxHash(), p2p.RandNodeAddress()) + TM2PB.NewValidatorUpdate( + pubKeyBLS{}, + DefaultDashVotingPower, + crypto.RandProTxHash(), + dashtypes.RandValidatorAddress(), + ) }) } diff --git a/types/validator.go b/types/validator.go index 15fc726402..1b4aff7c6d 100644 --- a/types/validator.go +++ b/types/validator.go @@ -6,11 +6,10 @@ import ( "fmt" "strings" - "github.com/tendermint/tendermint/crypto/bls12381" - "github.com/tendermint/tendermint/p2p" - "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/bls12381" ce "github.com/tendermint/tendermint/crypto/encoding" + "github.com/tendermint/tendermint/dash/dashtypes" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) @@ -19,10 +18,10 @@ import ( // make sure to update that method if changes are made here // The ProTxHash is part of Dash additions required for BLS threshold signatures type Validator struct { - PubKey crypto.PubKey `json:"pub_key"` - VotingPower int64 `json:"voting_power"` - ProTxHash ProTxHash `json:"pro_tx_hash"` - NodeAddress p2p.NodeAddress `json:"address"` + PubKey crypto.PubKey `json:"pub_key"` + VotingPower int64 `json:"voting_power"` + ProTxHash ProTxHash `json:"pro_tx_hash"` + NodeAddress dashtypes.ValidatorAddress `json:"address"` ProposerPriority int64 `json:"proposer_priority"` } @@ -50,11 +49,11 @@ func NewValidatorDefaultVotingPower(pubKey crypto.PubKey, proTxHash []byte) *Val // NewValidator returns a new validator with the given pubkey and voting power. func NewValidator(pubKey crypto.PubKey, votingPower int64, proTxHash []byte, address string) *Validator { var ( - addr p2p.NodeAddress + addr dashtypes.ValidatorAddress err error ) if address != "" { - addr, err = p2p.ParseNodeAddressWithPubkey(address, pubKey) + addr, err = dashtypes.ParseValidatorAddress(address) if err != nil { panic(err.Error()) } @@ -237,20 +236,17 @@ func ValidatorFromProto(vp *tmproto.Validator) (*Validator, error) { v.ProposerPriority = vp.GetProposerPriority() v.ProTxHash = vp.ProTxHash + var err error if vp.PubKey != nil && vp.PubKey.Sum != nil { - pk, err := ce.PubKeyFromProto(*vp.PubKey) - if err != nil { + if v.PubKey, err = ce.PubKeyFromProto(*vp.PubKey); err != nil { return nil, err } - v.PubKey = pk } if vp.NodeAddress != "" { - address, err := p2p.ParseNodeAddress(vp.NodeAddress) - if err != nil { + if v.NodeAddress, err = dashtypes.ParseValidatorAddress(vp.NodeAddress); err != nil { return nil, err } - v.NodeAddress = address } return v, nil diff --git a/types/validator_set.go b/types/validator_set.go index 1ce893cbd0..2e0bc40bae 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -10,14 +10,12 @@ import ( "strings" "github.com/dashevo/dashd-go/btcjson" - "github.com/tendermint/tendermint/crypto/merkle" - "github.com/tendermint/tendermint/p2p" - abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" cryptoenc "github.com/tendermint/tendermint/crypto/encoding" - + "github.com/tendermint/tendermint/crypto/merkle" + "github.com/tendermint/tendermint/dash/dashtypes" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) @@ -1539,7 +1537,7 @@ func ValidatorUpdatesRegenerateOnProTxHashes(proTxHashes []crypto.ProTxHash) abc privateKeys[i].PubKey(), DefaultDashVotingPower, orderedProTxHashes[i], - p2p.NodeAddress{}, + dashtypes.ValidatorAddress{}, ) valUpdates = append(valUpdates, valUpdate) } diff --git a/types/validator_test.go b/types/validator_test.go index a7a1cb58a7..ac46af7e71 100644 --- a/types/validator_test.go +++ b/types/validator_test.go @@ -121,12 +121,17 @@ func TestNewValidator(t *testing.T) { validator := NewValidator(pubKey, DefaultDashVotingPower, proTxHash, fmt.Sprintf("tcp://%s@127.0.0.1:12345", nodeID)) require.NotNil(t, validator) - assert.Equal(t, nodeID, validator.NodeAddress.NodeID) + newNodeID, err := validator.NodeAddress.NodeID() + require.NoError(t, err) + assert.Equal(t, nodeID, newNodeID) validator = NewValidator(pubKey, DefaultDashVotingPower, proTxHash, "127.0.0.1:23456") require.NotNil(t, validator) - assert.EqualValues(t, "127.0.0.1", validator.NodeAddress.Hostname) - assert.EqualValues(t, 23456, validator.NodeAddress.Port) - assert.EqualValues(t, "tcp", validator.NodeAddress.Protocol) - assert.EqualValues(t, nodeID, validator.NodeAddress.NodeID) + assert.EqualValues(t, "127.0.0.1", validator.NodeAddress.Hostname()) + assert.EqualValues(t, 23456, validator.NodeAddress.Port()) + assert.EqualValues(t, "tcp", validator.NodeAddress.Protocol()) + newNodeID, err = validator.NodeAddress.NodeID() + require.Error(t, err) + assert.Contains(t, err.Error(), "connection refused") + assert.Zero(t, newNodeID) } From 573da7526285a2feb55d0cbfc0e7175fe7c13eeb Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Fri, 26 Nov 2021 13:52:23 +0100 Subject: [PATCH 05/13] test(p2p/conn): remove unused privKeyWithNilPubKey --- dash/dashtypes/validator_address.go | 3 ++- p2p/conn/secret_connection_test.go | 13 ------------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/dash/dashtypes/validator_address.go b/dash/dashtypes/validator_address.go index bea1f54dff..bab1939707 100644 --- a/dash/dashtypes/validator_address.go +++ b/dash/dashtypes/validator_address.go @@ -52,7 +52,7 @@ func (va ValidatorAddress) Port() uint16 { return va.NodeAddress.Port } -// Protocol returns protocl name of this address, like "tcp" +// Protocol returns protocol name of this address, like "tcp" func (va ValidatorAddress) Protocol() string { return va.NodeAddress.Protocol } @@ -92,6 +92,7 @@ func (va ValidatorAddress) retrieveNodeID() (p2p.ID, error) { return "", err } + // TODO create a simplified version of MakeSecretConnection to just retrieve the public key sc, err := conn.MakeSecretConnection(connection, nil) if err != nil { return "", err diff --git a/p2p/conn/secret_connection_test.go b/p2p/conn/secret_connection_test.go index d197e9f86f..d1c4f541b2 100644 --- a/p2p/conn/secret_connection_test.go +++ b/p2p/conn/secret_connection_test.go @@ -17,7 +17,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/libs/async" tmos "github.com/tendermint/tendermint/libs/os" @@ -42,18 +41,6 @@ func (drw kvstoreConn) Close() (err error) { return err1 } -type privKeyWithNilPubKey struct { - orig crypto.PrivKey -} - -func (pk privKeyWithNilPubKey) Bytes() []byte { return pk.orig.Bytes() } -func (pk privKeyWithNilPubKey) Sign(msg []byte) ([]byte, error) { return pk.orig.Sign(msg) } -func (pk privKeyWithNilPubKey) SignDigest(msg []byte) ([]byte, error) { return pk.orig.SignDigest(msg) } -func (pk privKeyWithNilPubKey) PubKey() crypto.PubKey { return nil } -func (pk privKeyWithNilPubKey) Equals(pk2 crypto.PrivKey) bool { return pk.orig.Equals(pk2) } -func (pk privKeyWithNilPubKey) Type() string { return "privKeyWithNilPubKey" } -func (pk privKeyWithNilPubKey) TypeValue() crypto.KeyType { return crypto.KeyTypeAny } - func TestSecretConnectionHandshake(t *testing.T) { fooSecConn, barSecConn := makeSecretConnPair(t) if err := fooSecConn.Close(); err != nil { From caa650f7a2b448622095ca7c3bc024547362c576 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Fri, 26 Nov 2021 13:52:32 +0100 Subject: [PATCH 06/13] doc(adr-d001): inter-validator set node id lookup --- .../adr-d001-inter-validator-set-messaging.md | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/docs/architecture/adr-d001-inter-validator-set-messaging.md b/docs/architecture/adr-d001-inter-validator-set-messaging.md index 99213020e4..bd88251ef6 100644 --- a/docs/architecture/adr-d001-inter-validator-set-messaging.md +++ b/docs/architecture/adr-d001-inter-validator-set-messaging.md @@ -21,6 +21,8 @@ 2. [What systems will be affected?](#what-systems-will-be-affected) 3. [What new data structures are needed, what data structures need changes?](#what-new-data-structures-are-needed-what-data-structures-need-changes) 4. [What new APIs will be needed, what APIs will change?](#what-new-apis-will-be-needed-what-apis-will-change) + 1. [ABCI Protocol](#abci-protocol) + 2. [P2P Handshake](#p2p-handshake) 5. [What are the efficiency considerations (time/space)?](#what-are-the-efficiency-considerations-timespace) 6. [What are the expected access patterns (load/throughput)?](#what-are-the-expected-access-patterns-loadthroughput) 7. [Are there any logging, monitoring, or observability needs?](#are-there-any-logging-monitoring-or-observability-needs) @@ -127,10 +129,11 @@ through another Validator) to any other Validator which is a member of the same ### What new data structures are needed, what data structures need changes? -None +Implemented `dashtypes.ValidatorAddress` which represents an address of a validator received from ABCI app. ### What new APIs will be needed, what APIs will change? +#### ABCI Protocol In the ABCI protocol, we add a network address of each member of the active Validator Set to the `ResponseEndBlock.ValidatorSetUpdate.ValidatorUpdate` structure. The network address should be an URI: @@ -140,7 +143,7 @@ tcp://@
: where: -* `` - node ID (can be generated based on node public key, see [p2p.PubKeyToID](https://github.com/dashevo/tenderdash/blob/20d1f91c0adb29d1b5d03a945b17513a368759e4/p2p/key.go#L45)) +* `` - node ID (can be generated based on node P2P public key, see [p2p.PubKeyToID](https://github.com/dashevo/tenderdash/blob/20d1f91c0adb29d1b5d03a945b17513a368759e4/p2p/key.go#L45)) * `
` - IP address of the validator node * `` - p2p port number of the validator node @@ -156,9 +159,14 @@ message ValidatorUpdate { ``` -If ABCI app cannot determine node address, it can leave it empty. +If ABCI app cannot determine ``, it can leave it empty. In this case, Tenderdash will try to retrieve a public +key from remote host and derive node ID from it. -Note that the Dash Drive as an ABCI app should be able to determine node address if it is member of the active Validator Set. If the node is NOT a member of active Validator Set, the `node_address` should be empty. +#### P2P Handshake + +To retrieve node ID from a **remote node**, **local node** can establish a short-lived connection to the **remote node** to download its public key. This feature does not require any changes to the P2P protocol; it just uses existing capabilities. + +Public key request starts with a normal flow Diffie-Hellman protocol, as implemented in `conn.MakeSecretConnection()`. Once epheremal keys are exchanged between nodes, **local node** proceeds to `shareAuthSignature()`. However, instead of sending its own `AuthSigMessage`, it waits for **remote node**'s auth signature to arrive.As the communication is asynchronous, the **remote node** sends its `AuthSigMessage`. That's when the **local node** closes the connection. ### What are the efficiency considerations (time/space)? @@ -174,19 +182,28 @@ In the future, additional optimizations can further improve the performance, for 1. Each rotation of the active Validator Set will cause the ABCI application to send additional data to Tenderdash. The number of blocks between rotation events is a configuration parameter of the ABCI application. 2. Each validator in the active validator set will establish a few more tcp connections. +3. Retrieving public keys for validators can be resource and time-intensive, as it establishes new connections to validators and performs a DH handshake. This issue is mitigated by: + a. ensuring that this feature is used only when needed + b. refactoring and simplification of the P2P protocol and source code used to retrieve public keys + Note that for the needs of Inter-Validator Set Messaging, retrieving public keys is executed only for `log2(n-1)` Validators, where `n` is size of active Validator Set. + ### Are there any logging, monitoring, or observability needs? 1. Operator shall be able to determine a list of node peers, together with their type (Full, inactive Validator, active Validator) and connection status. This information can be part of debug logs. ### Are there any security considerations? -This change makes connectivity between Validators more predictable. +1. This change makes connectivity between Validators more predictable. It can make it a bit easier for a malicious user to deliberately block communication for a given Validator node to make it unable to participate in the Validator Set. - However, this risk is already present at the ABCI application level, so the change does not introduce new risks. +2. Missing node ID can be determined based on a public key, which is retrieved without proper validation. It cannot be guaranteed that it will not be spoofed. + + This can allow an attacker who controls the network connectivity to perform a man-in-the-middle attack. As the attacker doesn't have validator private keys, he/she cannot perform unauthorized operations. However, it is possible to intercept communication and isolate the attacked validator from the quorum. + + Note that, if the attacker controls the network connectivity, he can still drop that traffic on the network level. ### Are there any privacy considerations? From c5bce8f7b66ae8c8cf042fd925bd600d9eae3567 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Dec 2021 10:34:29 +0100 Subject: [PATCH 07/13] refactor(quorum): apply code review feedback --- dash/dashtypes/nodeid_resolver.go | 69 ++++++++++++++++++++++++ dash/dashtypes/validator_address.go | 56 +++++++++---------- dash/dashtypes/validator_address_test.go | 3 +- dash/quorum/mock/test_helpers.go | 3 +- dash/quorum/validator_conn_executor.go | 20 ++++--- 5 files changed, 109 insertions(+), 42 deletions(-) create mode 100644 dash/dashtypes/nodeid_resolver.go diff --git a/dash/dashtypes/nodeid_resolver.go b/dash/dashtypes/nodeid_resolver.go new file mode 100644 index 0000000000..9157145a6e --- /dev/null +++ b/dash/dashtypes/nodeid_resolver.go @@ -0,0 +1,69 @@ +package dashtypes + +import ( + "fmt" + "net" + "time" + + "github.com/tendermint/tendermint/p2p" + "github.com/tendermint/tendermint/p2p/conn" +) + +const ( + DefaultDialTimeout = 1000 * time.Millisecond + DefaultConnectionTimeout = 1 * time.Second +) + +type NodeIDResolver interface { + Resolve(ValidatorAddress) (p2p.ID, error) +} + +type nodeIDResolver struct { + DialerTimeout time.Duration + ConnectionTimeout time.Duration + // other dependencies +} + +func NewNodeIDResolver() NodeIDResolver { + return &nodeIDResolver{ + DialerTimeout: DefaultDialTimeout, + ConnectionTimeout: DefaultConnectionTimeout, + } +} + +// connect establishes a TCP connection to remote host. +// When err == nil, caller is responsible for closing of the connection +func (resolver nodeIDResolver) connect(host string, port uint16) (net.Conn, error) { + dialer := net.Dialer{ + Timeout: resolver.DialerTimeout, + } + + connection, err := dialer.Dial("tcp4", fmt.Sprintf("%s:%d", host, port)) + if err != nil { + return nil, fmt.Errorf("cannot lookup node ID: %w", err) + } + + if err := connection.SetDeadline(time.Now().Add(resolver.ConnectionTimeout)); err != nil { + connection.Close() + return nil, err + } + + return connection, nil +} + +// Resolve retrieves a node ID from remote node. +// Note that it is quite expensive, as it establishes secure connection to the other node +// which is dropped afterwards. +func (resolver nodeIDResolver) Resolve(va ValidatorAddress) (p2p.ID, error) { + connection, err := resolver.connect(va.Hostname(), va.Port()) + if err != nil { + return "", err + } + defer connection.Close() + + sc, err := conn.MakeSecretConnection(connection, nil) + if err != nil { + return "", err + } + return p2p.PubKeyToID(sc.RemotePubKey()), nil +} diff --git a/dash/dashtypes/validator_address.go b/dash/dashtypes/validator_address.go index bab1939707..7025544606 100644 --- a/dash/dashtypes/validator_address.go +++ b/dash/dashtypes/validator_address.go @@ -3,19 +3,25 @@ package dashtypes import ( "errors" "fmt" - "net" - "time" + "math" tmrand "github.com/tendermint/tendermint/libs/rand" "github.com/tendermint/tendermint/p2p" - "github.com/tendermint/tendermint/p2p/conn" ) // ValidatorAddress is a NodeAddress that does not require node ID to be set type ValidatorAddress struct { p2p.NodeAddress + resolver NodeIDResolver } +var ( + ErrNoHostname error = errors.New("no hostname") + ErrNoPort error = errors.New("no port") + ErrNoProtocol error = errors.New("no protocol") + ErrNoResolver error = errors.New("resolver not defined, validator address not initialized correctly") +) + // ParseValidatorAddress parses provided address, which should be in `proto://nodeID@host:port` form. // `proto://` and `nodeID@` parts are optional. func ParseValidatorAddress(address string) (ValidatorAddress, error) { @@ -23,7 +29,10 @@ func ParseValidatorAddress(address string) (ValidatorAddress, error) { if err != nil { return ValidatorAddress{}, err } - va := ValidatorAddress{NodeAddress: addr} + va := ValidatorAddress{ + NodeAddress: addr, + resolver: NewNodeIDResolver(), + } return va, va.Validate() } @@ -31,13 +40,13 @@ func ParseValidatorAddress(address string) (ValidatorAddress, error) { // It ignores missing node IDs. func (va ValidatorAddress) Validate() error { if va.NodeAddress.Protocol == "" { - return errors.New("no protocol") + return ErrNoProtocol } if va.NodeAddress.Hostname == "" { - return errors.New("no hostname") + return ErrNoHostname } if va.NodeAddress.Port <= 0 { - return errors.New("no port") + return ErrNoPort } return nil } @@ -70,7 +79,12 @@ func (va ValidatorAddress) NetAddress() (*p2p.NetAddress, error) { func (va *ValidatorAddress) NodeID() (p2p.ID, error) { if va.NodeAddress.NodeID == "" { var err error - va.NodeAddress.NodeID, err = va.retrieveNodeID() + + if va.resolver == nil { + return "", ErrNoResolver + } + + va.NodeAddress.NodeID, err = va.resolver.Resolve(*va) if err != nil { return "", err } @@ -78,32 +92,10 @@ func (va *ValidatorAddress) NodeID() (p2p.ID, error) { return va.NodeAddress.NodeID, nil } -// retrieveNodeID retrieves a node ID from remote node. -// Note that it is quite expensive, as it establishes secure connection to the other node -// which is dropped afterwards. -func (va ValidatorAddress) retrieveNodeID() (p2p.ID, error) { - dialer := net.Dialer{Timeout: 1000 * time.Millisecond} - connection, err := dialer.Dial("tcp4", fmt.Sprintf("%s:%d", va.NodeAddress.Hostname, va.NodeAddress.Port)) - if err != nil { - return "", fmt.Errorf("cannot lookup node ID: %w", err) - } - defer connection.Close() - if err := connection.SetDeadline(time.Now().Add(1 * time.Second)); err != nil { - return "", err - } - - // TODO create a simplified version of MakeSecretConnection to just retrieve the public key - sc, err := conn.MakeSecretConnection(connection, nil) - if err != nil { - return "", err - } - return p2p.PubKeyToID(sc.RemotePubKey()), nil -} - -// dashtypes.RandValidatorAddress generates a random validator address +// RandValidatorAddress generates a random validator address. Used in tests. func RandValidatorAddress() ValidatorAddress { nodeID := tmrand.Bytes(20) - port := (tmrand.Int() % 65535) + 1 + port := tmrand.Int()%math.MaxUint16 + 1 addr, err := ParseValidatorAddress(fmt.Sprintf("tcp://%x@127.0.0.1:%d", nodeID, port)) if err != nil { panic(fmt.Sprintf("cannot generate random validator address: %s", err)) diff --git a/dash/dashtypes/validator_address_test.go b/dash/dashtypes/validator_address_test.go index 76703f513c..315117eb0c 100644 --- a/dash/dashtypes/validator_address_test.go +++ b/dash/dashtypes/validator_address_test.go @@ -7,7 +7,6 @@ import ( ) func TestValidatorAddress_String(t *testing.T) { - tests := []struct { uri string want string @@ -54,7 +53,7 @@ func TestValidatorAddress_NodeID_fail(t *testing.T) { va, err := ParseValidatorAddress(tt.uri) assert.NoError(t, err) got, err := va.NodeID() - assert.Equal(t, err != nil, tt.wantErr, "wantErr=%s, but err = %s", tt.wantErr, err) + assert.Equal(t, err != nil, tt.wantErr, "wantErr=%t, but err = %s", tt.wantErr, err) assert.EqualValues(t, tt.want, got) }) } diff --git a/dash/quorum/mock/test_helpers.go b/dash/quorum/mock/test_helpers.go index 9a567c2f96..21beb83080 100644 --- a/dash/quorum/mock/test_helpers.go +++ b/dash/quorum/mock/test_helpers.go @@ -3,6 +3,7 @@ package mock import ( "encoding/binary" "fmt" + "math" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/dash/dashtypes" @@ -16,7 +17,7 @@ func NewNodeAddress(n uint64) string { nodeID := make([]byte, 20) binary.LittleEndian.PutUint64(nodeID, n) if n == 0 { - n = 65535 + n = math.MaxUint16 } return fmt.Sprintf("tcp://%x@127.0.0.1:%d", nodeID, uint16(n)) } diff --git a/dash/quorum/validator_conn_executor.go b/dash/quorum/validator_conn_executor.go index e303a52027..7a4cdf01f9 100644 --- a/dash/quorum/validator_conn_executor.go +++ b/dash/quorum/validator_conn_executor.go @@ -232,10 +232,10 @@ func (vc *ValidatorConnExecutor) setQuorumHash(newQuorumHash tmbytes.HexBytes) e return nil } -// me returns current node's validator object, if any, or nil if not found -func (vc *ValidatorConnExecutor) me() *types.Validator { - me := vc.validatorSetMembers[validatorMapIndexType(vc.proTxHash.String())] - return &me +// me returns current node's validator object, if any. `ok` if false when current node is not a validator +func (vc *ValidatorConnExecutor) me() (validator *types.Validator, ok bool) { + v, ok := vc.validatorSetMembers[validatorMapIndexType(vc.proTxHash.String())] + return &v, ok } // selectValidators selects `count` validators from current ValidatorSet. @@ -243,8 +243,8 @@ func (vc *ValidatorConnExecutor) me() *types.Validator { // Returns map indexed by validator address. func (vc *ValidatorConnExecutor) selectValidators() (validatorMap, error) { activeValidators := vc.validatorSetMembers - me := vc.me() - if me == nil { + me, ok := vc.me() + if !ok { return validatorMap{}, fmt.Errorf("current node is not member of active validator set") } @@ -296,11 +296,17 @@ func (vc *ValidatorConnExecutor) disconnectValidators(exceptions validatorMap) e return nil } +// isValidator returns true when current node is a validator +func (vc *ValidatorConnExecutor) isValidator() bool { + _, ok := vc.me() + return ok +} + // updateConnections processes current validator set, selects a few validators and schedules connections // to be established. It will also disconnect previous validators. func (vc *ValidatorConnExecutor) updateConnections() error { // We only do something if we are part of new ValidatorSet - if me := vc.me(); me == nil { + if !vc.isValidator() { vc.Logger.Debug("not a member of active ValidatorSet") // We need to disconnect connected validators. It needs to be done explicitly // because they are marked as persistent and will never disconnect themselves. From 22618fa97361854e664d853f1d30509bba9a1e50 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Dec 2021 10:38:19 +0100 Subject: [PATCH 08/13] refactor(dash/types): rename package dashtypes to types --- abci/example/kvstore/helpers.go | 2 +- dash/quorum/mock/mock_switch.go | 2 +- dash/quorum/mock/test_helpers.go | 2 +- dash/quorum/validator_conn_executor_test.go | 2 +- dash/{dashtypes => types}/nodeid_resolver.go | 2 +- dash/{dashtypes => types}/validator_address.go | 2 +- dash/{dashtypes => types}/validator_address_test.go | 2 +- node/node_test.go | 2 +- state/execution.go | 2 +- state/execution_test.go | 2 +- state/state_test.go | 2 +- test/e2e/tests/validator_test.go | 2 +- types/protobuf.go | 2 +- types/protobuf_test.go | 2 +- types/validator.go | 2 +- types/validator_set.go | 2 +- 16 files changed, 16 insertions(+), 16 deletions(-) rename dash/{dashtypes => types}/nodeid_resolver.go (98%) rename dash/{dashtypes => types}/validator_address.go (99%) rename dash/{dashtypes => types}/validator_address_test.go (98%) diff --git a/abci/example/kvstore/helpers.go b/abci/example/kvstore/helpers.go index d9ce769090..068e8ffa62 100644 --- a/abci/example/kvstore/helpers.go +++ b/abci/example/kvstore/helpers.go @@ -5,7 +5,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" cryptoenc "github.com/tendermint/tendermint/crypto/encoding" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" ) func ValUpdate( diff --git a/dash/quorum/mock/mock_switch.go b/dash/quorum/mock/mock_switch.go index 9bec5f47f8..e3ff892b9e 100644 --- a/dash/quorum/mock/mock_switch.go +++ b/dash/quorum/mock/mock_switch.go @@ -5,7 +5,7 @@ import ( "strings" "time" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/p2p/mocks" ) diff --git a/dash/quorum/mock/test_helpers.go b/dash/quorum/mock/test_helpers.go index 21beb83080..fcd1654191 100644 --- a/dash/quorum/mock/test_helpers.go +++ b/dash/quorum/mock/test_helpers.go @@ -6,7 +6,7 @@ import ( "math" "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" "github.com/tendermint/tendermint/libs/bytes" "github.com/tendermint/tendermint/types" ) diff --git a/dash/quorum/validator_conn_executor_test.go b/dash/quorum/validator_conn_executor_test.go index 2611968372..6d83885be3 100644 --- a/dash/quorum/validator_conn_executor_test.go +++ b/dash/quorum/validator_conn_executor_test.go @@ -10,9 +10,9 @@ import ( "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/dash/dashtypes" "github.com/tendermint/tendermint/dash/quorum/mock" "github.com/tendermint/tendermint/dash/quorum/selectpeers" + dashtypes "github.com/tendermint/tendermint/dash/types" tmbytes "github.com/tendermint/tendermint/libs/bytes" "github.com/tendermint/tendermint/libs/log" mmock "github.com/tendermint/tendermint/mempool/mock" diff --git a/dash/dashtypes/nodeid_resolver.go b/dash/types/nodeid_resolver.go similarity index 98% rename from dash/dashtypes/nodeid_resolver.go rename to dash/types/nodeid_resolver.go index 9157145a6e..2633e29a30 100644 --- a/dash/dashtypes/nodeid_resolver.go +++ b/dash/types/nodeid_resolver.go @@ -1,4 +1,4 @@ -package dashtypes +package types import ( "fmt" diff --git a/dash/dashtypes/validator_address.go b/dash/types/validator_address.go similarity index 99% rename from dash/dashtypes/validator_address.go rename to dash/types/validator_address.go index 7025544606..497389b670 100644 --- a/dash/dashtypes/validator_address.go +++ b/dash/types/validator_address.go @@ -1,4 +1,4 @@ -package dashtypes +package types import ( "errors" diff --git a/dash/dashtypes/validator_address_test.go b/dash/types/validator_address_test.go similarity index 98% rename from dash/dashtypes/validator_address_test.go rename to dash/types/validator_address_test.go index 315117eb0c..e14c02cff3 100644 --- a/dash/dashtypes/validator_address_test.go +++ b/dash/types/validator_address_test.go @@ -1,4 +1,4 @@ -package dashtypes +package types import ( "testing" diff --git a/node/node_test.go b/node/node_test.go index 2f3981fef1..378cb3fa95 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -10,7 +10,7 @@ import ( "time" "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/state/execution.go b/state/execution.go index f83671f7c7..38f749c482 100644 --- a/state/execution.go +++ b/state/execution.go @@ -7,7 +7,7 @@ import ( "time" "github.com/tendermint/tendermint/crypto/bls12381" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" diff --git a/state/execution_test.go b/state/execution_test.go index d1633a1534..4126a3f8d7 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -8,7 +8,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" diff --git a/state/state_test.go b/state/state_test.go index 5fd1169111..3a840271ee 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -16,7 +16,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" cryptoenc "github.com/tendermint/tendermint/crypto/encoding" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" tmrand "github.com/tendermint/tendermint/libs/rand" tmstate "github.com/tendermint/tendermint/proto/tendermint/state" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" diff --git a/test/e2e/tests/validator_test.go b/test/e2e/tests/validator_test.go index d32fe41dd7..4017caa055 100644 --- a/test/e2e/tests/validator_test.go +++ b/test/e2e/tests/validator_test.go @@ -7,7 +7,7 @@ import ( "github.com/dashevo/dashd-go/btcjson" "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" "github.com/stretchr/testify/require" diff --git a/types/protobuf.go b/types/protobuf.go index a19ca40c8f..1a84183fdf 100644 --- a/types/protobuf.go +++ b/types/protobuf.go @@ -5,7 +5,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/ed25519" diff --git a/types/protobuf_test.go b/types/protobuf_test.go index 593bd4c41a..0eb275861e 100644 --- a/types/protobuf_test.go +++ b/types/protobuf_test.go @@ -6,7 +6,7 @@ import ( "github.com/dashevo/dashd-go/btcjson" "github.com/tendermint/tendermint/crypto/bls12381" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/types/validator.go b/types/validator.go index 1b4aff7c6d..4ed0d7bd47 100644 --- a/types/validator.go +++ b/types/validator.go @@ -9,7 +9,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/bls12381" ce "github.com/tendermint/tendermint/crypto/encoding" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) diff --git a/types/validator_set.go b/types/validator_set.go index 2e0bc40bae..501a2ec815 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -15,7 +15,7 @@ import ( "github.com/tendermint/tendermint/crypto/bls12381" cryptoenc "github.com/tendermint/tendermint/crypto/encoding" "github.com/tendermint/tendermint/crypto/merkle" - "github.com/tendermint/tendermint/dash/dashtypes" + dashtypes "github.com/tendermint/tendermint/dash/types" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) From 8c550e7df1d1ef9ce552c2871be2d7e23e485f02 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Dec 2021 11:57:29 +0100 Subject: [PATCH 09/13] refactor(p2p): make ValidateID exportable --- p2p/address.go | 5 +++-- p2p/netaddress.go | 8 ++++---- p2p/switch.go | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/p2p/address.go b/p2p/address.go index fb0f7a8c71..430480145c 100644 --- a/p2p/address.go +++ b/p2p/address.go @@ -28,7 +28,8 @@ var ( // reSchemeIsHost tries to detect URLs where the scheme part is instead a // hostname, i.e. of the form "host:80/path" where host: is a hostname. - reSchemeIsHost = regexp.MustCompile(`^[^/:]+:\d+(/|$)`) + reSchemeIsHost = regexp.MustCompile(`^[^/:]+:\d+(/|$)`) + ErrEmptyNodeAddress = errors.New("empty node address") ) // NodeAddress is a node address URL. It differs from a transport Endpoint in @@ -49,7 +50,7 @@ type NodeAddress struct { // and validating it. func ParseNodeAddress(urlString string) (NodeAddress, error) { if urlString == "" { - return NodeAddress{}, fmt.Errorf("empty node address") + return NodeAddress{}, ErrEmptyNodeAddress } address, err := ParseNodeAddressWithoutValidation(urlString) if err != nil { diff --git a/p2p/netaddress.go b/p2p/netaddress.go index 77209217b2..925e0ba1ea 100644 --- a/p2p/netaddress.go +++ b/p2p/netaddress.go @@ -52,7 +52,7 @@ func NewNetAddress(id ID, addr net.Addr) *NetAddress { } } - if err := validateID(id); err != nil { + if err := ValidateID(id); err != nil { panic(fmt.Sprintf("Invalid ID %v: %v (addr: %v)", id, err, addr)) } @@ -75,7 +75,7 @@ func NewNetAddressString(addr string) (*NetAddress, error) { } // get ID - if err := validateID(ID(spl[0])); err != nil { + if err := ValidateID(ID(spl[0])); err != nil { return nil, ErrNetAddressInvalid{addrWithoutProtocol, err} } var id ID @@ -262,7 +262,7 @@ func (na *NetAddress) Routable() bool { // For IPv4 these are either a 0 or all bits set address. For IPv6 a zero // address or one that matches the RFC3849 documentation address format. func (na *NetAddress) Valid() error { - if err := validateID(na.ID); err != nil { + if err := ValidateID(na.ID); err != nil { return fmt.Errorf("invalid ID: %w", err) } @@ -405,7 +405,7 @@ func removeProtocolIfDefined(addr string) string { } -func validateID(id ID) error { +func ValidateID(id ID) error { if len(id) == 0 { return errors.New("no ID") } diff --git a/p2p/switch.go b/p2p/switch.go index 7ccdd0dba5..102e42c38b 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -605,7 +605,7 @@ func (sw *Switch) RemovePersistentPeer(addr string) error { func (sw *Switch) AddUnconditionalPeerIDs(ids []string) error { sw.Logger.Info("Adding unconditional peer ids", "ids", ids) for i, id := range ids { - err := validateID(ID(id)) + err := ValidateID(ID(id)) if err != nil { return fmt.Errorf("wrong ID #%d: %w", i, err) } @@ -617,7 +617,7 @@ func (sw *Switch) AddUnconditionalPeerIDs(ids []string) error { func (sw *Switch) AddPrivatePeerIDs(ids []string) error { validIDs := make([]string, 0, len(ids)) for i, id := range ids { - err := validateID(ID(id)) + err := ValidateID(ID(id)) if err != nil { return fmt.Errorf("wrong ID #%d: %w", i, err) } From b194e76780b4d75c1ab189b65228d2e23e3e0ab1 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Dec 2021 11:58:26 +0100 Subject: [PATCH 10/13] test(dash/types): increase validator address test coverage --- dash/types/validator_address.go | 7 ++ dash/types/validator_address_test.go | 114 ++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 4 deletions(-) diff --git a/dash/types/validator_address.go b/dash/types/validator_address.go index 497389b670..426c532f34 100644 --- a/dash/types/validator_address.go +++ b/dash/types/validator_address.go @@ -20,6 +20,7 @@ var ( ErrNoPort error = errors.New("no port") ErrNoProtocol error = errors.New("no protocol") ErrNoResolver error = errors.New("resolver not defined, validator address not initialized correctly") + ErrNoNodeID error = errors.New("no node ID") ) // ParseValidatorAddress parses provided address, which should be in `proto://nodeID@host:port` form. @@ -48,6 +49,12 @@ func (va ValidatorAddress) Validate() error { if va.NodeAddress.Port <= 0 { return ErrNoPort } + if len(va.NodeAddress.NodeID) > 0 { + if err := p2p.ValidateID(va.NodeAddress.NodeID); err != nil { + return err + } + } + return nil } diff --git a/dash/types/validator_address_test.go b/dash/types/validator_address_test.go index e14c02cff3..b708251dd9 100644 --- a/dash/types/validator_address_test.go +++ b/dash/types/validator_address_test.go @@ -1,19 +1,23 @@ package types import ( + "encoding/hex" "testing" "github.com/stretchr/testify/assert" + "github.com/tendermint/tendermint/libs/rand" + "github.com/tendermint/tendermint/p2p" ) func TestValidatorAddress_String(t *testing.T) { + nodeID := randNodeID() tests := []struct { uri string want string }{ { - uri: "tcp://node@fqdn.address.com:1234", - want: "tcp://node@fqdn.address.com:1234", + uri: "tcp://" + nodeID + "@fqdn.address.com:1234", + want: "tcp://" + nodeID + "@fqdn.address.com:1234", }, { uri: "tcp://fqdn.address.com:1234", @@ -33,6 +37,7 @@ func TestValidatorAddress_String(t *testing.T) { // TestValidatorAddress_NodeID_fail checks if NodeID lookup fails when trying to connect to ssh port // NOTE: Positive flow is tested as part of node_test.go TestNodeStartStop() func TestValidatorAddress_NodeID_fail(t *testing.T) { + nodeID := randNodeID() tests := []struct { uri string @@ -40,8 +45,8 @@ func TestValidatorAddress_NodeID_fail(t *testing.T) { wantErr bool }{ { - uri: "tcp://node@fqdn.address.com:1234", - want: "node", + uri: "tcp://" + nodeID + "@fqdn.address.com:1234", + want: nodeID, }, { uri: "tcp://127.0.0.1:22", @@ -58,3 +63,104 @@ func TestValidatorAddress_NodeID_fail(t *testing.T) { }) } } + +// TestValidatorAddress_HostPortProto verifies if host, port and proto is detected correctly when parsing +// ValidatorAddress +func TestValidatorAddress_HostPortProto(t *testing.T) { + nodeID := randNodeID() + + tests := []struct { + uri string + wantHost string + wantPort uint16 + wantProto string + wantNodeID string + wantValid bool + wantParseErr bool + }{ + { + uri: "tcp://" + nodeID + "@fqdn.address.com:1234", + wantHost: "fqdn.address.com", + wantPort: 1234, + wantProto: "tcp", + wantNodeID: nodeID, + wantValid: true, + }, + { + uri: "tcp://test@fqdn.address.com:1234", + wantHost: "fqdn.address.com", + wantPort: 1234, + wantProto: "tcp", + wantValid: false, + wantParseErr: true, + }, + { + uri: "tcp://127.0.0.1:22", + wantHost: "127.0.0.1", + wantPort: 22, + wantProto: "tcp", + wantValid: true, + }, + { + uri: "", + wantValid: false, + wantParseErr: true, + }, + { + uri: "tcp://127.0.0.1", + wantHost: "127.0.0.1", + wantPort: 0, + wantProto: "tcp", + wantValid: false, + wantParseErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.uri, func(t *testing.T) { + va, err := ParseValidatorAddress(tt.uri) + if tt.wantParseErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.EqualValues(t, tt.wantHost, va.Hostname()) + assert.EqualValues(t, tt.wantPort, va.Port()) + assert.EqualValues(t, tt.wantProto, va.Protocol()) + + if tt.wantNodeID != "" { + nodeID, err := va.NodeID() + assert.NoError(t, err) + assert.EqualValues(t, tt.wantNodeID, nodeID) + } + err = va.Validate() + if tt.wantValid { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + } + }) + } +} + +func TestValidatorAddress_NetAddress(t *testing.T) { + nodeID := randNodeID() + uri := "tcp://" + nodeID + "@127.0.0.1:1234" + + va, err := ParseValidatorAddress(uri) + assert.NoError(t, err) + + naddr, err := va.NetAddress() + assert.NoError(t, err) + assert.NoError(t, naddr.Valid()) + assert.EqualValues(t, naddr.IP.String(), "127.0.0.1") + assert.EqualValues(t, naddr.Port, 1234) + assert.EqualValues(t, naddr.ID, nodeID) +} + +// utility functions + +func randNodeID() string { + nodeID := rand.Bytes(p2p.IDByteLength) + return hex.EncodeToString(nodeID) +} From 7a25a223448b6e3a4e782821e44acfc8a0181095 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 2 Dec 2021 13:43:32 +0100 Subject: [PATCH 11/13] test(e2e): no node id in validator update address --- test/e2e/pkg/testnet.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/pkg/testnet.go b/test/e2e/pkg/testnet.go index 33a540c6c6..7bc6057eb8 100644 --- a/test/e2e/pkg/testnet.go +++ b/test/e2e/pkg/testnet.go @@ -729,7 +729,7 @@ func (n *Node) ValidatorUpdate(publicKey []byte) (abci.ValidatorUpdate, error) { // TODO TD-10 find real power power := types.DefaultDashVotingPower - address := n.AddressP2P(true) + address := n.AddressP2P(false) validatorUpdate := abci.UpdateValidator(proTxHash, publicKey, power, address) return validatorUpdate, nil } From ca4e933dcdedc3d3044717a1ac94dd170ad51dc9 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 6 Dec 2021 10:25:20 +0100 Subject: [PATCH 12/13] fix: node id not retrieved when processing val updates --- dash/quorum/validator_conn_executor.go | 8 ++++++++ dash/types/nodeid_resolver.go | 2 ++ dash/types/validator_address_test.go | 10 ++++++++++ 3 files changed, 20 insertions(+) diff --git a/dash/quorum/validator_conn_executor.go b/dash/quorum/validator_conn_executor.go index 7a4cdf01f9..74d1f63c89 100644 --- a/dash/quorum/validator_conn_executor.go +++ b/dash/quorum/validator_conn_executor.go @@ -253,6 +253,14 @@ func (vc *ValidatorConnExecutor) selectValidators() (validatorMap, error) { if err != nil { return validatorMap{}, err } + // fetch node IDs + for _, validator := range selectedValidators { + _, err := validator.NodeAddress.NodeID() + if err != nil { + vc.Logger.Debug("cannot determine node id for validator", "url", validator.String(), "error", err) + return nil, err + } + } return newValidatorMap(selectedValidators), nil } diff --git a/dash/types/nodeid_resolver.go b/dash/types/nodeid_resolver.go index 2633e29a30..a213f434e8 100644 --- a/dash/types/nodeid_resolver.go +++ b/dash/types/nodeid_resolver.go @@ -15,6 +15,7 @@ const ( ) type NodeIDResolver interface { + // Resolve retrieves a node ID from remote node. Resolve(ValidatorAddress) (p2p.ID, error) } @@ -51,6 +52,7 @@ func (resolver nodeIDResolver) connect(host string, port uint16) (net.Conn, erro return connection, nil } +// Resolve implements NodeIDResolver // Resolve retrieves a node ID from remote node. // Note that it is quite expensive, as it establishes secure connection to the other node // which is dropped afterwards. diff --git a/dash/types/validator_address_test.go b/dash/types/validator_address_test.go index b708251dd9..96f7a4c952 100644 --- a/dash/types/validator_address_test.go +++ b/dash/types/validator_address_test.go @@ -64,6 +64,16 @@ func TestValidatorAddress_NodeID_fail(t *testing.T) { } } +// TestValidatorAddress_NodeID_Positive +// func TestValidatorAddress_NodeID_Positive(t *testing.T) { +// uri := "tcp://10.186.73.7:26656" +// va, err := ParseValidatorAddress(uri) +// assert.NoError(t, err) +// got, err := va.NodeID() +// assert.NoError(t, err) +// t.Log("Node ID: " + got) +// } + // TestValidatorAddress_HostPortProto verifies if host, port and proto is detected correctly when parsing // ValidatorAddress func TestValidatorAddress_HostPortProto(t *testing.T) { From 2fb5152ca09abdc64e067d27296550cd79bbbeb0 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 9 Dec 2021 11:20:32 +0100 Subject: [PATCH 13/13] feat(dash/quorum): lookup node id in address book --- dash/quorum/mock/mock_switch.go | 8 +++- dash/quorum/validator_conn_executor.go | 40 +++++++++++++++++--- dash/quorum/validator_conn_executor_test.go | 7 +++- dash/types/nodeid_resolver.go | 42 ++++++++++++++++++--- dash/types/validator_address.go | 39 +------------------ dash/types/validator_address_test.go | 14 +++---- node/node_test.go | 4 +- p2p/pex/addrbook.go | 13 +++++++ p2p/switch.go | 7 ++++ p2p/test_util.go | 8 +++- types/validator_test.go | 13 +++---- 11 files changed, 126 insertions(+), 69 deletions(-) diff --git a/dash/quorum/mock/mock_switch.go b/dash/quorum/mock/mock_switch.go index e3ff892b9e..b872fc8ba5 100644 --- a/dash/quorum/mock/mock_switch.go +++ b/dash/quorum/mock/mock_switch.go @@ -31,6 +31,7 @@ type Switch struct { PersistentPeers map[string]bool History []SwitchHistoryEvent HistoryChan chan SwitchHistoryEvent + AddressBook p2p.AddrBook } // NewMockSwitch creates a new mock Switch @@ -40,10 +41,15 @@ func NewMockSwitch() *Switch { PersistentPeers: map[string]bool{}, History: []SwitchHistoryEvent{}, HistoryChan: make(chan SwitchHistoryEvent, 1000), + AddressBook: &p2p.AddrBookMock{}, } return isw } +func (sw *Switch) AddrBook() p2p.AddrBook { + return sw.AddressBook +} + // Peers implements Switch by returning sw.PeerSet func (sw *Switch) Peers() p2p.IPeerSet { return sw.PeerSet @@ -80,7 +86,7 @@ func (sw *Switch) DialPeersAsync(addrs []string) error { return err } - peer.On("ID").Return(parsed.NodeID()) + peer.On("ID").Return(parsed.NodeID) peer.On("String").Return(addr) if err := sw.PeerSet.Add(peer); err != nil { return err diff --git a/dash/quorum/validator_conn_executor.go b/dash/quorum/validator_conn_executor.go index 74d1f63c89..e4a91ad171 100644 --- a/dash/quorum/validator_conn_executor.go +++ b/dash/quorum/validator_conn_executor.go @@ -7,6 +7,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/dash/quorum/selectpeers" + dashtypes "github.com/tendermint/tendermint/dash/types" tmbytes "github.com/tendermint/tendermint/libs/bytes" "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/libs/service" @@ -35,6 +36,8 @@ type Switch interface { DialPeersAsync(addrs []string) error IsDialingOrExistingAddress(*p2p.NetAddress) bool StopPeerGracefully(p2p.Peer) + + AddrBook() p2p.AddrBook } type optionFunc func(vc *ValidatorConnExecutor) error @@ -61,6 +64,9 @@ type ValidatorConnExecutor struct { // quorumHash contains current quorum hash quorumHash tmbytes.HexBytes + // nodeIDResolvers can be used to determine a node ID for a validator + nodeIDResolvers []dashtypes.NodeIDResolver + // mux is a mutex to ensure only one goroutine is processing connections mux sync.Mutex @@ -86,6 +92,10 @@ func NewValidatorConnExecutor( validatorSetMembers: validatorMap{}, connectedValidators: validatorMap{}, quorumHash: make(tmbytes.HexBytes, crypto.QuorumHashSize), + nodeIDResolvers: []dashtypes.NodeIDResolver{ + dashtypes.NewAddrbookNodeIDResolver(sw.AddrBook()), + dashtypes.NewTCPNodeIDResolver(), + }, } baseService := service.NewBaseService(log.NewNopLogger(), validatorConnExecutorName, vc) vc.BaseService = *baseService @@ -238,6 +248,22 @@ func (vc *ValidatorConnExecutor) me() (validator *types.Validator, ok bool) { return &v, ok } +// resolveNodeID adds node ID to the validator address if it's not set +func (vc *ValidatorConnExecutor) resolveNodeID(va *dashtypes.ValidatorAddress) error { + if va.NodeID != "" { + return nil + } + for _, resolver := range vc.nodeIDResolvers { + nid, err := resolver.Resolve(*va) + if err == nil && nid != "" { + va.NodeID = nid + return nil + } + vc.Logger.Debug("node id not found, trying another method", "url", va.String(), "error", err) + } + return dashtypes.ErrNoNodeID +} + // selectValidators selects `count` validators from current ValidatorSet. // It uses algorithm described in DIP-6. // Returns map indexed by validator address. @@ -255,10 +281,10 @@ func (vc *ValidatorConnExecutor) selectValidators() (validatorMap, error) { } // fetch node IDs for _, validator := range selectedValidators { - _, err := validator.NodeAddress.NodeID() + err := vc.resolveNodeID(&validator.NodeAddress) if err != nil { vc.Logger.Debug("cannot determine node id for validator", "url", validator.String(), "error", err) - return nil, err + // no return, as it's not critical } } @@ -274,10 +300,12 @@ func (vc *ValidatorConnExecutor) disconnectValidator(validator types.Validator) return err } - id, err := validator.NodeAddress.NodeID() + err = vc.resolveNodeID(&validator.NodeAddress) if err != nil { return err } + id := validator.NodeAddress.NodeID + peer := vc.p2pSwitch.Peers().Get(id) if peer == nil { return fmt.Errorf("cannot stop peer %s: not found", id) @@ -331,9 +359,11 @@ func (vc *ValidatorConnExecutor) updateConnections() error { if err := vc.disconnectValidators(newValidators); err != nil { return fmt.Errorf("cannot disconnect unused validators: %w", err) } + vc.Logger.Debug("filtering validators", "validators", newValidators) // ensure that we can connect to all validators newValidators = vc.filterAddresses(newValidators) // Connect to new validators + vc.Logger.Debug("dialing validators", "validators", newValidators) if err := vc.dial(newValidators); err != nil { return fmt.Errorf("cannot dial validators: %w", err) } @@ -346,7 +376,7 @@ func (vc *ValidatorConnExecutor) filterAddresses(validators validatorMap) valida filtered := make(validatorMap, len(validators)) for id, validator := range validators { if vc.connectedValidators.contains(validator) { - vc.Logger.P2PDebug("validator already connected", "id", id) + vc.Logger.Debug("validator already connected", "id", id) continue } address, err := validator.NodeAddress.NetAddress() @@ -355,7 +385,7 @@ func (vc *ValidatorConnExecutor) filterAddresses(validators validatorMap) valida continue } if vc.p2pSwitch.IsDialingOrExistingAddress(address) { - vc.Logger.P2PDebug("already dialing this validator", "id", id, "address", address.String()) + vc.Logger.Debug("already dialing this validator", "id", id, "address", address.String()) continue } filtered[id] = validator diff --git a/dash/quorum/validator_conn_executor_test.go b/dash/quorum/validator_conn_executor_test.go index 6d83885be3..2724e827be 100644 --- a/dash/quorum/validator_conn_executor_test.go +++ b/dash/quorum/validator_conn_executor_test.go @@ -2,6 +2,7 @@ package quorum import ( "context" + "encoding/hex" "fmt" "testing" "time" @@ -16,6 +17,7 @@ import ( tmbytes "github.com/tendermint/tendermint/libs/bytes" "github.com/tendermint/tendermint/libs/log" mmock "github.com/tendermint/tendermint/mempool/mock" + "github.com/tendermint/tendermint/p2p" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" "github.com/tendermint/tendermint/proxy" sm "github.com/tendermint/tendermint/state" @@ -61,9 +63,10 @@ func TestValidatorConnExecutor_NotValidator(t *testing.T) { // TestValidatorConnExecutor_WrongAddress checks behavior in case of several issues in the address. // Expected behavior: invalid address is dialed. Previous addresses are disconnected. func TestValidatorConnExecutor_WrongAddress(t *testing.T) { - me := mock.NewValidator(65535) - addr1, err := dashtypes.ParseValidatorAddress("http://john@www.google.com:80") + zeroBytes := make([]byte, p2p.IDByteLength) + nodeID := hex.EncodeToString(zeroBytes) + addr1, err := dashtypes.ParseValidatorAddress("http://" + nodeID + "@www.domain-that-does-not-exist.com:80") require.NoError(t, err) val1 := mock.NewValidator(100) diff --git a/dash/types/nodeid_resolver.go b/dash/types/nodeid_resolver.go index a213f434e8..fefadc0632 100644 --- a/dash/types/nodeid_resolver.go +++ b/dash/types/nodeid_resolver.go @@ -19,14 +19,14 @@ type NodeIDResolver interface { Resolve(ValidatorAddress) (p2p.ID, error) } -type nodeIDResolver struct { +type tcpNodeIDResolver struct { DialerTimeout time.Duration ConnectionTimeout time.Duration // other dependencies } -func NewNodeIDResolver() NodeIDResolver { - return &nodeIDResolver{ +func NewTCPNodeIDResolver() NodeIDResolver { + return &tcpNodeIDResolver{ DialerTimeout: DefaultDialTimeout, ConnectionTimeout: DefaultConnectionTimeout, } @@ -34,7 +34,7 @@ func NewNodeIDResolver() NodeIDResolver { // connect establishes a TCP connection to remote host. // When err == nil, caller is responsible for closing of the connection -func (resolver nodeIDResolver) connect(host string, port uint16) (net.Conn, error) { +func (resolver tcpNodeIDResolver) connect(host string, port uint16) (net.Conn, error) { dialer := net.Dialer{ Timeout: resolver.DialerTimeout, } @@ -56,8 +56,8 @@ func (resolver nodeIDResolver) connect(host string, port uint16) (net.Conn, erro // Resolve retrieves a node ID from remote node. // Note that it is quite expensive, as it establishes secure connection to the other node // which is dropped afterwards. -func (resolver nodeIDResolver) Resolve(va ValidatorAddress) (p2p.ID, error) { - connection, err := resolver.connect(va.Hostname(), va.Port()) +func (resolver tcpNodeIDResolver) Resolve(va ValidatorAddress) (p2p.ID, error) { + connection, err := resolver.connect(va.Hostname, va.Port) if err != nil { return "", err } @@ -69,3 +69,33 @@ func (resolver nodeIDResolver) Resolve(va ValidatorAddress) (p2p.ID, error) { } return p2p.PubKeyToID(sc.RemotePubKey()), nil } + +type addrbookNodeIDResolver struct { + addrBook p2p.AddrBook +} + +// NewAddrbookNodeIDResolver creates new node ID resolver. +// It looks up fora node ID based on IP address, using the p2p addressbook. +func NewAddrbookNodeIDResolver(addrBook p2p.AddrBook) NodeIDResolver { + return addrbookNodeIDResolver{addrBook: addrBook} +} + +// Resolve implements NodeIDResolver +// Resolve retrieves a node ID from address book. +func (resolver addrbookNodeIDResolver) Resolve(va ValidatorAddress) (p2p.ID, error) { + ip := net.ParseIP(va.Hostname) + if ip == nil { + ips, err := net.LookupIP(va.Hostname) + if err != nil { + return "", p2p.ErrNetAddressLookup{Addr: va.Hostname, Err: err} + } + ip = ips[0] + } + + id := resolver.addrBook.FindIP(ip, va.Port) + if id == "" { + return "", ErrNoNodeID + } + + return id, nil +} diff --git a/dash/types/validator_address.go b/dash/types/validator_address.go index 426c532f34..e99c42a797 100644 --- a/dash/types/validator_address.go +++ b/dash/types/validator_address.go @@ -12,7 +12,6 @@ import ( // ValidatorAddress is a NodeAddress that does not require node ID to be set type ValidatorAddress struct { p2p.NodeAddress - resolver NodeIDResolver } var ( @@ -32,7 +31,6 @@ func ParseValidatorAddress(address string) (ValidatorAddress, error) { } va := ValidatorAddress{ NodeAddress: addr, - resolver: NewNodeIDResolver(), } return va, va.Validate() } @@ -58,47 +56,14 @@ func (va ValidatorAddress) Validate() error { return nil } -// Hostname returns host name of this address -func (va ValidatorAddress) Hostname() string { - return va.NodeAddress.Hostname -} - -// Port returns port number of this address -func (va ValidatorAddress) Port() uint16 { - return va.NodeAddress.Port -} - -// Protocol returns protocol name of this address, like "tcp" -func (va ValidatorAddress) Protocol() string { - return va.NodeAddress.Protocol -} - // NetAddress returns this ValidatorAddress as a *p2p.NetAddress that can be used to establish connection func (va ValidatorAddress) NetAddress() (*p2p.NetAddress, error) { - if _, err := va.NodeID(); err != nil { - return nil, fmt.Errorf("cannot determine node id for address %s: %w", va.String(), err) + if va.NodeID == "" { + return nil, fmt.Errorf("cannot determine node id for address %s", va.String()) } return va.NodeAddress.NetAddress() } -// NodeID() returns node ID. If it is not set, it will connect to remote node, retrieve its public key -// and calculate Node ID based on it. Noe this connection can be expensive. -func (va *ValidatorAddress) NodeID() (p2p.ID, error) { - if va.NodeAddress.NodeID == "" { - var err error - - if va.resolver == nil { - return "", ErrNoResolver - } - - va.NodeAddress.NodeID, err = va.resolver.Resolve(*va) - if err != nil { - return "", err - } - } - return va.NodeAddress.NodeID, nil -} - // RandValidatorAddress generates a random validator address. Used in tests. func RandValidatorAddress() ValidatorAddress { nodeID := tmrand.Bytes(20) diff --git a/dash/types/validator_address_test.go b/dash/types/validator_address_test.go index 96f7a4c952..1a1dd45619 100644 --- a/dash/types/validator_address_test.go +++ b/dash/types/validator_address_test.go @@ -57,8 +57,9 @@ func TestValidatorAddress_NodeID_fail(t *testing.T) { t.Run(tt.uri, func(t *testing.T) { va, err := ParseValidatorAddress(tt.uri) assert.NoError(t, err) - got, err := va.NodeID() - assert.Equal(t, err != nil, tt.wantErr, "wantErr=%t, but err = %s", tt.wantErr, err) + // todo lookup for an address + got := va.NodeID + // assert.Equal(t, err != nil, tt.wantErr, "wantErr=%t, but err = %s", tt.wantErr, err) assert.EqualValues(t, tt.want, got) }) } @@ -133,13 +134,12 @@ func TestValidatorAddress_HostPortProto(t *testing.T) { assert.Error(t, err) } else { assert.NoError(t, err) - assert.EqualValues(t, tt.wantHost, va.Hostname()) - assert.EqualValues(t, tt.wantPort, va.Port()) - assert.EqualValues(t, tt.wantProto, va.Protocol()) + assert.EqualValues(t, tt.wantHost, va.Hostname) + assert.EqualValues(t, tt.wantPort, va.Port) + assert.EqualValues(t, tt.wantProto, va.Protocol) if tt.wantNodeID != "" { - nodeID, err := va.NodeID() - assert.NoError(t, err) + nodeID := va.NodeID assert.EqualValues(t, tt.wantNodeID, nodeID) } err = va.Validate() diff --git a/node/node_test.go b/node/node_test.go index 378cb3fa95..166c117689 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -61,9 +61,9 @@ func TestNodeStartStop(t *testing.T) { // check if we can read node ID of this node va, err := dashtypes.ParseValidatorAddress(config.P2P.ListenAddress) assert.NoError(t, err) - id, err := va.NodeID() - assert.NoError(t, err) + id, err := dashtypes.NewTCPNodeIDResolver().Resolve(va) assert.Equal(t, n.nodeInfo.ID(), id) + assert.NoError(t, err) // stop the node go func() { diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go index 76b21e8dc4..fb84f173a4 100644 --- a/p2p/pex/addrbook.go +++ b/p2p/pex/addrbook.go @@ -50,6 +50,9 @@ type AddrBook interface { // Check if the address is in the book HasAddress(*p2p.NetAddress) bool + // Find by IP address + FindIP(net.IP, uint16) p2p.ID + // Do we need more peers? NeedMoreAddrs() bool // Is Address Book Empty? Answer should not depend on being in your own @@ -695,6 +698,16 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error { return a.addToNewBucket(ka, bucket) } +func (a *addrBook) FindIP(ip net.IP, port uint16) p2p.ID { + for nodeID, item := range a.addrLookup { + if item.Addr != nil && item.Addr.IP.Equal(ip) && item.Addr.Port == port { + return nodeID + } + } + + return "" +} + func (a *addrBook) randomPickAddresses(bucketType byte, num int) []*p2p.NetAddress { var buckets []map[string]*knownAddress switch bucketType { diff --git a/p2p/switch.go b/p2p/switch.go index 102e42c38b..901cbd8125 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -3,6 +3,7 @@ package p2p import ( "fmt" "math" + "net" "sync" "time" @@ -52,6 +53,7 @@ type AddrBook interface { MarkGood(ID) RemoveAddress(*NetAddress) HasAddress(*NetAddress) bool + FindIP(ip net.IP, port uint16) ID Save() } @@ -433,6 +435,11 @@ func (sw *Switch) SetAddrBook(addrBook AddrBook) { sw.addrBook = addrBook } +// AddrBook() returns address book used by a switch +func (sw *Switch) AddrBook() AddrBook { + return sw.addrBook +} + // MarkPeerAsGood marks the given peer as good when it did something useful // like contributed to consensus. func (sw *Switch) MarkPeerAsGood(peer Peer) { diff --git a/p2p/test_util.go b/p2p/test_util.go index d3534e8dcc..f07101799f 100644 --- a/p2p/test_util.go +++ b/p2p/test_util.go @@ -1,17 +1,17 @@ package p2p import ( + "encoding/hex" "fmt" "net" "time" + "github.com/tendermint/tendermint/config" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/libs/log" tmnet "github.com/tendermint/tendermint/libs/net" tmrand "github.com/tendermint/tendermint/libs/rand" - - "github.com/tendermint/tendermint/config" "github.com/tendermint/tendermint/p2p/conn" ) @@ -316,3 +316,7 @@ func (book *AddrBookMock) AddPrivateIDs(addrs []string) { book.PrivateAddrs[addr] = struct{}{} } } + +func (book *AddrBookMock) FindIP(net.IP, uint16) ID { + return ID(hex.EncodeToString(tmrand.Bytes(IDByteLength))) +} diff --git a/types/validator_test.go b/types/validator_test.go index ac46af7e71..b323e545e2 100644 --- a/types/validator_test.go +++ b/types/validator_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/tendermint/tendermint/crypto" + dashtypes "github.com/tendermint/tendermint/dash/types" "github.com/tendermint/tendermint/p2p" "github.com/stretchr/testify/assert" @@ -121,17 +122,15 @@ func TestNewValidator(t *testing.T) { validator := NewValidator(pubKey, DefaultDashVotingPower, proTxHash, fmt.Sprintf("tcp://%s@127.0.0.1:12345", nodeID)) require.NotNil(t, validator) - newNodeID, err := validator.NodeAddress.NodeID() - require.NoError(t, err) + newNodeID := validator.NodeAddress.NodeID assert.Equal(t, nodeID, newNodeID) validator = NewValidator(pubKey, DefaultDashVotingPower, proTxHash, "127.0.0.1:23456") require.NotNil(t, validator) - assert.EqualValues(t, "127.0.0.1", validator.NodeAddress.Hostname()) - assert.EqualValues(t, 23456, validator.NodeAddress.Port()) - assert.EqualValues(t, "tcp", validator.NodeAddress.Protocol()) - newNodeID, err = validator.NodeAddress.NodeID() - require.Error(t, err) + assert.EqualValues(t, "127.0.0.1", validator.NodeAddress.Hostname) + assert.EqualValues(t, 23456, validator.NodeAddress.Port) + assert.EqualValues(t, "tcp", validator.NodeAddress.Protocol) + newNodeID, err = dashtypes.NewTCPNodeIDResolver().Resolve(validator.NodeAddress) assert.Contains(t, err.Error(), "connection refused") assert.Zero(t, newNodeID) }