Skip to content
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

[subsystem-bench] Trigger own assignments in approval-voting #4772

Merged
merged 4 commits into from
Jun 25, 2024

Conversation

AndreiEres
Copy link
Contributor

No description provided.

@AndreiEres AndreiEres requested a review from alexggh June 12, 2024 13:06
keystore
.sr25519_generate_new(
ASSIGNMENT_KEY_TYPE_ID,
Some(state.test_authorities.key_seeds.first().unwrap().as_str()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: state.test_authorithies.key_seeds.get(NODE_UNDER_TEST) to make it future proof.

Copy link
Contributor

@sandreim sandreim left a comment

Choose a reason for hiding this comment

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

Don't we already do it:

?

@alexggh
Copy link
Contributor

alexggh commented Jun 12, 2024

Don't we already do it:

?

No, that's for the keystore we use for generating the messages for all other nodes in the network, the change here is the keystore that we provide to approval-voting when we instantiate it.

@AndreiEres AndreiEres requested a review from sandreim June 24, 2024 12:13
@AndreiEres AndreiEres added R0-silent Changes should not be mentioned in any release notes T12-benchmarks This PR/Issue is related to benchmarking and weights. labels Jun 24, 2024
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6530065

@AndreiEres AndreiEres added this pull request to the merge queue Jun 24, 2024
@AndreiEres AndreiEres removed this pull request from the merge queue due to a manual request Jun 24, 2024
@AndreiEres AndreiEres added this pull request to the merge queue Jun 25, 2024
Merged via the queue into master with commit 909bfc2 Jun 25, 2024
154 of 156 checks passed
@AndreiEres AndreiEres deleted the AndreiEres/trigger-assignments-in-bench branch June 25, 2024 09:32
alexggh added a commit that referenced this pull request Jul 17, 2024
Since #4772, the
benchamrks triggers its own assignments, but the mocks weren't properly
hooked up, so that the approval is sent as well, so they would have
counted as no-shows and impact the time it takes for a block to be
approved.

Fixed by adding mocks for availability recovery and
candidate-validation and hook those into the benchmark.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
github-merge-queue bot pushed a commit that referenced this pull request Jul 24, 2024
Since #4772, the
benchamrks triggers its own assignments, but the mocks weren't properly
hooked up, so that the approval is sent as well, so they would have
counted as no-shows and impact the time it takes for a block to be
approved.

Fixed by adding mocks for availability recovery and candidate-validation
and hook those into the benchmark.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…h#5042)

Since paritytech#4772, the
benchamrks triggers its own assignments, but the mocks weren't properly
hooked up, so that the approval is sent as well, so they would have
counted as no-shows and impact the time it takes for a block to be
approved.

Fixed by adding mocks for availability recovery and candidate-validation
and hook those into the benchmark.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
alexggh added a commit that referenced this pull request Aug 27, 2024
The accepted divergence rate of 1/1000 is execesive and leads to false
positives especially after #4772 and
#5042, so let's increase it to 1/100
since we do have some randomness in the system and there is no point in
being that strict.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
github-merge-queue bot pushed a commit that referenced this pull request Aug 28, 2024
The accepted divergence rate of 1/1000 is excessive and leads to false
positives especially after
#4772 and
#5042, so let's increase
it to 1/100 since we do have some randomness in the system and there is
no point in being that strict.

Fixes: #5463

---------

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants