Skip to content

Store peer penality records in the PeerDB in PeerInfo #7320

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

Open
wants to merge 9 commits into
base: unstable
Choose a base branch
from

Conversation

Patchoulis
Copy link

Issue Addressed

#7245

Proposed Changes

Added records regarding penalty actions to the peer info. The chosen details saved to the record seem to be the most
relevant ones from my knowledge, however I'm open to changes.

Additional Info

This is still a WIP. I'll try to test and fully validate that the changes work as expected sometime tomorrow. I'll also see if I can add some test cases as well.

@CLAassistant
Copy link

CLAassistant commented Apr 12, 2025

CLA assistant check
All committers have signed the CLA.

@Patchoulis Patchoulis marked this pull request as draft April 12, 2025 13:44
@chong-he chong-he added work-in-progress PR is a work-in-progress Networking labels Apr 14, 2025
@Patchoulis
Copy link
Author

I'm planning on writing the tests soon, but I was wondering if there's anything else you all believe I should add? Perhaps some other info that might be useful to store?

@chong-he
Copy link
Member

I'm planning on writing the tests soon, but I was wondering if there's anything else you all believe I should add? Perhaps some other info that might be useful to store?

replied on Discord, thanks!

@Patchoulis Patchoulis marked this pull request as ready for review April 14, 2025 07:00
@chong-he chong-he added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Apr 14, 2025
@Patchoulis Patchoulis changed the base branch from stable to unstable April 14, 2025 07:46
@Patchoulis Patchoulis requested a review from jxs as a code owner April 14, 2025 07:46
@Patchoulis
Copy link
Author

Apparently I just found out that it should have been into unstable instead of stable, fixed that.

@Patchoulis Patchoulis requested a review from dapplion April 26, 2025 08:34
@@ -752,6 +762,32 @@ impl<E: EthSpec> PeerDB<E> {
peer_id
}

fn add_penalty_record(&mut self, peer_id: &PeerId, penalty_record: PenaltyRecord) {
let info = self.peers.entry(*peer_id).or_insert_with(|| {
error!(%peer_id, "Adding a penalty record to a non-existant peer");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can happen in a race condition, not sure if it should be an error log

/// The result of the penalty
pub result: ScoreTransitionResult,
/// The time when the penalty occured in unix millis
pub time_stamp: u128,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pub time_stamp: u128,
pub timestamp_ms: u128,

@jimmygchen jimmygchen added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels May 12, 2025
Copy link

mergify bot commented May 15, 2025

All required checks have passed and there are no merge conflicts. This pull request may now be ready for another review.

@mergify mergify bot added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels May 15, 2025
@dapplion
Copy link
Collaborator

All required checks have passed and there are no merge conflicts. This pull request may now be ready for another review.

Not really, we are waiting for author input

@jimmygchen
Copy link
Member

jimmygchen commented May 22, 2025

Not really, we are waiting for author input

@dapplion lol yep, mergify automation rules have been updated now, thanks

@jimmygchen jimmygchen added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Networking waiting-on-author The reviewer has suggested changes and awaits thier implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants