This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Split block announce processing into two parts #6958
Merged
Merged
Changes from 1 commit
Commits
Show all changes
126 commits
Select commit
Hold shift + click to select a range
b336104
Split block announce processing into two parts
bkchr 95c1420
Apply suggestions from code review
bkchr 0416a0b
Update client/network/src/protocol/sync.rs
bkchr f974fe7
client/authority-discovery: Append PeerId to Multiaddr at most once (…
mxinden 7afdc67
expose Deposit (#6943)
xlc 67d811b
Add a `LightSyncState` field to the chain spec (#6894)
expenses e714641
Dynamically generate CHT roots on a full client (#6944)
expenses 508dfcd
Enable verification logic when executing benchmarks (#6929)
shawntabrizi 43cd1d0
grandpa: always create and send justification if there are any subscr…
andresilva 0de5451
.maintain/monitoring/alerting-rules: Add fd alert (#6946)
mxinden 20d4ca4
Fix benchmark read/write key tracker for keys in child storages. (#6905)
gui1117 4b3197c
client/authority-discovery: Limit number of addresses per authority (…
mxinden 9f590df
⛓ ✨Add ShiftNrg Network SS58 address type (#6942)
mswezey23 761ec7c
update tracing attribute (#6950)
gui1117 f7791d0
Fix unwraps and other issues with benchmarks (#6957)
shawntabrizi 127673f
Remove implementation of `Randomness for ()` (#6959)
bkchr 144192c
Fix staking fuzzer. (#6954)
kianenigma 1f33b6e
Enforce that ProtocolId is a string (#6953)
tomaka ebd6d5c
Support Staking Payout to Any Account (#6832)
shawntabrizi ce89cc9
Better prime election. (#6939)
kianenigma 117bb45
babe: fix report_equivocation weight (#6936)
andresilva 4c0a477
fix bench db wipe (#6965)
gui1117 3bddf6c
Implement request-responses protocols (#6634)
tomaka e4178cc
add generated weight info for pallet-collective (#6789)
apopiak 8f28655
client/*: Treat protocol name as str and not [u8] (#6967)
mxinden 6b2f7ea
update kvdb-rocksdb to 0.9.1 and rocksdb to 6.11.4 (#6963)
ordian 7d53c94
Use AsyncReadExt::read_exact, not just read (#6977)
tomaka 539f4d2
client/cli/src/config: Warn on low file descriptor limit (#6956)
mxinden 8182870
Update substrate bip39 version. (#6955)
cheme b622015
Inverting events set and changed in nicks pallet (#6989)
g2udevelopment dbbaef1
Silence the error about non-registered protocols (#6987)
tomaka 7972673
Change browser-demo build.sh to use python 3 again (#6992)
expenses 9231166
fix pallet-evm features (#6995)
xlc 9007d28
Move subcommands from sc-cli to nodes (#6948)
expenses 8502300
ci: deploy alerting rules: fix run on changes (#6998)
gabreal f723458
*: Update to Prometheus v0.10.0 (#6964)
mxinden 0f302b9
Ensure that handshake is sent back even in case of back-pressure (#6979)
tomaka 3541fa8
frame/authority-discovery: Have authorities() return both current and…
mxinden f8189fc
Stop sending messages on legacy substream altogether (#6975)
tomaka 658c389
manual seal is now consensus agnostic (#7010)
seunlanlege b309241
grandpa: report metrics on prevotes and precommits cast (#6970)
andresilva 130b7d4
Fix compact npos solution edge count calculation (#7021)
kianenigma 29c993a
Refactor & detach network metrics. (#6986)
romanb 4dee242
Node template complete import pipeline (#7014)
JoshOrndorff 68bdfec
client/authority-discovery: Throttle DHT requests (#7018)
mxinden 8c92258
Update Nicks docs to clarify that it is not production-ready (#6990)
danforbes 6deea30
Ignore wasm_gc for debug build. (#6962)
cheme 8f4db26
Make `--file` optional for `generate-node-key` (#7043)
bkchr 2bc6943
Downgrade wabt = 0.9.1 (#7042)
gnunicorn 52ae515
Add metadata shadows to multisig pallet (#7029)
27aaf0a
Fix broken link to democracy pallet. (#7026)
49191fd
Revert "Fix broken link to democracy pallet. (#7026)" (#7047)
gavofyork 79cc67d
Update the service tasks Grafana dashboard (#7038)
tomaka a22edf0
babe, grandpa: waive fees on valid equivocation report (#6981)
andresilva b4fbdda
Clarify Nicks docs (#7049)
danforbes 67c778b
Improves EVM gas price check (#7051)
crystalin 39d8f4d
Change wabt to wat (#7050)
pepyakin 5fa219d
Add Dock network id for address generation (#6714)
lovesh 0a850ae
Partial fix for transaction priority (#7034)
kianenigma 29b42c3
What happens if we remove wat? (#7056)
pepyakin 384f29f
Make SlashingSpans Public (#6961)
kianenigma d123792
client/authority-discovery/src/service: Improve docs (#7059)
mxinden d35044c
Decrease poll interval (#7063)
s3krit de59f04
Remove unused code (#7027)
f96646a
Disambiguate `BlockNumber` type in `decl_module` (#7061)
shawntabrizi 89e6d66
Implement `FromStr` for `Ss58AddressFormat` (#7068)
bkchr bd44423
Set reserved nodes with offchain worker. (#6996)
kaichaosun 0f61352
Rename `TRIGGER_WASM_BUILD` to `FORCE_WASM_BUILD` (#7080)
bkchr 0fd0d95
Make decoding of `compact<perthing>` saturating instead of invalid (#…
gui1117 006f3f0
state_machine no_std witness externalities (#6934)
cheme cc3040d
Support hex encoded secret key for `--node-key` (#7052)
bkchr 6db5fe3
Add a `build-sync-spec` subcommand and remove the CHT roots from the …
expenses bb7052d
Fix build sync spec (#7086)
expenses a9a3be3
Fail docs on warnings (#5923)
TriplEight 4b471dd
Fix `storage::read` (#7084)
bkchr 6c89d07
Add fuzzer for the compact custom codec implementation from PR #6720 …
viniul 8b82790
add instantiable support for treasury pallet (#7058)
pfcoder a5977f5
grandpa-rpc don't share subscription manager, only executor (#7039)
octol caab538
pallet-collective: allow customized default vote (#6984)
sorpaas 5f6987a
Upgrade to libp2p-0.28. (#7077)
romanb c071d06
pow: support uniform tie breaking in fork choice (#7073)
sorpaas 72ea91f
Allow remotes to not open a legacy substream (#7075)
tomaka f5e0c63
Use diener for Polkadot companion prs (#7102)
bkchr b2b0db5
Update ui tests for rust 1.46.0 (#7106)
bkchr 9d9cf8a
client/network: Expose number of entries per Kademlia bucket (#7104)
mxinden c2ef92d
Improve error output of wasm-builder when wasm ins't installed (#7105)
bkchr b805faf
Add ss58 address for Dark network (#6982)
c867bc2
Frame-support storage: make iterations and translate consistent (#5470)
gui1117 111a110
fix js dependancy alert, bumping bl version (#7110)
HarryHong f406f49
Make `transactional` attribute less scope dependent (#7112)
bkchr d2c7c1f
Add SS58 Registry (#7020)
joepetrowski 0ddcd66
Move Staking Weights to T::WeightInfo (#7007)
kianenigma 4e4c321
Send import notification always for re-orgs (#7118)
bkchr 9167fe0
WeightInfo for Vesting Pallet (#7103)
shawntabrizi 59b4668
Add benchmarking pipeline to node-template (#7122)
shawntabrizi 2c65036
Use tracing-based subscriber logging (#6825)
sorpaas 86026fc
Typo in error text (#7126)
Swader da02c0b
WeightInfo for ImOnline (#7128)
shawntabrizi f1380be
fix the new staking weight in substrate-node (#7131)
kianenigma 585629a
Remove warning about deprecated PeerIds (#7132)
tomaka 9bf2f2f
Fix db initialization for light client (#7130)
arkpar fa2fbe7
WeightInfo for Identity Pallet (#7107)
shawntabrizi fbf617e
Make sure we update the `Cargo.lock` in the polkadot companion (#7135)
bkchr 5a2c400
Tracing for wasm with bridging to native (#6916)
gnunicorn b7a3b2b
Pallet Indices (#7137)
shawntabrizi f8ee0e1
pow: replace the thread-base mining loop with a future-based mining w…
sorpaas ecaf240
Bounties (#5715)
xlc 6b1d600
Update SS58 configuration for Bifrost (#7142)
Dengjianping 7cc397f
Prometheus metrics for RPC calls (#7088)
grbIzl 98a6a8c
WeightInfo for Scheduler (#7138)
shawntabrizi f75c540
grandpa-rpc: use FinalityProofProvider to check finality for rpc (#6215)
octol 3a0cb2a
Make it compile
bkchr a837031
Rework the implementation and decrease the peer reputation on invalid
bkchr a0f3789
Implement limits for block announce validation
bkchr 20c00ea
Merge remote-tracking branch 'origin/master' into bkchr-async-sync
bkchr 3f977ab
Remove accidentally added file
bkchr e352e41
Merge remote-tracking branch 'origin/master' into bkchr-async-sync
bkchr f7f0873
Apply suggestions from code review
bkchr 4f252be
Rename `BlockAnnounceResult` to `PollBlockAnnounceValidation`
bkchr e08cbcf
Always return result using the internal future
bkchr 41aa638
Merge remote-tracking branch 'origin/master' into bkchr-async-sync
bkchr 2b18234
Move the polling and make sure all validation futures are registered
bkchr 047f80c
Ignore the future in the legacy stuff
bkchr 9cbfc19
Merge remote-tracking branch 'origin/master' into bkchr-async-sync
bkchr 8ea6c88
Remove leftover stuff
bkchr 8e5a00a
Update client/network/src/protocol/sync.rs
bkchr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -572,10 +572,11 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { | |
self.context_data.peers.iter().map(|(id, peer)| (id, &peer.info)) | ||
} | ||
|
||
pub fn on_custom_message( | ||
fn on_custom_message( | ||
&mut self, | ||
who: PeerId, | ||
data: BytesMut, | ||
cx: &mut std::task::Context, | ||
) -> CustomMessageOutcome<B> { | ||
let message = match <Message<B> as Decode>::decode(&mut &data[..]) { | ||
Ok(message) => message, | ||
|
@@ -600,7 +601,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { | |
GenericMessage::Status(_) => | ||
debug!(target: "sub-libp2p", "Received unexpected Status"), | ||
GenericMessage::BlockAnnounce(announce) => | ||
self.push_block_announce_validation(who.clone(), announce), | ||
return self.push_block_announce_validation(who.clone(), announce, cx), | ||
GenericMessage::Transactions(m) => | ||
self.on_transactions(who, m), | ||
GenericMessage::BlockResponse(_) => | ||
|
@@ -1168,11 +1169,18 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { | |
/// called later to check for finished validations. The result of the validation | ||
/// needs to be passed to [`Protocol::process_block_announce_validation_result`] | ||
/// to finish the processing. | ||
/// | ||
/// This function ensures that the block announce validation future is polled | ||
/// at least once so that the task is woken up when the future is ready. | ||
/// | ||
/// Returns the message outcome from polling and processing a potential finished | ||
/// block announce validation. | ||
fn push_block_announce_validation( | ||
&mut self, | ||
who: PeerId, | ||
announce: BlockAnnounce<B::Header>, | ||
) { | ||
cx: &mut std::task::Context, | ||
) -> CustomMessageOutcome<B> { | ||
let hash = announce.header.hash(); | ||
|
||
if let Some(ref mut peer) = self.context_data.peers.get_mut(&who) { | ||
|
@@ -1185,6 +1193,14 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> { | |
}; | ||
|
||
self.sync.push_block_announce_validation(who, hash, announce, is_best); | ||
|
||
// Make sure that the newly added block announce validation future was | ||
// polled once to be registered in the task. | ||
if let Poll::Ready(res) = self.sync.poll_block_announce_validation(cx) { | ||
self.process_block_announce_validation_result(res) | ||
} else { | ||
CustomMessageOutcome::None | ||
} | ||
} | ||
|
||
/// Process the result of the block announce validation. | ||
|
@@ -1581,6 +1597,15 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviour for Protocol<B, H> { | |
warn!(target: "sub-libp2p", "Inconsistent state, no peers for pending transaction!"); | ||
} | ||
} | ||
|
||
// Check if there is any block announcement validation finished. | ||
while let Poll::Ready(result) = self.sync.poll_block_announce_validation(cx) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
match self.process_block_announce_validation_result(result) { | ||
CustomMessageOutcome::None => {}, | ||
outcome => self.pending_messages.push_back(outcome), | ||
} | ||
} | ||
|
||
if let Some(message) = self.pending_messages.pop_front() { | ||
return Poll::Ready(NetworkBehaviourAction::GenerateEvent(message)); | ||
} | ||
|
@@ -1656,7 +1681,7 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviour for Protocol<B, H> { | |
self.on_peer_disconnected(peer_id) | ||
}, | ||
GenericProtoOut::LegacyMessage { peer_id, message } => | ||
self.on_custom_message(peer_id, message), | ||
self.on_custom_message(peer_id, message, cx), | ||
GenericProtoOut::Notification { peer_id, protocol_name, message } => | ||
match self.legacy_equiv_by_name.get(&protocol_name) { | ||
Some(Fallback::Consensus(engine_id)) => { | ||
|
@@ -1675,12 +1700,11 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviour for Protocol<B, H> { | |
} | ||
Some(Fallback::BlockAnnounce) => { | ||
if let Ok(announce) = message::BlockAnnounce::decode(&mut message.as_ref()) { | ||
self.push_block_announce_validation(peer_id, announce); | ||
self.push_block_announce_validation(peer_id, announce, cx) | ||
} else { | ||
warn!(target: "sub-libp2p", "Failed to decode block announce"); | ||
CustomMessageOutcome::None | ||
} | ||
|
||
CustomMessageOutcome::None | ||
} | ||
None => { | ||
debug!(target: "sub-libp2p", "Received notification from unknown protocol {:?}", protocol_name); | ||
|
@@ -1690,14 +1714,6 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviour for Protocol<B, H> { | |
}; | ||
|
||
if let CustomMessageOutcome::None = outcome { | ||
// Check if there is any block announcement validation finished. | ||
while let Poll::Ready(result) = self.sync.poll_block_announce_validation(cx) { | ||
match self.process_block_announce_validation_result(result) { | ||
CustomMessageOutcome::None => continue, | ||
outcome => return Poll::Ready(NetworkBehaviourAction::GenerateEvent(outcome)) | ||
} | ||
} | ||
|
||
Poll::Pending | ||
} else { | ||
Poll::Ready(NetworkBehaviourAction::GenerateEvent(outcome)) | ||
|
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing a context to an otherwise synchronous function gives the impression that this function is similar to a
poll
function that is repeatedly called. Buton_custom_message
is only called ... well on custom messages. I would much rather keep polling logic likepoll_block_announce_validation
within functions that adhere to the above convention.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that doesn't work, I would need to call
poll_block_announce_validation
as well inon_custom_message
. So, either I revert my latest commit and do it as it was done before, or we keep this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to poll after
on_custom_message
or we don't register the future in the task.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I am suggesting in #6958 (comment) is to ignore the fact that the future is not registered with the task for block announcements from the legacy substream for the following two reasons:
Protocol:poll
is polled regularly and includes apoll_block_announce_validation(cx)
at the very beginning.This concerns block announcements send via the legacy substream only which (a) is not used much and (b) is going away soon.
As far as I can see this is a reasonable trade-off. What do you think @bkchr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done