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

sc-network-test::Peer: block push methods return hashes vec #12944

Merged
merged 4 commits into from
Dec 19, 2022

Conversation

michalkucharczyk
Copy link
Contributor

@michalkucharczyk michalkucharczyk commented Dec 15, 2022

This commit reworks the block generation/push methods in sc-network-test::Peer.

Now methods are providing the vector of hashes that were built.

This allows to get rid of redundant block_hash_from_id call, as all hashes are known just after being built.

Similar approach was taken in BeefyTestNet::generate_blocks_and_sync method.

This PR is part of BlockId::Number refactoring analysis (paritytech/polkadot-sdk#53)

This commit reworks the block generation/push methods in
sc-network-test::Peer.

Now methods are providing the vector of hashes that were built.

This allows to get rid of redundant `block_hash_from_id` call, as all
hashes are known just after being built.

Similar approach was taken in BeefyTestNet::generate_blocks_and_sync
method.

This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292)
@michalkucharczyk michalkucharczyk added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Dec 15, 2022
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Dec 15, 2022
// Sanity checks.
context
.client
.state_at(hash)
.state_at(self.block.header.hash())
Copy link
Contributor Author

@michalkucharczyk michalkucharczyk Dec 15, 2022

Choose a reason for hiding this comment

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

note: this is not related to other changes, just removed excessive call to expect_block_hash_from_id.

@@ -285,13 +284,13 @@ fn sync_justifications() {
runtime.block_on(futures::future::poll_fn::<(), _>(|cx| {
net.poll(cx);

for hash in [hashof10, hashof15, hashof20] {
if net.peer(0).client().justifications(hash).unwrap() !=
for height in (10..21).step_by(5) {
Copy link
Contributor Author

@michalkucharczyk michalkucharczyk Dec 15, 2022

Choose a reason for hiding this comment

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

This was just reverted to original form (as it was before BlockId-refactor:

for height in (10..21).step_by(5) {
if net.peer(0).client().justifications(&BlockId::Number(height)).unwrap() !=
Some(Justifications::from((*b"FRNK", Vec::new())))
{
return Poll::Pending
}
if net.peer(1).client().justifications(&BlockId::Number(height)).unwrap() !=
Some(Justifications::from((*b"FRNK", Vec::new())))
{
return Poll::Pending
}
}

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Looks good!

client/beefy/src/tests.rs Outdated Show resolved Hide resolved
client/beefy/src/tests.rs Outdated Show resolved Hide resolved
@michalkucharczyk
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 1f0865c into master Dec 19, 2022
@paritytech-processbot paritytech-processbot bot deleted the mku-push-blocks-returns-hashes branch December 19, 2022 10:18
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
…ch#12944)

* sc-network-test::Peer: block push methods return hashes vec

This commit reworks the block generation/push methods in
sc-network-test::Peer.

Now methods are providing the vector of hashes that were built.

This allows to get rid of redundant `block_hash_from_id` call, as all
hashes are known just after being built.

Similar approach was taken in BeefyTestNet::generate_blocks_and_sync
method.

This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292)

* fix

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

Co-authored-by: Bastian Köcher <git@kchr.de>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
…ch#12944)

* sc-network-test::Peer: block push methods return hashes vec

This commit reworks the block generation/push methods in
sc-network-test::Peer.

Now methods are providing the vector of hashes that were built.

This allows to get rid of redundant `block_hash_from_id` call, as all
hashes are known just after being built.

Similar approach was taken in BeefyTestNet::generate_blocks_and_sync
method.

This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292)

* fix

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

Co-authored-by: Bastian Köcher <git@kchr.de>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

4 participants