feat: autodetect NodeID in ValdatorUpdate (TD-40) #213
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lklimek
force-pushed
the
TD-40-empty-node-id
branch
from
5cb7535
to
b4809f0
Compare
Codecov Report
@@ Coverage Diff @@
## TD-30-dedicated-slots #213 +/- ##
=========================================================
+ Coverage 45.21% 45.25% +0.03%
=========================================================
Files 286 288 +2
Lines 52793 52896 +103
=========================================================
+ Hits 23872 23938 +66
- Misses 26964 26993 +29
- Partials 1957 1965 +8
Continue to review full report at Codecov.
|
lklimek
force-pushed
the
TD-40-empty-node-id
branch
2 times, most recently
from
6e787d6
to
0ce2c02
Compare
lklimek
force-pushed
the
TD-40-empty-node-id
branch
from
4492723
to
7c9ad9b
Compare
shotonoff
reviewed
Nov 30, 2021
dash/dashtypes/validator_address.go
Outdated
| 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)) |
There was a problem hiding this comment.
RandValidatorAddress() is only used in tests. Changing it to return err makes it much harder. Besides, these error conditions should never happen.
lklimek
force-pushed
the
TD-40-empty-node-id
branch
from
dec29a5
to
d5fc0e0
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
waiting for integration testing |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
lklimek
force-pushed
the
TD-40-empty-node-id
branch
from
3e5a950
to
2fb5152
Compare
lklimek
changed the title
Convert validators map to string before logging. Without this change, elasticsearch maps each validator as a separate field, like: `json.validators.1AD56AAACE6385A240B3A85EC231ACDE3972CD21A09678A7924B28116314A29C.address.Hostname`
lklimek
added a commit
that referenced
this pull request
Jan 7, 2022
This commit adds the capability to establish direct communication between multiple validators that are members of the same validator set. See [ADR D001](https://github.com/dashevo/tenderdash/blob/bb69b12ef3fb45a38e85e05662bd93f907f153fc/docs/architecture/adr-d001-inter-validator-set-messaging.md) for more details. * feat: Add address to ABCI ValidatorUpdate message * feat:ipaddress parsing logic added * refactor: reorganize protobuf dependency tree * test: ipaddress unit tests added + some fixes * refactor: avoid large changes in *.proto to make backporting easier * refactor: move lines around to avoid future conflicts * refactor: format .proto files using `make proto-format` * refactor: use factory pattern for IPAddress * doc: fix comment of IPAddress.MustParseIP * refactor: iptables.Copy() improvements * refactor: move mustParseIP() to ipaddress_test.go * refactor: use String URI for validator address * feat: update code to work with abci.ValidatorUpdate.Address * test: updated tests to support abci ValidatorUpdate.Address * test: validator update address - unit tests green * refactor: add ValidatorAddress type * e2e/tests: fix validator address test * refactor: make linter happy * dash: validator connection executor, WIP * feat: Add address to ABCI ValidatorUpdate message * feat:ipaddress parsing logic added * refactor: reorganize protobuf dependency tree * test: ipaddress unit tests added + some fixes * refactor: avoid large changes in *.proto to make backporting easier * refactor: move lines around to avoid future conflicts * refactor: format .proto files using `make proto-format` * refactor: use factory pattern for IPAddress * doc: fix comment of IPAddress.MustParseIP * refactor: iptables.Copy() improvements * refactor: move mustParseIP() to ipaddress_test.go * refactor: use String URI for validator address * feat: update code to work with abci.ValidatorUpdate.Address * test: updated tests to support abci ValidatorUpdate.Address * test: validator update address - unit tests green * refactor: add ValidatorAddress type * e2e/tests: fix validator address test * refactor: make linter happy * refactor: rename file with ValidatorConnExecutor * feat:inter validator set - work in progress * e2e: enable p2p_debug and add more validator updates * feat: inter validator set messaging * test: inter validator set communication * doc: update adr-d001 to reflect current implementation * p2p: test TestTransportHandshake - wait for server to start * refactor: code cleanup of inter validator set * refactor: replace types.ValidatorAddress with p2p.NodeAddress * p2p: allow empty validator address * state: remove unused code * test(p2p): TestTransportHandshake - add some sync * test: improve TestValidatorConnExecutor tests * refactor: rneame validator(update).Address to NodeAddress * typo: fix linter issue * refactor: apply code review feedback (WIP) * refactor(dash): apply code review feedback, continued * refactor(dash): improve validatorMap * state: Add QuorumHash to EventValidatorSetUpdates * feat: select validators to connect directly according to DIP-6 (WIP) * fix: fix rebase conflicts * feat: algorithm to select validators according to DIP-6 * refactor: rename select_nodes.go to select_validators.go * feat: Use DIP-6 to select Validators for inter-validator set comm * dash: add some code comments * refactor: move files to separate packages * test: fix e2e tests for single and simple networks Validators in validator set contain address only after first quorum rotation. * refactor: apply code review feedback * refactor: store validator as value (not pointer) internally * refactor: change package names in dash/ * refactor(selectpeers): rename SelectValidatorsDIP6 to DIP6 * refactor: minor code formatting fixes * refactor: Apply suggestions from code review Co-authored-by: Dmitrii Golubev <dmitrii.golubev@deliveryhero.com> * refactor: minor code improvements * refactor: minor code improvements * test: revert handshake timeout * refactor: apply peer review feedback * refactor: fix comment Co-authored-by: Dmitrii Golubev <dmitrii.golubev@deliveryhero.com> * doc(adr-d001): update ValidatorUpdate struct * fix(selectpeers): add fallback for less than 5 validators * refactor(selectpeers): Add ValidatorSelector interface * test(p2p): tune sleep time when waiting for port to get opened * 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 * feat: autodetect NodeID in ValdatorUpdate (TD-40) (#213) * feat: autodetect NodeID in ValdatorUpdate * feat(dash/quorum): handle validators without address * feat(quorum): Autodetect node ID when connecting to quorum members * test(p2p/conn): remove unused privKeyWithNilPubKey * doc(adr-d001): inter-validator set node id lookup * refactor(quorum): apply code review feedback * refactor(dash/types): rename package dashtypes to types * refactor(p2p): make ValidateID exportable * test(dash/types): increase validator address test coverage * test(e2e): no node id in validator update address * fix: node id not retrieved when processing val updates * feat(dash/quorum): lookup node id in address book * fix(dash/quorum): log validators list as single string Convert validators map to string before logging. Without this change, elasticsearch maps each validator as a separate field, like: `json.validators.1AD56AAACE6385A240B3A85EC231ACDE3972CD21A09678A7924B28116314A29C.address.Hostname` * refactor: code comments, improved strings * doc(adr-d001): update doc after implementation * doc(adr-d001): apply feedback Co-authored-by: shotonoff <shotonoff@gmail.com>
lklimek
added a commit
that referenced
this pull request
Feb 15, 2022
* feat: improve logging for better elasticsearch compatibility (#220) Unified logging of height and round, fixed some bugs: * consensus: improve logging of vote messages to get better reporting * fix(pex): use Logger instead of Printf This fixes issues with parsing of logs for elastic * fix(scripts): consensus.CommitMessage not defined in wal2json * refactor(consensus,log): Unify logging of object metadata * fix(types): invalid vote sign bytes put into error msg * refactor(p2p): log ping-pong on P2PDebug level * refactor(log): revert addition of LogableObject * refactor(consensus): apply code review feedback * fix: fix the most of the compile errors * chore: update import section * feat(abci): InitChain can set initial core lock height (#222) * feat(consensus): InitChain can set initial core height * test(consensus): initial core height unit test * test(e2e): allow setting of core height in resp to InitApp * refactor(e2e): unify cfg name init_app_core_chain_locked_height * test(consensus): remove not needed InitChain response * fix(consensus): fix last core chain lock detection tests * refactor(abci): remove last_core_chain_locked_height from ResponseInfo * refactor(statesync): fix lint warnings * feat(maverick): support initial core height from InitChain * test(e2e): revert chainlock update height to 1000 in rotate.toml * fix: regenerate remote_client.go mock, some fixes by feedback * fix: remove unused global variables * build(deps): bump github.com/rs/cors from 1.8.0 to 1.8.2 Bumps [github.com/rs/cors](https://github.com/rs/cors) from 1.8.0 to 1.8.2. - [Release notes](https://github.com/rs/cors/releases) - [Commits](rs/cors@v1.8.0...v1.8.2) --- updated-dependencies: - dependency-name: github.com/rs/cors dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * fix: some fixes for 0.35 backport * fix: some fixes for 0.35 backport * feat(consensus): add empty block on h-1 and h-2 apphash change (#241) * feat(consensus): add empty block on h-1 and h-2 apphash change Before this change, empty block was created when apphash was changed since previous (height-1) block. This change changes this behavior to also check block (height-2). * test(consensus): empty block on h-1 and h-2 apphash change * rebase(consensus): apply code review feedback * fix(evidence): fix all failed tests in evidence package * fix(store): fix unit tests for internal/store package * fix(blocksync): fix unit tests for internal/blocksync package * fix(p2p): remove redundant sleep from TestMConnTransport_Listen * fix(blocksync): fic blocksync/v2 test * fix(rpc): allow reading "request_quorum_info" param for rpc "validators" handler * fix(state): fix internal/state package * fix(state): fix internal/statesync package * fix(test): fix internal/test/factory package * fix(privval): some fixes for unit tests * fix(privval): fix tests TestPrivvalVectors, TestStateSyncVectors * fix(consensus): fix consensus logic and tests * fix: remove generating AppHash from genesis * fix: remove debug printing * fix(mempool): fix TestSerialReap * fix(rpc): fix TestBroadcastEvidence_DuplicateVoteEvidence * feat: inter-validator set communication (#187) This commit adds the capability to establish direct communication between multiple validators that are members of the same validator set. See [ADR D001](https://github.com/dashevo/tenderdash/blob/bb69b12ef3fb45a38e85e05662bd93f907f153fc/docs/architecture/adr-d001-inter-validator-set-messaging.md) for more details. * feat: Add address to ABCI ValidatorUpdate message * feat:ipaddress parsing logic added * refactor: reorganize protobuf dependency tree * test: ipaddress unit tests added + some fixes * refactor: avoid large changes in *.proto to make backporting easier * refactor: move lines around to avoid future conflicts * refactor: format .proto files using `make proto-format` * refactor: use factory pattern for IPAddress * doc: fix comment of IPAddress.MustParseIP * refactor: iptables.Copy() improvements * refactor: move mustParseIP() to ipaddress_test.go * refactor: use String URI for validator address * feat: update code to work with abci.ValidatorUpdate.Address * test: updated tests to support abci ValidatorUpdate.Address * test: validator update address - unit tests green * refactor: add ValidatorAddress type * e2e/tests: fix validator address test * refactor: make linter happy * dash: validator connection executor, WIP * feat: Add address to ABCI ValidatorUpdate message * feat:ipaddress parsing logic added * refactor: reorganize protobuf dependency tree * test: ipaddress unit tests added + some fixes * refactor: avoid large changes in *.proto to make backporting easier * refactor: move lines around to avoid future conflicts * refactor: format .proto files using `make proto-format` * refactor: use factory pattern for IPAddress * doc: fix comment of IPAddress.MustParseIP * refactor: iptables.Copy() improvements * refactor: move mustParseIP() to ipaddress_test.go * refactor: use String URI for validator address * feat: update code to work with abci.ValidatorUpdate.Address * test: updated tests to support abci ValidatorUpdate.Address * test: validator update address - unit tests green * refactor: add ValidatorAddress type * e2e/tests: fix validator address test * refactor: make linter happy * refactor: rename file with ValidatorConnExecutor * feat:inter validator set - work in progress * e2e: enable p2p_debug and add more validator updates * feat: inter validator set messaging * test: inter validator set communication * doc: update adr-d001 to reflect current implementation * p2p: test TestTransportHandshake - wait for server to start * refactor: code cleanup of inter validator set * refactor: replace types.ValidatorAddress with p2p.NodeAddress * p2p: allow empty validator address * state: remove unused code * test(p2p): TestTransportHandshake - add some sync * test: improve TestValidatorConnExecutor tests * refactor: rneame validator(update).Address to NodeAddress * typo: fix linter issue * refactor: apply code review feedback (WIP) * refactor(dash): apply code review feedback, continued * refactor(dash): improve validatorMap * state: Add QuorumHash to EventValidatorSetUpdates * feat: select validators to connect directly according to DIP-6 (WIP) * fix: fix rebase conflicts * feat: algorithm to select validators according to DIP-6 * refactor: rename select_nodes.go to select_validators.go * feat: Use DIP-6 to select Validators for inter-validator set comm * dash: add some code comments * refactor: move files to separate packages * test: fix e2e tests for single and simple networks Validators in validator set contain address only after first quorum rotation. * refactor: apply code review feedback * refactor: store validator as value (not pointer) internally * refactor: change package names in dash/ * refactor(selectpeers): rename SelectValidatorsDIP6 to DIP6 * refactor: minor code formatting fixes * refactor: Apply suggestions from code review Co-authored-by: Dmitrii Golubev <dmitrii.golubev@deliveryhero.com> * refactor: minor code improvements * refactor: minor code improvements * test: revert handshake timeout * refactor: apply peer review feedback * refactor: fix comment Co-authored-by: Dmitrii Golubev <dmitrii.golubev@deliveryhero.com> * doc(adr-d001): update ValidatorUpdate struct * fix(selectpeers): add fallback for less than 5 validators * refactor(selectpeers): Add ValidatorSelector interface * test(p2p): tune sleep time when waiting for port to get opened * 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 * feat: autodetect NodeID in ValdatorUpdate (TD-40) (#213) * feat: autodetect NodeID in ValdatorUpdate * feat(dash/quorum): handle validators without address * feat(quorum): Autodetect node ID when connecting to quorum members * test(p2p/conn): remove unused privKeyWithNilPubKey * doc(adr-d001): inter-validator set node id lookup * refactor(quorum): apply code review feedback * refactor(dash/types): rename package dashtypes to types * refactor(p2p): make ValidateID exportable * test(dash/types): increase validator address test coverage * test(e2e): no node id in validator update address * fix: node id not retrieved when processing val updates * feat(dash/quorum): lookup node id in address book * fix(dash/quorum): log validators list as single string Convert validators map to string before logging. Without this change, elasticsearch maps each validator as a separate field, like: `json.validators.1AD56AAACE6385A240B3A85EC231ACDE3972CD21A09678A7924B28116314A29C.address.Hostname` * refactor: code comments, improved strings * doc(adr-d001): update doc after implementation * doc(adr-d001): apply feedback Co-authored-by: shotonoff <shotonoff@gmail.com> * fix: don't disconnect already disconnected validators Fixed: Validator disconnected outside of Validator Set was not found in the Switch but never removed from ValidatorConnExecutor.connectedValidators. * feat(consensus): add create_proof_block_range config option (#243) Added create_proof_block_range configuration option to control the proof block creation algorithm. * fix: cycle dependency in "types" package * fix(types): modify and remove some test in types package * refactor: remove unnecessary type conversion, update comment docs * fix: light client package * refactor: improve validator-set generator * fix: some modification to ber able to run unit tests * fix: increase MaxHeaderBytes till 646 bytes * fix: TestTxFilter unit test at tx_filter_test.go * fix: TestGCFifo, TestGCRandom, reduce a size of elements to 100 * wip: backport 0.7 on top of 0.8 * fix: Fix issues in 0.7 to 0.8 backport * fix: e2e generator, remove UnmixedP2P and Legacy test cases * refactor: remove a few unused variables and functions, clean up the code * test(quorum): fix validar conn tests * refactor: modification after PR feedback * refactor: self-review of 0.7 -> 0.8 backport * fix: validator address string should contain protocol * fix: modify a light client to use default height as 0 * fix: update code style * build(deps): bump github.com/prometheus/client_golang (#249) Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.11.0 to 1.12.0. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md) - [Commits](prometheus/client_golang@v1.11.0...v1.12.0) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump github.com/BurntSushi/toml from 0.4.1 to 1.0.0 Bumps [github.com/BurntSushi/toml](https://github.com/BurntSushi/toml) from 0.4.1 to 1.0.0. - [Release notes](https://github.com/BurntSushi/toml/releases) - [Commits](BurntSushi/toml@v0.4.1...v1.0.0) --- updated-dependencies: - dependency-name: github.com/BurntSushi/toml dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(consensus): create only 1 proof block by default * chore(release): release script and initial changelog (#250) * feat(release): release script and initial change log * chore(releaase): Update version release PR description * chore(release)!: bump ABCI version and update release.sh to change TMVersionDefault automatically (#253) * chore(release): update version in version/version.go * chore(release)!: bump ABCI version and update release script * fix(node): change TestMaxProposalBlockSize test to support tenderdash values and behaviour * fix(ci): modify coverage.yml * feat(quorum): port validator set connectivity to 0.8 (WIP) * test(dash/quorum): Adjust unit tests for Tenderdash 0.8 Majority of changes are due to: * defining that we cannot use `seed == 0` in mocks, as it's used for TCP port numbers which should be 1-65535 * dialing each node individually inside ValidatorConnExecutor * test(node): update node test to new data structures * refactor(dash/quorum): rename Switch to DashConnectionManager * refactor(p2p): remove unused code introduced in 0.7 * refactor(quorum): Code cleanup and renames for consistency * refactor(dash/quorum): code comments and whitespace * chore(release): update changelog and version to 0.7.0 Co-authored-by: shotonoff <dmitrii.golubev@dash.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: shotonoff <shotonoff@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
In #187 , we require ABCI app to provide Node ID as part of validator address. But ABCI app doesn’t have it. We need to determine it inside Tenderdash if not provided.
What was done?
ValidatorAddressthat is just likep2p.NodeAddress, but doesn't require NodeIDHow Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only