fix(core): fixes stale chain metadata being sent to listening state #5039
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CjS77
added
P-acks_required
sdbondi
force-pushed
the
core-remove-chain-metadata-list
branch
from
b22e2ad to
f3d2c9c
Compare
stringhandler
approved these changes
Dec 12, 2022
CjS77
removed
the
P-reviews_required
sdbondi
added a commit
to sdbondi/tari
that referenced
this pull request
Dec 12, 2022
stringhandler
pushed a commit
that referenced
this pull request
Dec 12, 2022
Description --- - adds check for peer banning in the listening state - fix build/test error in #5039 Motivation and Context --- Do one final check that the peer is not banned before processing chain metadata in the listening state (can happen due to race conditions) Fix test failure How Has This Been Tested? ---
|
Nice, I will test it later on with a stress test by unbanning all manually banned peers. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
Fixes #5037 and #5030 (needs to be confirmed since this case is not easy to reproduce)
What I've observed is that the peer is banned twice. The first time, for 30 minutes (between 04:07:23.802123900 and 2022-12-09 04:41:26.365429300) then again permanently (bad metadata sig). It's clear in the logs that the pong is received before being banned, the node is banned (chain metadata service clears the peer's metadata) but the message is already in the domain pipeline so another one is received. The chain metadata service now keeps the peer's chain metadata and sends it to the listening state every time because it is not cleared.
TL;DR classic race condition.
Order of events:
How Has This Been Tested?
Manually: rewind-blockchain and sync, enters sync mode timeously
Removed some tests that test removed functions
Added a new unit test for determine_sync_state