feat(op-node): Peer Score Hardening #4743

refcell · 2023-01-19T22:16:52Z

Description

Closes CLI-3524
Closes CLI-3245
Closes CLI-3249
Closes CLI-3247

Changelog

Adds peer scoring to the op-node.
Peer scoring metrics.
Peer scoring CLI Flags

changeset-bot · 2023-01-19T22:16:55Z

⚠️ No Changeset found

Latest commit: 831f9f6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

op-node/p2p/gossip.go

tynes · 2023-01-19T23:19:27Z

Not opposed to the comments being broken out into their own PR

refcell · 2023-01-20T01:32:47Z

Not opposed to the comments being broken out into their own PR

Will do going forward - keeping for now as i'm still learning.

op-node/p2p/node.go

trianglesphere

Really nice doc additions in the heartbeat & sources packages, but could you pull them out of this PR into a separate PR. We try to keep PRs as a single unit rather than pulling in a bunch of disparate changes.

op-node/p2p/gossip.go

op-node/metrics/metrics.go

op-node/p2p/gossip.go

op-node/p2p/peer_params.go

op-node/p2p/peer_scorer.go

Co-authored-by: protolambda <proto@protolambda.com>

op-node/p2p/peer_scorer.go

tynes · 2023-01-23T18:57:10Z

Going to defer most review here to @protolambda and @trianglesphere as I'm not super familiar with the internals of libp2p and have other things that I need to prioritize

Good job with the documented code @refcell

mergify · 2023-01-23T19:01:14Z

Hey @refcell! This PR has merge conflicts. Please fix them before continuing review.

op-node/p2p/peer_gater.go

codecov · 2023-03-10T00:38:28Z

Codecov Report

Merging #4743 (831f9f6) into develop (962ac1d) will decrease coverage by 4.30%.
The diff coverage is 26.40%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4743      +/-   ##
===========================================
- Coverage    40.51%   36.22%   -4.30%     
===========================================
  Files          288      207      -81     
  Lines        20119    17224    -2895     
  Branches       602        0     -602     
===========================================
- Hits          8152     6240    -1912     
+ Misses       11346    10355     -991     
- Partials       621      629       +8

Flag	Coverage Δ
bedrock-go-tests	`36.22% <26.40%> (-0.28%)`	⬇️
contracts-tests	`?`
core-utils-tests	`?`
dtl-tests	`?`
fault-detector-tests	`?`
sdk-tests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
op-node/flags/p2p_flags.go	`100.00% <ø> (ø)`
op-node/metrics/metrics.go	`1.33% <0.00%> (-0.05%)`	⬇️
op-node/p2p/cli/load_config.go	`0.00% <0.00%> (ø)`
op-node/p2p/mocks/Peerstore.go	`0.00% <0.00%> (ø)`
op-node/p2p/prepared.go	`0.00% <0.00%> (ø)`
op-node/p2p/topic_params.go	`0.00% <0.00%> (ø)`
op-node/service.go	`0.00% <0.00%> (ø)`
op-node/sources/batching.go	`85.56% <ø> (+2.91%)`	⬆️
op-node/p2p/mocks/ConnectionGater.go	`5.16% <5.16%> (ø)`
op-node/p2p/rpc_server.go	`71.42% <8.33%> (ø)`
... and 9 more

... and 91 files with indirect coverage changes

ajsutton

I don't know the implementation details of go libp2p enough to review a lot of this, but left a few comments for the parts I do understand. :)

op-node/metrics/metrics.go

op-node/p2p/peer_params.go

op-e2e/system_test.go

protolambda

+1 on the invalid-messages testing note by Adrian. We can do so in a separate follow-up PR though. This one is consistent and complete to merge, and already quite large. If you do mark as resolved, do open an issue for the ban testing so we can track it

op-e2e/system_test.go

mergify · 2023-03-11T01:58:46Z

This PR has been added to the merge queue, and will be merged soon.

mergify · 2023-03-11T01:58:48Z

This PR is next in line to be merged, and will be merged as soon as checks pass.

trianglesphere · 2023-03-13T18:41:10Z

op-node/sources/batching.go

@@ -115,7 +115,6 @@ func (ibc *IterativeBatchCall[K, V]) Fetch(ctx context.Context) error {
 			}
 			return ctx.Err()
 		default:
-			break


why was this removed? I think it's a no-op, but it seems really unrelated

feat: a foray into opnode p2p - peer score metrics

f22dac9

refcell self-assigned this Jan 19, 2023

tynes reviewed Jan 19, 2023

View reviewed changes

op-node/p2p/gossip.go Outdated Show resolved Hide resolved

feat: peer scoring config

117fd62

fix: metrics histogram

4e19f01

refcell force-pushed the refcell/p2p_cli branch from b333e23 to 4e19f01 Compare January 20, 2023 20:58

refcell changed the title ~~feat: a foray into opnode p2p - peer score metrics~~ feat: op-node Peer Score Metrics Jan 21, 2023

feat: initial peer and topic scoring configs

f080fa3

semgrep-app bot reviewed Jan 23, 2023

View reviewed changes

op-node/p2p/node.go Outdated Show resolved Hide resolved

semgrep-app bot reviewed Jan 23, 2023

View reviewed changes

op-node/p2p/node.go Show resolved Hide resolved

refcell marked this pull request as ready for review January 23, 2023 15:22

refcell requested a review from a team as a code owner January 23, 2023 15:22

refcell requested a review from tynes January 23, 2023 15:22

fix: lints

e18788f

trianglesphere reviewed Jan 23, 2023

View reviewed changes

protolambda reviewed Jan 23, 2023

View reviewed changes

refcell mentioned this pull request Jan 23, 2023

feat: op-node docs #4766

Merged

Update op-node/metrics/metrics.go

d92d1c0

Co-authored-by: protolambda <proto@protolambda.com>

tynes reviewed Jan 23, 2023

View reviewed changes

op-node/p2p/peer_scorer.go Outdated Show resolved Hide resolved

mergify bot added the conflict label Jan 23, 2023

refcell added 2 commits January 23, 2023 16:21

feat: config

1630ad3

fix nits ✨

2e0320a

mergify bot removed the conflict label Jan 24, 2023

refcell changed the title ~~feat: op-node Peer Score Metrics~~ feat: op-node peer scoring Jan 24, 2023

mslipper marked this pull request as draft January 26, 2023 18:17

semgrep-app bot reviewed Mar 9, 2023

View reviewed changes

op-node/p2p/peer_gater.go Outdated Show resolved Hide resolved

refcell added 6 commits March 9, 2023 15:03

switch metrics to gauge vec

e96b8fe

fix metrics and add e2e test

e58e5ec

nil dereference

9cbd205

fix formatting and lints ✨

4ff4d35

fix peer scores test

69af4e5

more test fixing

4d0bb35

refcell marked this pull request as ready for review March 9, 2023 23:41

refcell added 2 commits March 9, 2023 19:03

Merge branch 'develop' into refcell/p2p_cli

38a10d7

fix logging construction

5ae60bf

refcell requested review from protolambda and ajsutton March 10, 2023 00:50

ajsutton reviewed Mar 10, 2023

View reviewed changes

op-node/metrics/metrics.go Outdated Show resolved Hide resolved

op-node/p2p/peer_params.go Show resolved Hide resolved

op-e2e/system_test.go Show resolved Hide resolved

refcell added 2 commits March 10, 2023 10:07

Merge branch 'develop' into refcell/p2p_cli

eb73def

clean flags and metrics

efcf30a

protolambda approved these changes Mar 10, 2023

View reviewed changes

op-e2e/system_test.go Show resolved Hide resolved

refcell added 4 commits March 10, 2023 20:31

comment out tests

40fc43c

stash system test peer scoring changes for merge

409fbfe

Merge branch 'master' into refcell/p2p_cli

5e65978

Merge branch 'develop' into refcell/p2p_cli

831f9f6

refcell requested review from protolambda and ajsutton March 11, 2023 01:54

mergify bot merged commit c785e35 into develop Mar 11, 2023

mergify bot deleted the refcell/p2p_cli branch March 11, 2023 01:58

mergify bot added the on-merge-train label Mar 11, 2023

mergify bot removed the on-merge-train label Mar 11, 2023

trianglesphere reviewed Mar 13, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(op-node): Peer Score Hardening #4743

feat(op-node): Peer Score Hardening #4743

refcell commented Jan 19, 2023 •

edited

Loading

changeset-bot bot commented Jan 19, 2023 •

edited

Loading

tynes commented Jan 19, 2023

refcell commented Jan 20, 2023

trianglesphere left a comment

tynes commented Jan 23, 2023

mergify bot commented Jan 23, 2023

codecov bot commented Mar 10, 2023 •

edited

Loading

ajsutton left a comment

protolambda left a comment

mergify bot commented Mar 11, 2023

mergify bot commented Mar 11, 2023

trianglesphere Mar 13, 2023

feat(op-node): Peer Score Hardening #4743

feat(op-node): Peer Score Hardening #4743

Conversation

refcell commented Jan 19, 2023 • edited Loading

changeset-bot bot commented Jan 19, 2023 • edited Loading

⚠️ No Changeset found

tynes commented Jan 19, 2023

refcell commented Jan 20, 2023

trianglesphere left a comment

Choose a reason for hiding this comment

tynes commented Jan 23, 2023

mergify bot commented Jan 23, 2023

codecov bot commented Mar 10, 2023 • edited Loading

Codecov Report

ajsutton left a comment

Choose a reason for hiding this comment

protolambda left a comment

Choose a reason for hiding this comment

mergify bot commented Mar 11, 2023

mergify bot commented Mar 11, 2023

trianglesphere Mar 13, 2023

Choose a reason for hiding this comment

refcell commented Jan 19, 2023 •

edited

Loading

changeset-bot bot commented Jan 19, 2023 •

edited

Loading

codecov bot commented Mar 10, 2023 •

edited

Loading