From f2119c35de4118fbe6b45e578e9b6cc72ad8b3cb Mon Sep 17 00:00:00 2001
From: Sean Braithwaite <brapse@gmail.com>
Date: Mon, 15 Apr 2019 16:38:45 +0200
Subject: [PATCH] adr: PeerBehaviour updates (#3558)

* [adr] Peer behaviour adr updates
* [docs] fix Behaved function signature
* [adr] typo fix in code example
---
 ...behaviour.md => adr-039-peer-behaviour.md} | 84 +++++++++++--------
 1 file changed, 49 insertions(+), 35 deletions(-)
 rename docs/architecture/{adr-037-peer-behaviour.md => adr-039-peer-behaviour.md} (57%)

diff --git a/docs/architecture/adr-037-peer-behaviour.md b/docs/architecture/adr-039-peer-behaviour.md
similarity index 57%
rename from docs/architecture/adr-037-peer-behaviour.md
rename to docs/architecture/adr-039-peer-behaviour.md
index 36b024482..4ad051a35 100644
--- a/docs/architecture/adr-037-peer-behaviour.md
+++ b/docs/architecture/adr-039-peer-behaviour.md
@@ -1,7 +1,8 @@
-# ADR 037: Peer Behaviour Interface
+# ADR 039: Peer Behaviour Interface
 
 ## Changelog
 * 07-03-2019: Initial draft
+* 14-03-2019: Updates from feedback
 
 ## Context
 
@@ -19,36 +20,46 @@ and ties up the reactors in a larger dependency graph when testing.
 
 Introduce a `PeerBehaviour` interface and concrete implementations which
 provide methods for reactors to signal peer behaviour without direct
-coupling `p2p.Switch`.  Introduce a ErrPeer to provide 
-concrete reasons for stopping peers.
+coupling `p2p.Switch`.  Introduce a ErrorBehaviourPeer to provide
+concrete reasons for stopping peers. Introduce GoodBehaviourPeer to provide
+concrete ways in which a peer contributes.
 
 ### Implementation Changes
 
 PeerBehaviour then becomes an interface for signaling peer errors as well
 as for marking peers as `good`.
 
-XXX: It might be better to pass p2p.ID instead of the whole peer but as
-a first draft maintain the underlying implementation as much as
-possible.
-
 ```go
 type PeerBehaviour interface {
-    Errored(peer Peer, reason ErrPeer)
-    MarkPeerAsGood(peer Peer)
+    Behaved(peer Peer, reason GoodBehaviourPeer)
+    Errored(peer Peer, reason ErrorBehaviourPeer)
 }
 ```
 
 Instead of signaling peers to stop with arbitrary reasons:
 `reason interface{}` 
 
-We introduce a concrete error type ErrPeer:
+We introduce a concrete error type ErrorBehaviourPeer:
 ```go
-type ErrPeer int
+type ErrorBehaviourPeer int
 
 const (
-    ErrPeerUnknown = iota
-    ErrPeerBadMessage
-    ErrPeerMessageOutofOrder
+    ErrorBehaviourUnknown = iota
+    ErrorBehaviourBadMessage
+    ErrorBehaviourMessageOutofOrder
+    ...
+)
+```
+
+To provide additional information on the ways a peer contributed, we introduce
+the GoodBehaviourPeer type.
+
+```go
+type GoodBehaviourPeer int
+
+const (
+    GoodBehaviourVote = iota
+    GoodBehaviourBlockPart
     ...
 )
 ```
@@ -60,11 +71,11 @@ type SwitchedPeerBehaviour struct {
     sw *Switch
 }
 
-func (spb *SwitchedPeerBehaviour) Errored(peer Peer, reason ErrPeer) {
+func (spb *SwitchedPeerBehaviour) Errored(peer Peer, reason ErrorBehaviourPeer) {
     spb.sw.StopPeerForError(peer, reason)
 }
 
-func (spb *SwitchedPeerBehaviour) MarkPeerAsGood(peer Peer) {
+func (spb *SwitchedPeerBehaviour) Behaved(peer Peer, reason GoodBehaviourPeer) {
     spb.sw.MarkPeerAsGood(peer)
 }
 
@@ -75,51 +86,54 @@ func NewSwitchedPeerBehaviour(sw *Switch) *SwitchedPeerBehaviour {
 }
 ```
 
-Reactors, which are often difficult to unit test[<sup>2</sup>](#references). could use an implementation which exposes the signals produced by the reactor in
+Reactors, which are often difficult to unit test[<sup>2</sup>](#references) could use an implementation which exposes the signals produced by the reactor in
 manufactured scenarios:
 
 ```go
-type PeerErrors map[Peer][]ErrPeer
-type GoodPeers map[Peer]bool
+type ErrorBehaviours map[Peer][]ErrorBehaviourPeer
+type GoodBehaviours map[Peer][]GoodBehaviourPeer
 
 type StorePeerBehaviour struct {
-    pe PeerErrors
-    gp GoodPeers
+    eb ErrorBehaviours
+    gb GoodBehaviours
 }
 
 func NewStorePeerBehaviour() *StorePeerBehaviour{
     return &StorePeerBehaviour{
-        pe: make(PeerErrors),
-        gp: GoodPeers{},
+        eb: make(ErrorBehaviours),
+        gb: make(GoodBehaviours),
     }
 }
 
-func (spb StorePeerBehaviour) Errored(peer Peer, reason ErrPeer) {
-    if _, ok := spb.pe[peer]; !ok {
-        spb.pe[peer] = []ErrPeer{reason}
+func (spb StorePeerBehaviour) Errored(peer Peer, reason ErrorBehaviourPeer) {
+    if _, ok := spb.eb[peer]; !ok {
+        spb.eb[peer] = []ErrorBehaviours{reason}
     } else {
-        spb.pe[peer] = append(spb.pe[peer], reason)
+        spb.eb[peer] = append(spb.eb[peer], reason)
     }
 }
 
-func (mpb *StorePeerBehaviour) GetPeerErrors() PeerErrors {
-    return mpb.pe
+func (mpb *StorePeerBehaviour) GetErrored() ErrorBehaviours {
+    return mpb.eb
 }
 
-func (spb *StorePeerBehaviour) MarkPeerAsGood(peer Peer) {
-    if _, ok := spb.gp[peer]; !ok {
-        spb.gp[peer] = true
+
+func (spb StorePeerBehaviour) Behaved(peer Peer, reason GoodBehaviourPeer) {
+    if _, ok := spb.gb[peer]; !ok {
+        spb.gb[peer] = []GoodBehaviourPeer{reason}
+    } else {
+        spb.gb[peer] = append(spb.gb[peer], reason)
     }
 }
 
-func (spb *StorePeerBehaviour) GetGoodPeers() GoodPeers {
-    return spb.gp
+func (spb *StorePeerBehaviour) GetBehaved() GoodBehaviours {
+    return spb.gb
 }
 ```
 
 ## Status
 
-Proposed
+Accepted
 
 ## Consequences