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

Process/keep messages/connections from PoSe-banned MNs #2967

Merged
merged 3 commits into from
Jun 13, 2019

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Jun 5, 2019

No description provided.

@UdjinM6 UdjinM6 added this to the 14.1 milestone Jun 5, 2019
@UdjinM6 UdjinM6 changed the title Process/keep messages from PoSe-banned MNs Process/keep messages/connections from PoSe-banned MNs Jun 5, 2019
@codablock
Copy link

Can you further explain why you think we need c08498e and 083dee7? As far as I understand only the first commit is needed

@UdjinM6
Copy link
Author

UdjinM6 commented Jun 11, 2019

c08498e - we must recognize/relay/mine dstx-es, otherwise some mixing txes might stuck if a MN was pose-banned recently. We have rate limits for dstx-es in place, so this is safe.

083dee7 - few reasons:

  1. GetValidMNByService had a misleading name :)
  2. Dropping connections to PoSe-banned (quorum) nodes reduces our own intra-quorum connectivity which can result in our isolation if all our quorum neighbours are pose-banned. They still could be up and running and relaying intra-quorum messages, so keeping connections could help quorums stay healthy a bit longer I think. As long as the corresponding collateral is not spent, it's fine to identify the node as a masternode on a network level imo.

@codablock
Copy link

Hmm ok, agree on the need of both commits then. However, the last commit will result in a lot of log spam in case a PoSe banned MN is actually down. We should add some backoff code for the MN connections so that we don't retry MNs too often which failed to connect before.

Copy link

@codablock codablock left a comment

Choose a reason for hiding this comment

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

utACK, assuming that the log spam will be so annoying that we eventually implement the backoff code I mentioned.

@UdjinM6 UdjinM6 merged commit 4739dad into dashpay:develop Jun 13, 2019
codablock pushed a commit to codablock/dash that referenced this pull request Jun 21, 2019
* Process/keep/count votes from PoSe-banned MNs

* Process dstx from PoSe-banned MNs

* Recognize PoSe-banned MNs as MNs
@UdjinM6 UdjinM6 deleted the fixinvalidmns branch November 26, 2020 13:26
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