`relay chain selection` and `dispute-coordinator` fixes and improvements #4752

sandreim · 2022-01-20T12:28:35Z

Fixes #4748

Work in progress.

Fixes:

Dont error in finality_target_with_longest_chain()
Track missing session info errors and propagate error to avoid sending incomplete data
Implement approach from approval-voting for processing active leaves such that even if we fail to process a block, we will eventually process it through a descendant.

The below items will be taken care of in a subsequent PR.

Import improvements(always import trusted votes):

backing

approval

own votes

votes scraped from chain

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Make sure to send errors to subsystems requesting data depending on missing session info Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/core/dispute-coordinator/src/real/initialized.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

drahnr

A few nits, one conceptual question, the general approach looks good 👍

node/core/dispute-coordinator/src/real/initialized.rs

node/subsystem-util/src/rolling_session_window.rs

node/service/src/relay_chain_selection.rs

node/core/dispute-coordinator/src/real/initialized.rs

node/core/dispute-coordinator/src/real/ordering/mod.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/core/dispute-coordinator/src/real/initialized.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/core/dispute-coordinator/src/real/initialized.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/core/dispute-coordinator/src/real/mod.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

…or-fixes

node/service/src/relay_chain_selection.rs

node/core/dispute-coordinator/src/real/ordering/mod.rs

node/core/dispute-coordinator/src/real/mod.rs

node/core/dispute-coordinator/src/real/initialized.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/core/dispute-coordinator/src/real/ordering/mod.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

drahnr · 2022-01-25T17:49:42Z

node/service/src/relay_chain_selection.rs

@@ -54,7 +54,7 @@ use std::sync::Arc;
 ///
 /// This is a safety net that should be removed at some point in the future.
 // Until it's not, make sure to also update `MAX_HEADS_LOOK_BACK` in `approval-voting`
-// when changing its value.
+// and `MAX_BATCH_SCRAPE_ANCESTORS` in `dispute-coordinator` when changing its value.


nit: We should probably add these shared constants to a separate helper crate, and import it, to avoid these manual keep in sync-todos

Agreed, do you have any suggestion on what would be the best place to move this ?

AFAIK we also want to remove this in the near future, so if we were to create a new separate crate just to hold these constants, it won't be a good investment.

Removed this, we now scrape up to and including last finalized block.

I think it's out of s ope for this PR, but a tracking issue would be nice

The reason we have though lookup limits is to avoid a very busy (and wasteful) loop on startup if the node is syncing and is very much behind the tip of the chain. Imagine if we get the first leaf, but not finality notification. The difference between the could be millions of blocks in theory. However, I'm not sure how much of a problem it's in practice.

I meant the const extraction, not the issue you described, sorry about the brevity

Looks good other than one concern: #4752 (comment).

Addressed it. We'll look back up to max(last_finalized_block_number, new_leaf -MAX_BATCH_SCRAPE_ANCESTORS);

I meant the const extraction, not the issue you described, sorry about the brevity

#4788 is here

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

ordian

Looks good other than one concern: #4752 (comment).

max finality lag Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

node/core/dispute-coordinator/src/real/mod.rs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

drahnr · 2022-01-26T13:18:15Z

node/core/dispute-coordinator/src/real/initialized.rs

+					// Limit our search to last finalized block, or up to max finality lag.
+					let block_number = std::cmp::max(
+						block_number,
+						new_leaf.number.saturating_sub(MAX_BATCH_SCRAPE_ANCESTORS),


ordian · 2022-01-26T14:06:18Z

bot merge

eskimor · 2022-01-27T10:16:34Z

The below items will be taken care of in a subsequent PR.

Please create a ticket for those.

sandreim · 2022-01-27T11:30:35Z

The below items will be taken care of in a subsequent PR.

Please create a ticket for those.

#4793

* master: Fix incomplete sorting. (#4795) Companion for better way to resolve `Phase::Emergency` via governance #10663 (#4757) Refactor and fix usage of `get_session_index()` and `get_session_info_by_index()` (#4735) `relay chain selection` and `dispute-coordinator` fixes and improvements (#4752) Fix tests (#4787) log concluded disputes (#4785) availability-distribution: look for leaf ancestors within the same session (#4596) Companion for Use proper bounded vector type for nominations #10601 (#4709) Fix release profile (#4778) [ci] remove publish-s3-release (#4779) Companion for substrate#10632 (#4689) [ci] pipeline chores (#4775) New changelog scripts (#4491)

sandreim added 4 commits January 19, 2022 20:04

Dont error in finality_target_with_longest_chain

c5289f0

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix

badb22e

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Add error flag

f64d52c

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Add error flag in dispute-coordinator

7e54dfe

Make sure to send errors to subsystems requesting data depending on missing session info Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jan 20, 2022

sandreim commented Jan 20, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/initialized.rs Outdated Show resolved Hide resolved

sandreim added B0-silent Changes should not be mentioned in any release notes C3-medium PR touches the given topic and has a medium impact on builders. labels Jan 20, 2022

sandreim added 3 commits January 20, 2022 15:56

Scrape ancestors

94ea7be

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fmt

b27a5d8

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix

b79a345

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

drahnr suggested changes Jan 20, 2022

View reviewed changes

drahnr reviewed Jan 20, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/initialized.rs Outdated Show resolved Hide resolved

drahnr reviewed Jan 20, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/initialized.rs Outdated Show resolved Hide resolved

drahnr reviewed Jan 20, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/ordering/mod.rs Outdated Show resolved Hide resolved

sandreim added 5 commits January 20, 2022 19:40

Fix naming

f8dd188

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

review feedback

ffccad7

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fmt

5c75c41

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

💬 fixes

c7997a2

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

consume

762024c

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim requested a review from eskimor January 21, 2022 13:31

drahnr reviewed Jan 24, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/initialized.rs Outdated Show resolved Hide resolved

sandreim added 2 commits January 24, 2022 11:50

fix tests

f765694

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

typo

f7276df

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim marked this pull request as ready for review January 24, 2022 11:51

github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jan 24, 2022

drahnr approved these changes Jan 24, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/initialized.rs Outdated Show resolved Hide resolved

node/core/dispute-coordinator/src/real/initialized.rs Outdated Show resolved Hide resolved

sandreim added 2 commits January 24, 2022 14:01

review fixes

b16ce9c

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Bump scraped blocks LRU capacity

6b437d8

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim added C1-low PR touches the given topic and has a low impact on builders. and removed C3-medium PR touches the given topic and has a medium impact on builders. labels Jan 24, 2022

drahnr reviewed Jan 24, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/mod.rs Outdated Show resolved Hide resolved

sandreim added 2 commits January 24, 2022 19:59

🧯 🔥

7dc4ec4

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Merge remote-tracking branch 'origin' into sandreim/dispute-coordinat…

bebe50c

…or-fixes

ordian reviewed Jan 25, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/initialized.rs Outdated Show resolved Hide resolved

sandreim added 2 commits January 25, 2022 16:17

remove prints

ed6664a

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Increase scraped blocks cache size

978d280

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

ordian reviewed Jan 25, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/ordering/mod.rs Show resolved Hide resolved

sandreim added 2 commits January 25, 2022 17:14

more review fixes

718d343

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

another fix

876d209

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

drahnr reviewed Jan 25, 2022

View reviewed changes

sandreim added 3 commits January 25, 2022 20:18

fix target_ancestor

ef544aa

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Scrape up to max finalized block

a47920f

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

undo comment change

593d635

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

ordian approved these changes Jan 26, 2022

View reviewed changes

Limit ancestry lookup to last finalized block or

1f224b8

max finality lag Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

bkchr reviewed Jan 26, 2022

View reviewed changes

node/core/dispute-coordinator/src/real/mod.rs Outdated Show resolved Hide resolved

debug damage

3ab8f60

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

drahnr reviewed Jan 26, 2022

View reviewed changes

sandreim mentioned this pull request Jan 26, 2022

Extract MAX_FINALITY_LAG constant from relay_chain_selection #4788

Closed

paritytech-processbot bot merged commit 102bc8c into master Jan 26, 2022

paritytech-processbot bot deleted the sandreim/dispute-coordinator-fixes branch January 26, 2022 14:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`relay chain selection` and `dispute-coordinator` fixes and improvements #4752

`relay chain selection` and `dispute-coordinator` fixes and improvements #4752

sandreim commented Jan 20, 2022 •

edited

Loading

drahnr left a comment •

edited

Loading

drahnr Jan 25, 2022

sandreim Jan 25, 2022

sandreim Jan 25, 2022

drahnr Jan 25, 2022

ordian Jan 26, 2022

drahnr Jan 26, 2022 •

edited

Loading

sandreim Jan 26, 2022 •

edited

Loading

sandreim Jan 26, 2022

ordian left a comment

drahnr Jan 26, 2022

ordian commented Jan 26, 2022

eskimor commented Jan 27, 2022

sandreim commented Jan 27, 2022

relay chain selection and dispute-coordinator fixes and improvements #4752

relay chain selection and dispute-coordinator fixes and improvements #4752

Conversation

sandreim commented Jan 20, 2022 • edited Loading

drahnr left a comment • edited Loading

Choose a reason for hiding this comment

drahnr Jan 25, 2022

Choose a reason for hiding this comment

sandreim Jan 25, 2022

Choose a reason for hiding this comment

sandreim Jan 25, 2022

Choose a reason for hiding this comment

drahnr Jan 25, 2022

Choose a reason for hiding this comment

ordian Jan 26, 2022

Choose a reason for hiding this comment

drahnr Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

sandreim Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

sandreim Jan 26, 2022

Choose a reason for hiding this comment

ordian left a comment

Choose a reason for hiding this comment

drahnr Jan 26, 2022

Choose a reason for hiding this comment

ordian commented Jan 26, 2022

eskimor commented Jan 27, 2022

sandreim commented Jan 27, 2022

`relay chain selection` and `dispute-coordinator` fixes and improvements #4752

`relay chain selection` and `dispute-coordinator` fixes and improvements #4752

sandreim commented Jan 20, 2022 •

edited

Loading

drahnr left a comment •

edited

Loading

drahnr Jan 26, 2022 •

edited

Loading

sandreim Jan 26, 2022 •

edited

Loading