Skip to content

Track failed tx proposals in consensus#3208

Merged
NotGyro merged 9 commits intomasterfrom
milliec/03-07-Track_failed_tx_proposals_in_consensus
Apr 8, 2023
Merged

Track failed tx proposals in consensus#3208
NotGyro merged 9 commits intomasterfrom
milliec/03-07-Track_failed_tx_proposals_in_consensus

Conversation

@NotGyro
Copy link
Copy Markdown
Contributor

@NotGyro NotGyro commented Mar 8, 2023

  • Adds a field, tx_proposal_failures to ClientSessionTracking
  • Implements the method fail_tx_proposal() on ClientSessionTracking
  • Invokes session.fail_tx_proposal(now) when a tx proposal is found to be invalid.

Motivation

This will be used to track how many failed tx proposals have been sent recently, so that bad connections can be dropped, to prevent attacks and harmful behavior from buggy clients. See: #2977

Future Work

  • Actually drop clients
  • Prometheus metrics related to this system.
  • Code to determine if a particular client / IP address needs to be blocked temporarily or indefinitely.

@NotGyro
Copy link
Copy Markdown
Contributor Author

NotGyro commented Mar 8, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@NotGyro NotGyro requested review from cbeck88, jcape and samdealy and removed request for samdealy March 27, 2023 23:57
@NotGyro NotGyro force-pushed the milliec/03-07-Track_failed_tx_proposals_in_consensus branch from 08fe3f6 to cf0b095 Compare March 27, 2023 23:58
@NotGyro NotGyro marked this pull request as ready for review March 28, 2023 00:20
@NotGyro NotGyro added the consensus Related only to the consensus protocol or service label Mar 28, 2023
@NotGyro NotGyro self-assigned this Mar 28, 2023
@varsha888 varsha888 requested a review from awygle March 28, 2023 17:26
Copy link
Copy Markdown
Contributor

@awygle awygle left a comment

Choose a reason for hiding this comment

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

This seems fine as far as it goes. I assume this tracking will actually be used in a follow-on PR?

Comment thread consensus/service/src/api/client_api_service.rs Outdated
Comment thread consensus/service/src/api/client_api_service.rs Outdated
Comment thread consensus/service/src/api/client_api_service.rs Outdated
Comment thread consensus/service/src/api/client_api_service.rs
@NotGyro NotGyro force-pushed the milliec/03-07-Track_failed_tx_proposals_in_consensus branch from c9ecddf to 13376da Compare April 8, 2023 02:01
@NotGyro NotGyro merged commit 11a6767 into master Apr 8, 2023
@NotGyro NotGyro deleted the milliec/03-07-Track_failed_tx_proposals_in_consensus branch April 8, 2023 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consensus Related only to the consensus protocol or service

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants