-
Notifications
You must be signed in to change notification settings - Fork 707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix 0007-dispute-freshly-finalized.zndsl failing #3893
Conversation
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
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.
@ordian do you have any concerns with this? If so we could turn off coalescing for this test. IIRC we had a good reason to test for a value of 0 here, or maybe I am wrong.
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.
Seems reasonable. I'd probably add some sleep before this check to ensure finality is not lagging after some time.
|
||
# Check lag - dispute conclusion | ||
honest: reports polkadot_parachain_disputes_finality_lag is 0 | ||
honest: reports polkadot_parachain_disputes_finality_lag is lower than 2 |
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.
was this metric also lagging?
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.
Yes, it was let me figure it why.
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.
Yeah, that's expected with the way we compute polkadot_parachain_disputes_finality_lag
, because DetermineUndisputedChain
block_descriptions: subchain_block_descriptions, |
approval-voting
:let lag_disputes = initial_leaf_number.saturating_sub(subchain_number); |
Test started failing after 66051ad which enabled approval coalescing, that was expected to happen because the test required an polkadot_parachain_approval_checking_finality_lag of 0, which can't happen with max_approval_coalesce_count greater than 1 because we always delay the approval for no_show_duration_ticks/2 in case we can coalesce it with other approvals. So relax a bit the restrictions, since we don't actually care that the lags are 0, but the fact the finalities are progressing and are not stuck. Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Test started failing after 66051ad which enabled approval coalescing, that was expected to happen because the test required an polkadot_parachain_approval_checking_finality_lag of 0, which can't happen with max_approval_coalesce_count greater than 1 because we always delay the approval for no_show_duration_ticks/2 in case we can coalesce it with other approvals. So relax a bit the restrictions, since we don't actually care that the lags are 0, but the fact the finalities are progressing and are not stuck. Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Test started failing after paritytech@66051ad which enabled approval coalescing, that was expected to happen because the test required an polkadot_parachain_approval_checking_finality_lag of 0, which can't happen with max_approval_coalesce_count greater than 1 because we always delay the approval for no_show_duration_ticks/2 in case we can coalesce it with other approvals. So relax a bit the restrictions, since we don't actually care that the lags are 0, but the fact the finalities are progressing and are not stuck. Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Test started failing after 66051ad which enabled approval coalescing, that was expected to happen because the test required an polkadot_parachain_approval_checking_finality_lag of 0, which can't happen with max_approval_coalesce_count greater than 1 because we always delay the approval for no_show_duration_ticks/2 in case we can coalesce it with other approvals.
So relax a bit the restrictions, since we don't actually care that the lags are 0, but the fact the finalities are progressing and are not stuck.