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

Conversation

@bkchr
Copy link
Member

@bkchr bkchr commented Jan 29, 2021

There was a bug that when the block announcement validation returned an
error, the slot reserved for this validation wasn't freed. This could
lead to a situation where we rejected any block announcement from such a
peer for that the block announcement returned an error multiple times.

There was a bug that when the block announcement validation returned an
error, the slot reserved for this validation wasn't freed. This could
lead to a situation where we rejected any block announcement from such a
peer for that the block announcement returned an error multiple times.
@bkchr bkchr 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 Jan 29, 2021
@bkchr bkchr requested review from andresilva, arkpar and tomaka January 29, 2021 13:51
@bkchr
Copy link
Member Author

bkchr commented Jan 29, 2021

Because of me being to dumb to use saturating_sub, this could actually lead to stop syncing from peers on chains without any block announcement data being added. Normally that probably shouldn't happen as we only receive one block announcement at a time.

Copy link
Contributor

@tomaka tomaka left a comment

Choose a reason for hiding this comment

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

🤷

@s3krit s3krit mentioned this pull request Jan 29, 2021
3 tasks
@andresilva
Copy link
Contributor

bot merge

@ghost
Copy link

ghost commented Jan 29, 2021

Waiting for commit status.

@ghost
Copy link

ghost commented Jan 29, 2021

Merge failed: "Required status check \"continuous-integration/gitlab-check-labels\" is pending."

@s3krit
Copy link
Contributor

s3krit commented Jan 29, 2021

bot merge

@ghost
Copy link

ghost commented Jan 29, 2021

Trying merge.

@ghost ghost merged commit 9659509 into master Jan 29, 2021
@ghost ghost deleted the bkchr-fix-sync-issue-9999 branch January 29, 2021 16:29
s3krit pushed a commit that referenced this pull request Jan 29, 2021
* Sync: Fix issue of not freeing a block announcement slot

There was a bug that when the block announcement validation returned an
error, the slot reserved for this validation wasn't freed. This could
lead to a situation where we rejected any block announcement from such a
peer for that the block announcement returned an error multiple times.

* Better logging

* Fuck I'm dumb

* 🤦
This pull request was closed.
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.

6 participants