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

feat: always broadcast a BlockResponse, even if globally accepted #5454

Merged
merged 4 commits into from
Nov 19, 2024

Conversation

hstove
Copy link
Contributor

@hstove hstove commented Nov 12, 2024

Changes signer behavior to broadcast a BlockResponse, even if the signer has already marked the block as GloballyAccepted or GloballyRejected

@jcnelson
Copy link
Member

I just want to make sure there's no DoS risk here. What stops the signer from broadcasting the same BlockResponse over and over forever? Do we do this only once per proposal?

@hstove
Copy link
Contributor Author

hstove commented Nov 13, 2024

I just want to make sure there's no DoS risk here. What stops the signer from broadcasting the same BlockResponse over and over forever? Do we do this only once per proposal?

Yes, the signer only broadcasts once per proposal. This code path is also only after the signer gets a block validation response - so any DOS vector would already be spamming the node's block validation endpoint

@jcnelson
Copy link
Member

That's what I thought; just wanted to make sure.

jcnelson
jcnelson previously approved these changes Nov 13, 2024
jferrant
jferrant previously approved these changes Nov 18, 2024
@hstove hstove requested a review from jcnelson November 18, 2024 17:50
jcnelson
jcnelson previously approved these changes Nov 19, 2024
Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

LGTM; please address @obycode's comment.

@hstove
Copy link
Contributor Author

hstove commented Nov 19, 2024

I've updated the code to not re-broadcast if the block is LocallyAccepted or LocallyRejected. So, this PR is essentially changing the check from "if globally X, ignore" to "if locally X, ignore"

Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

LGTM!

@hstove hstove added this pull request to the merge queue Nov 19, 2024
Merged via the queue into develop with commit 7985b33 Nov 19, 2024
145 of 147 checks passed
@obycode obycode deleted the feat/always-send-block-response branch November 19, 2024 21:58
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust signer to send BlockResponse even after block is marked as GloballyAccepted (or rejected)
5 participants