Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(consensus): add create_proof_block_range config option #243

Merged
merged 4 commits into from
Jan 7, 2022

Conversation

lklimek
Copy link
Collaborator

@lklimek lklimek commented Jan 5, 2022

Issue being fixed or feature implemented

Added create_proof_block_range configuration option to control the proof block creation algorithm.

What was done?

  1. Added create_proof_block_range with default value of 2.
  2. Updated needProofBlock() to reflect new configuration option.

How Has This Been Tested?

With unit tests:

go test -test.v -test.run TestMempoolNoProgressUntilTxsAvailable ./consensus/ | grep needProofBlock

Breaking Changes

none

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@lklimek lklimek added this to the 0.7.0 milestone Jan 5, 2022
@lklimek lklimek self-assigned this Jan 5, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #243 (ca7332c) into v0.7-dev (5ebbd20) will increase coverage by 0.02%.
The diff coverage is 66.08%.

Impacted file tree graph

@@             Coverage Diff              @@
##           v0.7-dev     #243      +/-   ##
============================================
+ Coverage     45.20%   45.22%   +0.02%     
============================================
  Files           280      280              
  Lines         52278    52313      +35     
============================================
+ Hits          23632    23660      +28     
- Misses        26727    26733       +6     
- Partials       1919     1920       +1     
Impacted Files Coverage Δ
abci/types/types.pb.go 3.08% <0.00%> (ø)
config/toml.go 73.33% <ø> (ø)
statesync/syncer.go 75.70% <ø> (+1.26%) ⬆️
test/e2e/generator/generate.go 0.00% <0.00%> (ø)
config/config.go 72.01% <25.00%> (-0.40%) ⬇️
consensus/state.go 68.16% <90.00%> (+0.22%) ⬆️
consensus/reactor.go 74.59% <100.00%> (-0.93%) ⬇️
consensus/replay.go 69.34% <100.00%> (ø)
node/node.go 55.71% <100.00%> (+0.11%) ⬆️
p2p/conn/connection.go 79.88% <100.00%> (+0.18%) ⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b87850...ca7332c. Read the comment docs.

@lklimek lklimek requested a review from shotonoff January 5, 2022 15:37
@lklimek lklimek changed the title feat(consensus): add create_proof_block_range conf option feat(consensus): add create_proof_block_range config option Jan 5, 2022
Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Copy link
Collaborator

@shotonoff shotonoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is only one minor suggestion, if it is possible then it would be nice to initialize the components that are necessary for a test before a loop

for proofBlockRange := int64(1); proofBlockRange <= 3; proofBlockRange++ {
t.Logf("Checking proof block range %d", proofBlockRange)

config := ResetConfig("consensus_mempool_txs_available_test")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we move the creation of the config out of the loop?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not trivial, and each test run should be independent. If I move this initialization code, it will create the configs, data directory etc. and each subsequent run will reuse it.

consensus/mempool_test.go Outdated Show resolved Hide resolved
@lklimek lklimek merged commit 8acb1d0 into v0.7-dev Jan 7, 2022
@lklimek lklimek deleted the TD-50-two-blocks-setting branch January 7, 2022 16:29
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>
shotonoff pushed a commit that referenced this pull request May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants