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

[Consensus and Collection] Refactors guarantee dissemination #1406

Merged
merged 8 commits into from Oct 2, 2021

Conversation

yhassanzadeh13
Copy link
Contributor

@yhassanzadeh13 yhassanzadeh13 commented Oct 1, 2021

@codecov-commenter
Copy link

codecov-commenter commented Oct 1, 2021

Codecov Report

Merging #1406 (cc02029) into master (5b3a98a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1406   +/-   ##
=======================================
  Coverage   55.38%   55.38%           
=======================================
  Files         512      512           
  Lines       31972    31964    -8     
=======================================
- Hits        17708    17704    -4     
+ Misses      11880    11877    -3     
+ Partials     2384     2383    -1     
Flag Coverage Δ
unittests 55.38% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
engine/consensus/ingestion/engine.go 45.63% <ø> (+0.12%) ⬆️
engine/collection/pusher/engine.go 65.38% <100.00%> (-0.66%) ⬇️
admin/server.go 50.00% <0.00%> (-12.50%) ⬇️
admin/command_runner.go 78.51% <0.00%> (-0.75%) ⬇️
engine/collection/synchronization/engine.go 62.90% <0.00%> (ø)
...ngine/common/synchronization/finalized_snapshot.go 72.91% <0.00%> (+4.16%) ⬆️
module/mempool/epochs/transactions.go 100.00% <0.00%> (+5.26%) ⬆️

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 5b3a98a...cc02029. Read the comment docs.

@@ -144,7 +138,7 @@ func (e *Engine) SubmitCollectionGuarantee(guarantee *flow.CollectionGuarantee)
// network usage significantly by implementing a simple retry mechanism here and
// only sending to a single consensus node.
// => https://github.com/dapperlabs/flow-go/issues/4358
Copy link
Member

Choose a reason for hiding this comment

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

better to update this comment to avoid confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yhassanzadeh13 yhassanzadeh13 marked this pull request as ready for review October 1, 2021 23:19
Copy link
Member

@jordanschalm jordanschalm left a comment

Choose a reason for hiding this comment

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

🎸

@zhangchiqing zhangchiqing merged commit 3e27bfb into master Oct 2, 2021
@zhangchiqing zhangchiqing deleted the yahya/5899-fix-guarantee-dissemination branch October 2, 2021 04:25
zhangchiqing added a commit that referenced this pull request Oct 5, 2021
* fvm bench test

* bench test upgrade

* add create account and ft transfer bench

* Add a comparative bench

* adb more benchmarks

* review fixes

* increase benchmarking string length

* Addition of AWS S3 uploader

* Add S3 bucket option to execution CLI

* Use single programs instance

* fix lint issues and revert unnecesary changes

* refactor - part1

* refactor - part 2

* let batchNFT benchmark accept block executor

* Enable transaction fees on mainnet

* adjust CLI flags

* go mod tidy

* simplify admin server implementation

* Extracted rootBlock command from finalize command for bootstrap util. Added encoding and writing to disk encoded root block and votes

* Implemented QC creation as part of finalizing cmd. Updated docs. Implementing reading/writing of DKG and root block

* Fixed typo, updated tests

* Updated QC building logic. Fixed signing of QC

* Updated godoc

* Updated comments. Reverted last commit

* Updated how root block is created. Separated root block and votes

* update QCVoter and DKG Broker to handle access node fallbacks

- update CLI for LN/SN nodes to remove access-address, and secure-access-node-api flags

- add --access-node-ids flag

- update unit tests

- add fallback logic to retry logic of Broadcast and Vote

* Added encoding of full DKG to construct QC in finilize step

* Updated reading of votes, dkg data in finalizer. Simplified logic for generating QC

* implement pull root block and push root block vote

* fux kubg

* Updated tests

* lint fixes

* Fixed tests

* Linted. Fixed tests

* Update cmd/bootstrap/run/qc.go

* renamed methods for reading node infos to reflect that fact that we are _reading_ the infos and not generating them

* consolidated consistency checks

* cleanup

* added error check

* format

* update localnet/integration tests to generate votes separately

* put votes in same directory

* fix path join

* address comments

* fix cmd namings

* rename root block vote

* pass in identities separate from participant data

* add log

* Extracted rootBlock command from finalize command for bootstrap util. Added encoding and writing to disk encoded root block and votes

* unwrap encodable type of key for partners

* Implemented QC creation as part of finalizing cmd. Updated docs. Implementing reading/writing of DKG and root block

* Fixed typo, updated tests

* Updated QC building logic. Fixed signing of QC

* Updated godoc

* Updated comments. Reverted last commit

* Updated how root block is created. Separated root block and votes

* Added encoding of full DKG to construct QC in finilize step

* Updated reading of votes, dkg data in finalizer. Simplified logic for generating QC

* Updated tests

* Fixed tests

* Linted. Fixed tests

* Update cmd/bootstrap/run/qc.go

* renamed methods for reading node infos to reflect that fact that we are _reading_ the infos and not generating them

* consolidated consistency checks

* cleanup

* format

* update localnet/integration tests to generate votes separately

* put votes in same directory

* fix path join

* change file names

* Update finalize_test.go

* fix filename

* fixing makefile and other fixes (#1378)

* fixing makefile and other fixes

* removing special nocgo docker target for the transit script

* fix test (lint)

* Update README.md

* LInted and fixed unit tests

* Fixed network test

* Updated godoc

* fixing the rootblock.json download path

* fixing log message

* restrict fees to mainnet testnet and canary

* split sign and upload into two steps

* update localnet

- update local net flags for LN/SN nodes

- update default access node count to 2

-  put flow client config prep in common function

* Update peerManager.go

* fix bug

* Update peerManager.go

* update integration tests

- update integration tests flags and container configs (from #1323)

* add missing error return when node fails to prepare flow client conn opts

* Update flow_client.go

* update dkg tests controller factory client arg

* update comments and log statement formatting

* Update flow_client.go

* add fallback to poll/submitResult

* add log

* fix bug creating flow client opts

we always used the public key from the first AN, making any fallback
attempts useless (wrong key)

* [Consensus] fix startup time (#1402)

* fix startup time

* remove log from debugging

* maintain ordering of ANs

* update var names

* update flag help string, improve logging,

- use trim prefix

* [crypto] Move ADX detection to crypto Makefile

Fixes #1229

* [crypto] De-quarantine BLST tests

Their flakiness was fixed by #1227

* Update broker.go

* [Consensus and Collection] Refactors guarantee dissemination  (#1406)

* refactors pusher engine multicast

* refactors ingestion engine publish mechanism on consensus node side

* fixes lint

* fixes a comment

* [Consensus] Revert unsealed reason (#1332)

* revert unsealed reason

* fix linter

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

* Update command_runner_test.go

* fix typo

Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com>
Co-authored-by: Martin Gallagher <martin@martingallagher.com>
Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>
Co-authored-by: ramtinms <ramtin@axiomzen.co>
Co-authored-by: Ramtin M. Seraj <ramtinms@users.noreply.github.com>
Co-authored-by: François Garillot <francois.garillot@dapperlabs.com>
Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>
Co-authored-by: Yurii Oleksyshyn <yuraolex@gmail.com>
Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
Co-authored-by: Vishal <1117327+vishalchangrani@users.noreply.github.com>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Co-authored-by: Yahya Hassanzadeh <yhassanzadeh13@ku.edu.tr>
zhangchiqing added a commit that referenced this pull request Oct 18, 2021
* rename root block vote

* pass in identities separate from participant data

* add log

* Extracted rootBlock command from finalize command for bootstrap util. Added encoding and writing to disk encoded root block and votes

* unwrap encodable type of key for partners

* Implemented QC creation as part of finalizing cmd. Updated docs. Implementing reading/writing of DKG and root block

* Fixed typo, updated tests

* Updated QC building logic. Fixed signing of QC

* Updated godoc

* Updated comments. Reverted last commit

* Updated how root block is created. Separated root block and votes

* Added encoding of full DKG to construct QC in finilize step

* Updated reading of votes, dkg data in finalizer. Simplified logic for generating QC

* Updated tests

* Fixed tests

* Linted. Fixed tests

* Update cmd/bootstrap/run/qc.go

* renamed methods for reading node infos to reflect that fact that we are _reading_ the infos and not generating them

* consolidated consistency checks

* cleanup

* format

* update localnet/integration tests to generate votes separately

* put votes in same directory

* fix path join

* change file names

* Update finalize_test.go

* fix filename

* fixing makefile and other fixes (#1378)

* fixing makefile and other fixes

* removing special nocgo docker target for the transit script

* fix test (lint)

* Update README.md

* address comments

* LInted and fixed unit tests

* Fixed network test

* Updated godoc

* add todo to enforce encryption

* [sync] Add (very) basic rapid test for sync engine

When a block B at height h is requested through RequestBlock(B.ID()) and then received through HandleBlock(B):
- an interleaving request for a different block B' at height h' == h may have happened,
- if it is handled (through HandleBlock(B')) before the HandleBlock(B),
- then it will populate c.heights[h']
- and return a status of Received on the getRequestStatus(h, B.ID()) in HandleBlock(B)
- as a consequence HandleBlock(B) will return false

* [sync] Fix height accounting from forks

Makes getRequestStatus(height, ID) return a height status only if there's no superseding ID status
and the height status isn't a mismatch for the query ID.

* [sync] add height tracking logic to rapid tests

Model height request tracking

* lint

* Clarify godoc
Add extra check for error type in a test

* split max delay and sampled delay

add test case for default delay base values

* fixing the rootblock.json download path

* fixing log message

* lint

* Update module/dkg/controller.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* restrict fees to mainnet testnet and canary

* Update core.go

* split sign and upload into two steps

* update localnet

- update local net flags for LN/SN nodes

- update default access node count to 2

-  put flow client config prep in common function

* add comments

* added comments and added AddComponent method to component manager

* Networking and scaffold changes

* fix build errors

* fix build errors

* remove unnecessary contexts.

* fix tests

* generate mocks

* fix more tests

* Update peerManager.go

* fix bug

* Update peerManager.go

* fix tests

* more test fixes

* Update network.go

* remove mocks

* fix more tests

* update integration tests

- update integration tests flags and container configs (from #1323)

* fix tesets

* add missing error return when node fails to prepare flow client conn opts

* Update flow_client.go

* update dkg tests controller factory client arg

* modify component implementations

* fix build error

* fix lint

* address comments

* go mod tidy

* update comments and log statement formatting

* Update flow_client.go

* add fallback to poll/submitResult

* add log

* fix bug creating flow client opts

we always used the public key from the first AN, making any fallback
attempts useless (wrong key)

* [Consensus] fix startup time (#1402)

* fix startup time

* remove log from debugging

* maintain ordering of ANs

* update var names

* update flag help string, improve logging,

- use trim prefix

* [crypto] Move ADX detection to crypto Makefile

Fixes #1229

* [crypto] De-quarantine BLST tests

Their flakiness was fixed by #1227

* add blockID logging
rename reference block for clarity

* Update broker.go

* [Consensus and Collection] Refactors guarantee dissemination  (#1406)

* refactors pusher engine multicast

* refactors ingestion engine publish mechanism on consensus node side

* fixes lint

* fixes a comment

* Add insanely long rapid test

* added comments

* update test

* better channel closed checks

* better error checking

* fix subtle race condition

* reduce channel close latency allowance

* [Consensus] Revert unsealed reason (#1332)

* revert unsealed reason

* fix linter

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

* Update component-interface.md

* update indentation

* docs: fix dead links

* Add details about RunComponent

* Add comments and fix indentation

* add period

* fix syntax error

* add '*' default value for --access-node-ids flag for LN/SN nodes

* fix lint

* Update command_runner_test.go

* Update integration/testnet/client.go

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

* add sentinel for disabled dkg client

* ensure emulator link is enabled at end of test

* fix typo

* fix cadence type assertion

the test assume a particular ordering of Cadence maps, which no longer
can be guaranteed by the implementation

* update var names, remove unused funcs, refactor

* Merge branch 'khalil/1595-access-node-ids-default' of github.com:onflow/flow-go into khalil/1583-stake-node-util-func

* grammar, add sanity check if 0 node ids returned for default, remove obsolete struct field ContainerConfig.Unstaked

* Update cmd/util/cmd/common/validation.go

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

* fix linter

* update signaler implementation and runcomponent

* Update irrecoverable API to match semantics of Context API

* Update component manager

* Update startable

* update network and middleware

* fix command runner and scaffold

* fixing subtle race condition

* Update run_component_test.go

* Delete failures

* fix component tests

* fix test failures

* Update irrecoverable_example_test.go

* fix lint

* Implement log level admin command

* Update command_runner_test.go

* fix test bug

* update client funcs, use normal flow account, add TestEpochJoin placeholder test

- add CreateAccount and ExecuteScriptBytes funcs to client

- remove locked tokens from staking flow, use normal flow account

- add placeholder tests for epoch join

* test(sync-engine): mode HandleHeight in rapid tests of sync engine

FIxes dapperlabs/flow-go#5894

* test(sync engine): Improvements in heigth modeling for the sync engine tests

This now models the canceling of height requests by other requests.

* [test-only, sync-engine] Simplify request tracking

* Update component.go

* Update component-interface.md

* use get node info script and compare node id info

* address comments

* Move command into new file

* re-point links from content/ to the rendered page

* add comment describing test

* Update integration/tests/epochs/suite.go

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

* Update epoch_test.go

* fix lint

* tidy

* run_component_test minor updates to tests and  documentation

* remove unused func

* remove redundant key generation code

* remove error return, minor updates

* Update component-interface.md

* indentation

* add hotstuff view to ping route

* address comments

* remove redundant redirects

* fix lint

* applies fix (#1416)

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* fix tests

* fix engine unit

Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
Co-authored-by: Yurii Oleksyshyn <yuraolex@gmail.com>
Co-authored-by: Vishal <1117327+vishalchangrani@users.noreply.github.com>
Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>
Co-authored-by: François Garillot <francois.garillot@dapperlabs.com>
Co-authored-by: François Garillot <4142+huitseeker@users.noreply.github.com>
Co-authored-by: Maks Pawlak <120831+m4ksio@users.noreply.github.com>
Co-authored-by: Kay-Zee <kan@axiomzen.co>
Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com>
Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>
Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>
Co-authored-by: ramtinms <ramtin@axiomzen.co>
Co-authored-by: Ramtin M. Seraj <ramtinms@users.noreply.github.com>
Co-authored-by: Martin Gallagher <martin@martingallagher.com>
Co-authored-by: Yahya Hassanzadeh <yhassanzadeh13@ku.edu.tr>
Co-authored-by: Mangirdas <mangirdas@judeikis.lt>
Co-authored-by: gomisha <misha@gomisha.com>
SupunS pushed a commit that referenced this pull request Oct 28, 2021
* fvm bench test

* bench test upgrade

* add create account and ft transfer bench

* Add a comparative bench

* adb more benchmarks

* review fixes

* increase benchmarking string length

* Addition of AWS S3 uploader

* Add S3 bucket option to execution CLI

* Use single programs instance

* fix lint issues and revert unnecesary changes

* refactor - part1

* refactor - part 2

* let batchNFT benchmark accept block executor

* Enable transaction fees on mainnet

* adjust CLI flags

* go mod tidy

* simplify admin server implementation

* Extracted rootBlock command from finalize command for bootstrap util. Added encoding and writing to disk encoded root block and votes

* Implemented QC creation as part of finalizing cmd. Updated docs. Implementing reading/writing of DKG and root block

* Fixed typo, updated tests

* Updated QC building logic. Fixed signing of QC

* Updated godoc

* Updated comments. Reverted last commit

* Updated how root block is created. Separated root block and votes

* update QCVoter and DKG Broker to handle access node fallbacks

- update CLI for LN/SN nodes to remove access-address, and secure-access-node-api flags

- add --access-node-ids flag

- update unit tests

- add fallback logic to retry logic of Broadcast and Vote

* Added encoding of full DKG to construct QC in finilize step

* Updated reading of votes, dkg data in finalizer. Simplified logic for generating QC

* implement pull root block and push root block vote

* fux kubg

* Updated tests

* lint fixes

* Fixed tests

* Linted. Fixed tests

* Update cmd/bootstrap/run/qc.go

* renamed methods for reading node infos to reflect that fact that we are _reading_ the infos and not generating them

* consolidated consistency checks

* cleanup

* added error check

* format

* update localnet/integration tests to generate votes separately

* put votes in same directory

* fix path join

* address comments

* fix cmd namings

* rename root block vote

* pass in identities separate from participant data

* add log

* Extracted rootBlock command from finalize command for bootstrap util. Added encoding and writing to disk encoded root block and votes

* unwrap encodable type of key for partners

* Implemented QC creation as part of finalizing cmd. Updated docs. Implementing reading/writing of DKG and root block

* Fixed typo, updated tests

* Updated QC building logic. Fixed signing of QC

* Updated godoc

* Updated comments. Reverted last commit

* Updated how root block is created. Separated root block and votes

* Added encoding of full DKG to construct QC in finilize step

* Updated reading of votes, dkg data in finalizer. Simplified logic for generating QC

* Updated tests

* Fixed tests

* Linted. Fixed tests

* Update cmd/bootstrap/run/qc.go

* renamed methods for reading node infos to reflect that fact that we are _reading_ the infos and not generating them

* consolidated consistency checks

* cleanup

* format

* update localnet/integration tests to generate votes separately

* put votes in same directory

* fix path join

* change file names

* Update finalize_test.go

* fix filename

* fixing makefile and other fixes (#1378)

* fixing makefile and other fixes

* removing special nocgo docker target for the transit script

* fix test (lint)

* Update README.md

* LInted and fixed unit tests

* Fixed network test

* Updated godoc

* fixing the rootblock.json download path

* fixing log message

* restrict fees to mainnet testnet and canary

* split sign and upload into two steps

* update localnet

- update local net flags for LN/SN nodes

- update default access node count to 2

-  put flow client config prep in common function

* Update peerManager.go

* fix bug

* Update peerManager.go

* update integration tests

- update integration tests flags and container configs (from #1323)

* add missing error return when node fails to prepare flow client conn opts

* Update flow_client.go

* update dkg tests controller factory client arg

* update comments and log statement formatting

* Update flow_client.go

* add fallback to poll/submitResult

* add log

* fix bug creating flow client opts

we always used the public key from the first AN, making any fallback
attempts useless (wrong key)

* [Consensus] fix startup time (#1402)

* fix startup time

* remove log from debugging

* maintain ordering of ANs

* update var names

* update flag help string, improve logging,

- use trim prefix

* [crypto] Move ADX detection to crypto Makefile

Fixes #1229

* [crypto] De-quarantine BLST tests

Their flakiness was fixed by #1227

* Update broker.go

* [Consensus and Collection] Refactors guarantee dissemination  (#1406)

* refactors pusher engine multicast

* refactors ingestion engine publish mechanism on consensus node side

* fixes lint

* fixes a comment

* [Consensus] Revert unsealed reason (#1332)

* revert unsealed reason

* fix linter

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

* Update command_runner_test.go

* fix typo

Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com>
Co-authored-by: Martin Gallagher <martin@martingallagher.com>
Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>
Co-authored-by: ramtinms <ramtin@axiomzen.co>
Co-authored-by: Ramtin M. Seraj <ramtinms@users.noreply.github.com>
Co-authored-by: François Garillot <francois.garillot@dapperlabs.com>
Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>
Co-authored-by: Yurii Oleksyshyn <yuraolex@gmail.com>
Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
Co-authored-by: Vishal <1117327+vishalchangrani@users.noreply.github.com>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Co-authored-by: Yahya Hassanzadeh <yhassanzadeh13@ku.edu.tr>
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.

None yet

4 participants