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.

Sign up for GitHub

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

Jump to bottom

[node] Add "connections.block_proposal_max_age_secs" and ignore block proposals more than the configured amount #5552

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

[node] Add "connections.block_proposal_max_age_secs" and ignore block proposals more than the configured amount #5552

wants to merge 10 commits into from
+74 −7

Conversation

jferrant
Copy link
Collaborator

Closes #5439

@jferrant jferrant marked this pull request as ready for review December 13, 2024 18:54
@jferrant jferrant requested review from a team as code owners December 13, 2024 18:54
@aldur aldur added this to the 3.1.0.0.3 milestone Dec 16, 2024
obycode
obycode previously approved these changes Dec 16, 2024
View reviewed changes
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! Just one minor comment.

stackslib/src/net/api/postblock_proposal.rs Outdated Show resolved Hide resolved
jcnelson
jcnelson previously approved these changes Dec 16, 2024
View reviewed changes
@jferrant jferrant dismissed stale reviews from jcnelson and obycode via 86dc10f December 16, 2024 20:16
@jferrant jferrant requested review from obycode and jcnelson December 16, 2024 20:16
obycode
obycode previously approved these changes Dec 16, 2024
View reviewed changes
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.

👍

hstove
hstove requested changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@hstove hstove left a comment

Choose a reason for hiding this comment

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

I think you forgot to add block_proposal_max_age_secs to ConnectionOptionsFile (with related code to pass into ConnectionOptions).

(This is such an easy thing to forget, it'd be great if we had more of a serde_toml approach or something to make this cleaner and less footgunable)

@jferrant
Copy link
Collaborator Author

block_proposal_max_age_secs

Ugh I totally did. Good catch. Thank you!

@jferrant jferrant requested a review from a team as a code owner December 16, 2024 20:37
@jferrant jferrant requested review from hstove and obycode December 16, 2024 20:37
@obycode
Copy link
Contributor

obycode commented Dec 16, 2024

I think you forgot to add block_proposal_max_age_secs to ConnectionOptionsFile (with related code to pass into ConnectionOptions).

(This is such an easy thing to forget, it'd be great if we had more of a serde_toml approach or something to make this cleaner and less footgunable)

💯 Totally agree!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@obycode obycode obycode approved these changes

@hstove hstove hstove approved these changes

@jcnelson jcnelson jcnelson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Stacks Core Eng
Status: Status: 💻 In Progress
Milestone
3.1.0.0.3
Development

Successfully merging this pull request may close these issues.
5 participants
@jferrant @obycode @jcnelson @hstove @aldur