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(code/blocksync): Randomize choice of peer to sync from #492

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

romac
Copy link
Member

@romac romac commented Oct 23, 2024

No description provided.

Copy link

codecov bot commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 90.56604% with 5 lines in your changes missing coverage. Please review.

Project coverage is 78.81%. Comparing base (856324a) to head (a52b5a3).
Report is 1 commits behind head on blocksync-mvp.

Files with missing lines Patch % Lines
code/crates/blocksync/src/handle.rs 80.00% 5 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           blocksync-mvp     #492      +/-   ##
=================================================
+ Coverage          78.70%   78.81%   +0.11%     
=================================================
  Files                124      124              
  Lines               8715     8735      +20     
=================================================
+ Hits                6859     6884      +25     
+ Misses              1856     1851       -5     
Flag Coverage Δ
integration 78.91% <90.57%> (+0.11%) ⬆️
mbt 22.02% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@romac romac merged commit 6e320ee into blocksync-mvp Oct 23, 2024
13 checks passed
@romac romac deleted the romac/blocksync-random-peer branch October 23, 2024 14:50
@romac romac mentioned this pull request Oct 23, 2024
7 tasks
romac added a commit that referenced this pull request Oct 30, 2024
* feat(code): Add dummy `BlockSync` actor

* Add `/block_sync` pubsub channel

* Rename `broadcast` to `publish`

* Use `eyre` instead of boxed `Error`

* Have peers publish their status every time they move to a new height/round

* Fix bug where `StartRound` effect was performed twice

* Move status handling into BlockSync actor

* Less noise

* Move status update logic onto BlockSync actor

* Decouple GossipConsensus actor from Consensus and BlockSync actors

* Push latest hight from consensus to blocksync

* Revert "Decouple GossipConsensus actor from Consensus and BlockSync actors"

This reverts commit 0a59ada.

* feat(code): BlockSync RPC (#457)

* Define RPC behaviour for BlockSync

* Hook BlockSync behaviour into networking layer

* Revert to encoding-agnostic networking layer

* Hook BlockSync actor to BlockSync behaviour

* Fix unwraps

* Remove round from status

* code(feat): Add block store (#458)

* Add block store for blocksync testing

* Sync helper get stored block at height from host

* Add block store pruning

* Update Cargo.lock

* Add blocksync protocol initial implementation, wip

* Add config flag for maximum number of blocks in store

* Ask for the block only from peer at lower height.

* Fix mistake in host, should return ReceivedProposedValue and not
ProposedValue.

* When processing the synced block use the on_ handlers instead of
apply_driver_input.

* When storing the decision look for full proposal at proposal round and
NOT consensus round

* Change max_retain_blocks in config.toml to match the default.

* Set voting power for all vals to 1 for easier debugging.

* Use on_vote instead of apply_driver_input so our own commit is stored
and used on decided operations.

* Fix spelling.

* Remove lower number of peers (was used for testing).

* Store the commits in a set to avoid duplicates.

* Return if proposal from non-proposer.

* Add TODO for potential min block height in Status

* Change max_retain_blocks default to 100.

* Small cleanup

* Work around libp2p bug where a node cannot publish to a topic if restarted too quickly

See libp2p/rust-libp2p#5097

* Cleanup

* Ensure validator set is always sorted properly, even after being deserialized from genesis

* Update spawn.fish

* Move BlockSyncState to blocksync crate

* Add batched request for blocks

* Fix bug where only a single block would be sent back

* Revert block batching

* Cleanup

* Remove outdated comment

* Post-merge fixes

* Post-merge fix

* Cleanup

* chore(code): Extract logic from `BlockSync` actor and into `blocksync` crate (#487)

* chore(code): Extract BlockSync logic from actor and into `blocksync` crate

* Cleanup

* Fix doc

* Fix logs

* Use custom timeout per test

* Add timeouts to BlockSync requests (#490)

* feat(code/blocksync): Add timeouts to BlockSync requests

* Fix parsing of blocksync config

* Randomize choice of peer to sync from (#492)

* Add basic integration test (#493)

* Re-enable mempool in integration test

* Add config option `blocksync.enabled`

* Send back actual block bytes instead of just the block hash

* Fix example config file

* test(code/blocksync): Introduce small DSL for expressing test scenarios (#496)

* Introduce small DSL for expressing test scenarios

* Add basic integration test for BlockSync

* Update fish script

* Allow overriding the transport used in tests via a `MALACHITE_TRANSPORT` env variable

* Update test

* Add doc comments

* Use TCP by default in tests

Can be overriden locally with `MALACHITE_TRANSPORT=quic`

* Run integration tests sequentially

* Run integration tests in release mode

* Fix coverage job

* Try again

* Do not panic when tracing subscribers are set twice

* Enable basic optimizations in debug mode

* Update MSRV to 1.82

* feat(code/blocksync): Only request sync from peers which have the requested block in their store (#495)

* feat(code/blocksync): Only request sync from peers which have the requested block in their store

For this, we extend the status with the earliest block height available
in the store, to ensure we only request a block from peers which have
told us they have it.

* Remove `blocksync::State::random_peer_at_or_above()` method

* Update clippy msrv

* Add metrics

* Ensure Consensus and BlockSync actors never fail

---------

Co-authored-by: Anca Zamfir <ancazamfir@users.noreply.github.com>
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.

1 participant