feat(network): blacklist peers after a threshold of malicious activities has been reached

[AlonLStarkWare]

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 84.37500% with 5 lines in your changes missing coverage. Please review.

Project coverage is 75.62%. Comparing base (b0cfe82) to head (93c2214).
Report is 392 commits behind head on main.

Files with missing lines	Patch %	Lines
...papyrus_network/src/network_manager/swarm_trait.rs	0.00%	4 Missing ⚠️
crates/papyrus_network/src/peer_manager/mod.rs	90.90%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #991      +/-   ##
==========================================
+ Coverage   74.18%   75.62%   +1.43%     
==========================================
  Files         359      359              
  Lines       36240    38093    +1853     
  Branches    36240    38093    +1853     
==========================================
+ Hits        26886    28806    +1920     
+ Misses       7220     7100     -120     
- Partials     2134     2187      +53     

Flag	Coverage Δ
	?

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ShahakShama]

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: 3 of 4 files reviewed, 4 unresolved discussions (waiting on @AlonLStarkWare)

---

crates/papyrus_network/src/network_manager/swarm_trait.rs line 132 at r1 (raw file):

            .behaviour_mut()
            .peer_manager
            .report_peer(peer_id, ReputationModifier::Malicious { maliciousness: 1.0 });

I don't like the name maliciousness. How about score? penalty?

---

crates/papyrus_network/src/peer_manager/mod.rs line 27 at r1 (raw file):

#[derive(Clone, Copy)]
pub enum ReputationModifier {
    Malicious { maliciousness: f64 },

Add documentation that maliciousness should be between 0 and 1 and if the sum of malicious activity is above 1 then the node is considered malicious

---

crates/papyrus_network/src/peer_manager/peer.rs line 27 at r1 (raw file):

    fn remove_connection_id(&mut self, connection_id: ConnectionId);

    fn get_maliciousness(&self) -> f64;

Instead of these two functions, define the following functions:

1. report(&mut self, maliciousness: f64)
2. is_malicious(&self) -> bool
3. reset_maliciousness(&mut self)

The main idea is that the functions should contain as much "implementation details" inside them rather than exposing the implementation in the API

---

crates/papyrus_network/src/peer_manager/test.rs line 289 at r1 (raw file):

    peer_manager
        .report_session(outbound_session_id, ReputationModifier::Malicious { maliciousness: 1.0 })

Add 1.0 as a constant called MALICIOUS in peer_manager/mod.rs. Name the constant according to the name you chose for the rename of maliciousness (e.g MALICIOUS_SCORE)

[AlonLStarkWare]

Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @ShahakShama)

---

crates/papyrus_network/src/network_manager/swarm_trait.rs line 132 at r1 (raw file):

Previously, ShahakShama wrote…

I don't like the name maliciousness. How about score? penalty?

Of course, it was a placeholder name. How about misconduct_score?

---

crates/papyrus_network/src/peer_manager/peer.rs line 27 at r1 (raw file):

Previously, ShahakShama wrote…

Instead of these two functions, define the following functions:

1. report(&mut self, maliciousness: f64)
2. is_malicious(&self) -> bool
3. reset_maliciousness(&mut self)

The main idea is that the functions should contain as much "implementation details" inside them rather than exposing the implementation in the API

Done.

---

crates/papyrus_network/src/peer_manager/test.rs line 289 at r1 (raw file):

Previously, ShahakShama wrote…

Add 1.0 as a constant called MALICIOUS in peer_manager/mod.rs. Name the constant according to the name you chose for the rename of maliciousness (e.g MALICIOUS_SCORE)

Done.

[ShahakShama]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @AlonLStarkWare)

---

crates/papyrus_network/src/network_manager/swarm_trait.rs line 128 at r1 (raw file):

    }

    fn report_peer_as_malicious(&mut self, peer_id: PeerId) {

Add TODO to change this to report_peer, receive a score here as an argument and receive a score from the user when they report peer with BroadcastedMessageMetadata

---

crates/papyrus_network/src/network_manager/swarm_trait.rs line 132 at r1 (raw file):

Previously, AlonLStarkWare (Alon-Lukatch-Starkware) wrote…

Of course, it was a placeholder name. How about misconduct_score?

Great! now rename ReputationModifier::Malicious to ReputationModifier::Misconduct

---

crates/papyrus_network/src/peer_manager/mod.rs line 29 at r2 (raw file):

#[derive(Clone, Copy)]
pub enum ReputationModifier {
    Malicious { misconduct_score: f64 }, /* misconduct_score is in the range [0, 1]. When a

Change the comment into a /// on top of the variant

---

crates/papyrus_network/src/peer_manager/mod.rs line 30 at r2 (raw file):

pub enum ReputationModifier {
    Malicious { misconduct_score: f64 }, /* misconduct_score is in the range [0, 1]. When a
                                          * peer's misconduct_score reaches 1, it is

when a peer's total/sum misconduct_score reaches 1, ...

---

crates/papyrus_network/src/peer_manager/peer.rs line 50 at r2 (raw file):

            timed_out_until: get_instant_now(),
            connection_ids: Vec::new(),
            misconduct_score: 0 as f64,

write 0f64 instead of 0 as f64

---

crates/papyrus_network/src/peer_manager/peer.rs line 96 at r2 (raw file):

    fn reset_misconduct_score(&mut self) {
        self.misconduct_score = 0 as f64;

write 0f64 instead of 0 as f64

---

crates/papyrus_network/src/peer_manager/peer.rs line 104 at r2 (raw file):

    fn is_malicious(&self) -> bool {
        1.0 <= self.misconduct_score

write 1.0f64 just to be sure that rust doesn't auto cast misconduct_score to f32

[AlonLStarkWare]

Reviewable status: 0 of 4 files reviewed, 6 unresolved discussions (waiting on @ShahakShama)

---

crates/papyrus_network/src/network_manager/swarm_trait.rs line 128 at r1 (raw file):

Previously, ShahakShama wrote…

Add TODO to change this to report_peer, receive a score here as an argument and receive a score from the user when they report peer with BroadcastedMessageMetadata

Done.

---

crates/papyrus_network/src/peer_manager/mod.rs line 29 at r2 (raw file):

Previously, ShahakShama wrote…

Change the comment into a /// on top of the variant

Done.

---

crates/papyrus_network/src/peer_manager/mod.rs line 30 at r2 (raw file):

Previously, ShahakShama wrote…

when a peer's total/sum misconduct_score reaches 1, ...

Done.

---

crates/papyrus_network/src/peer_manager/peer.rs line 50 at r2 (raw file):

Previously, ShahakShama wrote…

write 0f64 instead of 0 as f64

Done.

---

crates/papyrus_network/src/peer_manager/peer.rs line 96 at r2 (raw file):

Previously, ShahakShama wrote…

write 0f64 instead of 0 as f64

Done.

---

crates/papyrus_network/src/peer_manager/peer.rs line 104 at r2 (raw file):

Previously, ShahakShama wrote…

write 1.0f64 just to be sure that rust doesn't auto cast misconduct_score to f32

Done.

[ShahakShama]

Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AlonLStarkWare)

---

crates/papyrus_network/src/network_manager/swarm_trait.rs line 128 at r1 (raw file):

Previously, AlonLStarkWare (Alon-Lukatch-Starkware) wrote…

Done.

Where's the TODO

[AlonLStarkWare]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ShahakShama)

---

crates/papyrus_network/src/network_manager/swarm_trait.rs line 128 at r1 (raw file):

Previously, ShahakShama wrote…

Where's the TODO

Added in the trait

[ShahakShama]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @AlonLStarkWare)

[ShahakShama]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @AlonLStarkWare)