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

revert: fix ban header syn peer if no headers provided (#3297) #3306

Merged

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Sep 6, 2021

Description

This reverts commit 570e222.

Motivation and Context

#3297 (comment)

Having thought about it a bit - header sync is designed to be self contained. It does not have the information required to ban a peer that "lied" in listening mode.

Suggest that we pass through the chain metadata and peer that 'kicked' the base node into sync into header sync so
that the peer can be banned if it fails to follow through. We should also not prevent mining when in header sync until a
split and better chain have been comfirmed by header sync.

How Has This Been Tested?

Existing tests pass

@sdbondi sdbondi changed the title revert "fix: ban header sync peer if no headers provided (#3297)" revert: "fix: ban header sync peer if no headers provided (#3297)" Sep 6, 2021
@sdbondi sdbondi changed the title revert: "fix: ban header sync peer if no headers provided (#3297)" revert: fix: ban header sync peer if no headers provided (#3297) Sep 6, 2021
@sdbondi sdbondi changed the title revert: fix: ban header sync peer if no headers provided (#3297) revert: fix: ban header syn peer if no headers provided (#3297) Sep 6, 2021
@sdbondi sdbondi changed the title revert: fix: ban header syn peer if no headers provided (#3297) revert: fix ban header syn peer if no headers provided (#3297) Sep 6, 2021
@aviator-app aviator-app bot added mq-failed and removed mq-failed labels Sep 6, 2021
@aviator-app aviator-app bot merged commit bc95c18 into tari-project:development Sep 6, 2021
@sdbondi sdbondi deleted the core-header-sync-protocol-fix branch September 8, 2021 04:24
Cifko pushed a commit to Cifko/tari that referenced this pull request Sep 10, 2021
…3297) (tari-project#3306)


Description
---
This reverts commit 570e222.

Motivation and Context
---
tari-project#3297 (comment)

Having thought about it a bit - header sync is designed to be self contained. It does not have the information required to ban a peer that "lied" in listening mode. 

Suggest that we pass through the chain metadata and peer that 'kicked' the base node into sync into header sync so 
that the peer can be banned if it fails to follow through. We should also not prevent mining when in header sync until a
split and better chain have been comfirmed by header sync.

How Has This Been Tested?
---
Existing tests pass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants