Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Rework consensus instance communication with the network worker #958

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Apr 1, 2020

Up to now consensus instances used the main channel to communicate with
the background network worker. This lead to a race condition when
sending a local collation and dropping the router before driving the
send local collation future until it is finished. This pr changes the
communication between worker and the instances to use their own
channels. This has the advantage that we don't need an extra
DropConsensusNetworking message as the network is dropped
automatically when the last sender is dropped.

Up to now consensus instances used the main channel to communicate with
the background network worker. This lead to a race condition when
sending a local collation and dropping the router before driving the
send local collation future until it is finished. This pr changes the
communication between worker and the instances to use their own
channels. This has the advantage that we don't need an extra
`DropConsensusNetworking` message as the network is dropped
automatically when the last sender is dropped.
@bkchr bkchr added the A0-please_review Pull request needs code review. label Apr 1, 2020
Copy link
Contributor

@expenses expenses left a comment

Choose a reason for hiding this comment

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

Can't see any problems with it here :^)

Copy link
Contributor

@montekki montekki left a comment

Choose a reason for hiding this comment

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

looks ok

@gavofyork gavofyork merged commit 639dfd6 into master Apr 1, 2020
@gavofyork gavofyork deleted the bkchr-consensus-instance-drop branch April 1, 2020 15:02
Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

A little late to the party, but looks fine

HCastano added a commit that referenced this pull request May 4, 2021
b2099c5c Bump Substrate to `b094edaf` (#958)
3f037094 Bump endowment amounts on Rialto and Millau (#957)
b21fd07c Bump Substrate WASM builder (#947)
30ccd07c Bump Substrate to `ec180313` (#955)
a7422ab1 Upgrade to GitHub-native Dependabot (#945)
ed20ef34 Move pallet-bridge-dispatch types to primitives (#948)
2070c4d6 Endow accounts and add `bridgeIds` to chainspec. (#951)
f43c9243 Fix account derivation in CLI (#952)
9ac07e73 Add backbone configuration of cargo-spellcheck (#924)
2761c3fe Message dispatch support multiple instances (#942)
801c99f3 Add Wococo<>Rococo Header Relayer (#925)
21f49051 Remove Westend<>Rococo header sync (#940)
06235f16 do not panic if pallet is not yet initialized (#937)
a13ee0bc Bump Substrate (#939)
f8680cbf jsonrpsee alpha6 (#938)
6163bcbf reonnect to failed client in on-demand relay background task (#936)
14e82bea Do not spawn additional task for on-demand relays (#933)
b1557b88 Relay at least one header for every source chain session (#923)
9420649c Remove deprecated Runtime Header APIs (#932)
9627011e Update README.md (#931)
7b736b9c Truncate output in logs. (#930)
faad06e3 Make sure that relayers have dates in logs. (#927)
07734535 Update dump-logs script. (#928)
c2d56b2e Add pruning to bechmarks & update weights. (#918)
a30c51dc Add properties to Chain Spec (#917)
d691c73e Fix issue with on-demand headers relay not starting (#921)
8ee55c1e Fix image publishing. (#922)
f51fb59d Prefix in relay loops logs (#920)

git-subtree-dir: bridges
git-subtree-split: b2099c5c0baf569e2ec7228507b6e4f3972143cc
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants