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

Update disputes prioritisation in dispute-coordinator #6130

Merged
merged 39 commits into from
Nov 11, 2022
Merged

Update disputes prioritisation in dispute-coordinator #6130

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
56731cf
Scraper processes CandidateBacked events
tdimitrov Oct 5, 2022
056a606
Change definition of best-effort
tdimitrov Oct 5, 2022
9794e99
Fix `dispute-coordinator` tests
tdimitrov Oct 7, 2022
d44055e
Unit test for dispute filtering
tdimitrov Oct 7, 2022
032475a
Clarification comment
tdimitrov Oct 8, 2022
a70d7d8
Add tests
tdimitrov Oct 8, 2022
8bafd7a
Fix logic
tdimitrov Oct 10, 2022
dd6fbc0
Add metrics for refrained participations
tdimitrov Oct 10, 2022
baa7e20
Revert "Add tests"
tdimitrov Oct 10, 2022
d7af3f7
Revert "Unit test for dispute filtering"
tdimitrov Oct 10, 2022
18f7a14
fix dispute-coordinator tests
tdimitrov Oct 10, 2022
b82df59
Fix scraping
tdimitrov Oct 11, 2022
1523bce
new tests
tdimitrov Oct 11, 2022
2c6272f
Small fixes in guide
tdimitrov Oct 13, 2022
cd42bcb
Apply suggestions from code review
tdimitrov Oct 20, 2022
0551b7c
Fix some comments and remove a pointless test
tdimitrov Oct 20, 2022
2906d2a
Code review feedback
tdimitrov Oct 31, 2022
eaa250f
Clarification comment in tests
tdimitrov Nov 2, 2022
9b40f11
Some tests
tdimitrov Nov 7, 2022
7e3040b
Reference counted `CandidateHash` in scraper
tdimitrov Nov 7, 2022
77cc49e
Proper handling for Backed and Included candidates in scraper
tdimitrov Nov 8, 2022
dcd0465
Update comments in tests
tdimitrov Nov 9, 2022
f925dd3
Guide update
tdimitrov Nov 9, 2022
9dd091a
Fix cleanup logic for `backed_candidates_by_block_number`
tdimitrov Nov 9, 2022
e533e6d
Simplify cleanup
tdimitrov Nov 9, 2022
fcb99d0
Merge branch 'master' into disputes-backed
tdimitrov Nov 9, 2022
56f4e2a
Make spellcheck happy
tdimitrov Nov 9, 2022
aecc3e0
Update tests
tdimitrov Nov 9, 2022
a97e29a
Extract candidate backing logic in separate struct
tdimitrov Nov 10, 2022
d29b300
Code review feedback
tdimitrov Nov 10, 2022
0d498dc
Treat backed and included candidates in the same fashion
tdimitrov Nov 11, 2022
40b4115
Update some comments
tdimitrov Nov 11, 2022
4c030a1
Small improvements in test
tdimitrov Nov 11, 2022
76bca96
spell check
tdimitrov Nov 11, 2022
a57ffaa
Fix some more comments
tdimitrov Nov 11, 2022
f9a4407
clean -> prune
tdimitrov Nov 11, 2022
c441720
Code review feedback
tdimitrov Nov 11, 2022
f283517
Reword comment
tdimitrov Nov 11, 2022
f7393a7
spelling
tdimitrov Nov 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix some more comments
  • Loading branch information
@tdimitrov
tdimitrov committed Nov 11, 2022
commit a57ffaa270fd3ea77d8f510aa4cfd95e566dc687
4 changes: 2 additions & 2 deletions node/core/dispute-coordinator/src/scraping/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ impl ChainScraper {

/// Prune finalized candidates.
///
/// Once a candidate lives in a relay chain block that's behind `CANDIDATE_LIFETIME_AFTER_FINALIZATION`
/// blocks after finalization we can treat it as low priority.
/// We keep each candidate for `CANDIDATE_LIFETIME_AFTER_FINALIZATION` blocks after finalization.
/// After that we treat it as low priority.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is an example of a dispute for which we've already pruned the backed/included candidates but would still want to participate? Maybe confirmed disputes? I'm just trying to sort out why we say 'low priority' rather than that the dispute is dropped.

Copy link
Member

@eskimor eskimor Nov 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, we would still queue participation for such a dispute if it suddenly got confirmed. But honestly that comment does not really add value here. This is the chain scraper, it should only be concerned about having scraped information ready, not about the priority a participation gets queued with.

pub fn process_finalized_block(&mut self, finalized_block_number: &BlockNumber) {
// `CANDIDATE_LIFETIME_AFTER_FINALIZATION - 1` because `finalized_block_number`counts to the
// candidate lifetime.
Expand Down
2 changes: 0 additions & 2 deletions node/core/dispute-coordinator/src/scraping/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,8 +422,6 @@ fn scraper_cleans_finalized_candidates() {
const TEST_TARGET_BLOCK_NUMBER: BlockNumber = 2;

// How many blocks should we skip before sending a leaf update.
// `3` because we need 1 ancestor after the finalized block (this is what the testing code expects). `ActiveLeaf` updates are generated for
// blocks 1..3 => there will be 1 ancestor after the finalzied block.
const BLOCKS_TO_SKIP: usize = 3;

futures::executor::block_on(async {
Expand Down