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

Remove "to_block" field from BlockRequests #12447

Merged
merged 2 commits into from
Oct 8, 2022

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Oct 7, 2022

On the receiving side, this field is completely ignored when we receive a request.
On the sender side, we always set this field to None, with the sole exception of warp syncing where we set it to the same value as the starting block because we want one specific block.

As far as I know, not only this field isn't used right now but has never been used.

Since this field is useless, this PR removes it.

Because we're using protobuf, it's not a problem if a client on an old version sends us a warp sync request where the field is defined. The now-unrecognized field will simply be ignored.

If this is approved, a corresponding issue should be opened in the w3f/specs repo.

@tomaka tomaka added 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. labels Oct 7, 2022
@tomaka tomaka requested a review from arkpar October 7, 2022 15:23
@tomaka
Copy link
Contributor Author

tomaka commented Oct 8, 2022

bot merge

@paritytech-processbot
Copy link

Error: Statuses failed for 8c6ae67

@shawntabrizi
Copy link
Member

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 460fff9 into paritytech:master Oct 8, 2022
@tomaka tomaka deleted the rm-to-block-field branch October 9, 2022 07:07
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Remove "to_block" field from BlockRequests

* Maybe fix the tests
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants