* We actually don't need to rate limit redundant requests.
Those redundant requests should not actually happen, but still.
* Add some logging.
* Also log message when the receiving side hit the rate limit.
* Update node/network/dispute-distribution/src/sender/mod.rs
Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
Co-authored-by: eskimor <eskimor@no-such-url.com>
Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
* Change best effort queue behaviour in `dispute-coordinator`
Use the same type of queue (`BTreeMap<CandidateComparator,
ParticipationRequest>`) for best effort and priority in
`dispute-coordinator`.
Rework `CandidateComparator` to handle unavailable parent
block numbers.
Best effort queue will order disputes the same way as priority does - by
parent's block height. Disputes on candidates for which the parent's
block number can't be obtained will be treated with the lowest priority.
* Fix tests: Handle `ChainApiMessage::BlockNumber` in `handle_sync_queries`
* Some tests are deadlocking on sending messages via overseer so change `SingleItemSink`to `mpsc::Sender` with a buffer of 1
* Fix a race in test after adding a buffered queue for overseer messages
* Fix the rest of the tests
* Guide update - best-effort queue
* Guide update: clarification about spam votes
* Fix tests in `availability-distribution`
* Update comments
* Add `make_buffered_subsystem_context` in `subsystem-test-helpers`
* Code review feedback
* Code review feedback
* Code review feedback
* Don't add best effort candidate if it is already in priority queue
* Remove an old comment
* Fix insert in best_effort
* Fix typos
* Filter unconfirmed disputes in provisioner - random_selection
* Rework dispute coordinator to return `DisputeStatus` with
`ActiveDisputes` message.
* Rework the random_selection implementation of `select_disptues` in
`provisioner` to return only confirmed disputes.
* Filter unconfirmed disputes in provisioner - prioritized_selection
* Add test for unconfirmed disputes handling
* Fix `dispute-distribution` tests
Was seeing these warnings when running `cargo check --all`:
```
warning: for loop over an `Option`. This is more readably written as an `if let` statement
--> node/core/approval-voting/src/lib.rs:1147:21
|
1147 | for activated in update.activated {
| ^^^^^^^^^^^^^^^^
|
= note: `#[warn(for_loops_over_fallibles)]` on by default
help: to check pattern in a loop use `while let`
|
1147 | while let Some(activated) = update.activated {
| ~~~~~~~~~~~~~~~ ~~~
help: consider using `if let` to clear intent
|
1147 | if let Some(activated) = update.activated {
| ~~~~~~~~~~~~ ~~~
```
My guess is that `activated` used to be a SmallVec or similar, as is
`deactivated`. It was changed to an `Option`, the `for` still compiled (it's
technically correct, just weird), and the compiler didn't catch it until now.
* Add `DisputeState` to `DisputeCoordinatorMessage::RecentDisputes`
The new signature of the message is:
```
RecentDisputes(oneshot::Sender<Vec<(SessionIndex, CandidateHash, DisputeStatus)>>),
```
As part of the change also add `DispiteStatus` to
`polkadot_node_primitives`.
* Move dummy_signature() in primitives/test-helpers
* Enable staging runtime api on Rococo
* Implementation
* Move disputes to separate module
* Vote prioritisation
* Duplicates handling
* Double vote handling
* Unit tests
* Logs and metrics
* Code review feedback
* Fix ACTIVE/INACTIVE separation and update partition names
* Add `fn dispute_is_inactive` to node primitives and refactor `fn get_active_with_status()` logic
* Keep the 'old' logic if the staging api is not enabled
* Fix some comments in tests
* Add warning message if there are any inactive_unknown_onchain disputes
* Add file headers and remove `use super::*;` usage outside tests
* Adding doc comments
* Fix test methods names
* Fix staging api usage
* Fix `get_disputes` runtime function implementation
* Fix compilation error
* Fix arithmetic operations in tests
* Use smaller test data
* Rename `RuntimeApiRequest::StagingDisputes` to `RuntimeApiRequest::Disputes`
* Remove `staging-client` feature flag
* fmt
* Remove `vstaging` feature flag
* Some comments regarding the staging api
* Rename dispute selection modules in provisioner
with_staging_api -> prioritized_selection
without_staging_api -> random_selection
* Comments for staging api
* Comments
* Additional logging
* Code review feedback
process_selected_disputes -> into_multi_dispute_statement_set
typo
In trait VoteType: vote_value -> is_valid
* Code review feedback
* Fix metrics
* get_disputes -> disputes
* Get time only once during partitioning
* Fix partitioning
* Comments
* Reduce the number of hardcoded api versions
* Code review feedback
* Unused import
* Comments
* More precise log messages
* Code review feedback
* Code review feedback
* Code review feedback - remove `trait VoteType`
* Code review feedback
* Trace log for DisputeCoordinatorMessage::QueryCandidateVotes counter in vote_selection
* Use new ProtocolName in peer_set.rs
* Use new ProtocolName for request-response protocols
* Use new ProtocolName in polkadot-network-bridge
* Import and conversion fixes
* Use ProtocolName re-exported in sc_network
* update lockfile for {"substrate"}
Co-authored-by: parity-processbot <>
* Bump crate versions
* Bump spec_version to 9280 for kusama
* Bump spec_version to 9280 for polkadot
* Bump spec_version to 9280 for rococo
* Bump spec_version to 9280 for westend
* update Cargo.lock
Co-authored-by: parity-processbot <>
* Don't import backing statements directly
into the dispute coordinator. This also gets rid of a redundant
signature check. Both should have some impact on backing performance.
In general this PR should make us scale better in the number of parachains.
Reasoning (aka why this is fine):
For the signature check: As mentioned, it is a redundant check. The
signature has already been checked at this point. This is even made
obvious by the used types. The smart constructor is not perfect as
discussed [here](https://github.com/paritytech/polkadot/issues/3455),
but is still a reasonable security.
For not importing to the dispute-coordinator: This should be good as the
dispute coordinator does scrape backing votes from chain. This suffices
in practice as a super majority of validators must have seen a backing
fork in order for a candidate to get included and only included
candidates pose a threat to our system. The import from chain is
preferable over direct import of backing votes for two reasons:
1. The import is batched, greatly improving import performance. All
backing votes for a candidate are imported with a single import.
And indeed we were able to see in metrics that importing votes
from chain is fast.
2. We do less work in general as not every candidate for which
statements are gossiped might actually make it on a chain. The
dispute coordinator as with the current implementation would still
import and keep those votes around for six sessions.
While redundancy is good for reliability in the event of bugs, this also
comes at a non negligible cost. The dispute-coordinator right now is the
subsystem with the highest load, despite the fact that it should not be
doing much during mormal operation and it is only getting worse
with more parachains as the load is a direct function of the number of statements.
We'll see on Versi how much of a performance improvement this PR
* Get rid of dead code.
* Dont send approval vote
* Make it pass CI
* Bring back tests for fixing them later.
* Explicit signature check.
* Resurrect approval-voting tests (not fixed yet)
* Send out approval votes in dispute-distribution.
Use BTreeMap for ordered dispute votes.
* Bring back an important warning.
* Fix approval voting tests.
* Don't send out dispute message on import + test
+ Some cleanup.
* Guide changes.
Note that the introduced complexity is actually redundant.
* WIP: guide changes.
* Finish guide changes about dispute-coordinator
conceputally. Requires more proof read still.
Also removed obsolete implementation details, where the code is better
suited as the source of truth.
* Finish guide changes for now.
* Remove own approval vote import logic.
* Implement logic for retrieving approval-votes
into approval-voting and approval-distribution subsystems.
* Update roadmap/implementers-guide/src/node/disputes/dispute-coordinator.md
Co-authored-by: asynchronous rob <rphmeier@gmail.com>
* Review feedback.
In particular: Add note about disputes of non included candidates.
* Incorporate Review Remarks
* Get rid of superfluous space.
* Tidy up import logic a bit.
Logical vote import is now separated, making the code more readable and
maintainable.
Also: Accept import if there is at least one invalid signer that has not
exceeded its spam slots, instead of requiring all of them to not exceed
their limits. This is more correct and a preparation for vote batching.
* We don't need/have empty imports.
* Fix tests and bugs.
* Remove error prone redundancy.
* Import approval votes on dispute initiated/concluded.
* Add test for approval vote import.
* Make guide checker happy (hopefully)
* Another sanity check + better logs.
* Reasoning about boundedness.
* Use `CandidateIndex` as opposed to `CoreIndex`.
* Remove redundant import.
* Review remarks.
* Add metric for calls to request signatures
* More review remarks.
* Add metric on imported approval votes.
* Include candidate hash in logs.
* More trace log
* Break cycle.
* Add some tracing.
* Cleanup allowed messages.
* fmt
* Tracing + timeout for get inherent data.
* Better error.
* Break cycle in all places.
* Clarified comment some more.
* Typo.
* Break cycle approval-distribution - approval-voting.
Co-authored-by: asynchronous rob <rphmeier@gmail.com>
* foo
* rolling session window
* fixup
* remove use statemetn
* fmt
* split NetworkBridge into two subsystems
Pending cleanup
* split
* chore: reexport OrchestraError as OverseerError
* chore: silence warnings
* fixup tests
* chore: add default timenout of 30s to subsystem test helper ctx handle
* single item channel
* fixins
* fmt
* cleanup
* remove dead code
* remove sync bounds again
* wire up shared state
* deal with some FIXMEs
* use distinct tags
Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com>
* use tag
Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com>
* address naming
tx and rx are common in networking and also have an implicit meaning regarding networking
compared to incoming and outgoing which are already used with subsystems themselvesq
* remove unused sync oracle
* remove unneeded state
* fix tests
* chore: fmt
* do not try to register twice
* leak Metrics type
Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com>
Co-authored-by: Andronik <write@reusable.software>
* Move NewGossipTopology -> SessionGridTopology outside as this implementation is shared
* Add method to return peers difference between topologies
* Implement basic grid topology usage for the bitfield distribution
* Fix tests
* Oops, fix tests
* Add some tests for random routing
* Add a unit test for topology distribution
* Store the current and the previous topology to match sessions boundaries
* Update tests
* Update node/network/bitfield-distribution/src/lib.rs
Co-authored-by: Andronik <write@reusable.software>
* Update node/network/protocol/src/grid_topology.rs
Co-authored-by: Andronik <write@reusable.software>
* Update node/network/bitfield-distribution/src/lib.rs
Co-authored-by: Andronik <write@reusable.software>
* Add some debug
* Fix tests as HashSet order is undefined
* Move session bounded topology to the common code part
* Fix tests
* Allow to select routing by peer index
* Implement grid topology in the statement distribution subsystem
* Fix tests compilation
* Fix test
* Refactor API slightly
* Address review comments
* Reduce runtime error logging severity
* Update node/network/protocol/src/grid_topology.rs
Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>
* Update node/network/bitfield-distribution/src/tests.rs
Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>
* Fmt run
* Use named struct
* Fix logging stuff
* One more accidental fmt damage
* Increase active queue size and add metrics
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
* Revert "Increase active queue size and add metrics"
This reverts commit c4f48e8bded6dfeb9c62814ba2f8d815c34b04cf.
* Use validator index to choose the routing strategy
Noted by: @rphmeier
* Fix test after distribution logic fix
Co-authored-by: Andronik <write@reusable.software>
Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>
Co-authored-by: Andrei Sandu <andrei-mihail@parity.io>
* Double grandpa gossip duration.
* Make resend period slightly larger.
So it won't get triggered by additional grandpa delay.
* Bump other values as well.
* Don't change gossip duration on Polkadot.
(and Westend as it is meant to be a testbed for Polkadot)
Mattia L.V. Bradascio