-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* [adr] ADR 037: Peer Behaviour inital draft * Update docs/architecture/adr-037-peer-behaviour.md Co-Authored-By: brapse <brapse@gmail.com> * Update docs/architecture/adr-037-peer-behaviour.md Co-Authored-By: brapse <brapse@gmail.com> * [docs] adr-037 Better footnote styling * [ADR] ADR-037 adjust Footnotes for github markdown * [ADR] ADR-037 fix numbered list
- Loading branch information
Showing
1 changed file
with
145 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,145 @@ | ||
# ADR 037: Peer Behaviour Interface | ||
|
||
## Changelog | ||
* 07-03-2019: Initial draft | ||
|
||
## Context | ||
|
||
The responsibility for signaling and acting upon peer behaviour lacks a single | ||
owning component and is heavily coupled with the network stack[<sup>1</sup>](#references). Reactors | ||
maintain a reference to the `p2p.Switch` which they use to call | ||
`switch.StopPeerForError(...)` when a peer misbehaves and | ||
`switch.MarkAsGood(...)` when a peer contributes in some meaningful way. | ||
While the switch handles `StopPeerForError` internally, the `MarkAsGood` | ||
method delegates to another component, `p2p.AddrBook`. This scheme of delegation | ||
across Switch obscures the responsibility for handling peer behaviour | ||
and ties up the reactors in a larger dependency graph when testing. | ||
|
||
## Decision | ||
|
||
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. | ||
|
||
### 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) | ||
} | ||
``` | ||
|
||
Instead of signaling peers to stop with arbitrary reasons: | ||
`reason interface{}` | ||
|
||
We introduce a concrete error type ErrPeer: | ||
```go | ||
type ErrPeer int | ||
|
||
const ( | ||
ErrPeerUnknown = iota | ||
ErrPeerBadMessage | ||
ErrPeerMessageOutofOrder | ||
... | ||
) | ||
``` | ||
|
||
As a first iteration we provide a concrete implementation which wraps | ||
the switch: | ||
```go | ||
type SwitchedPeerBehaviour struct { | ||
sw *Switch | ||
} | ||
|
||
func (spb *SwitchedPeerBehaviour) Errored(peer Peer, reason ErrPeer) { | ||
spb.sw.StopPeerForError(peer, reason) | ||
} | ||
|
||
func (spb *SwitchedPeerBehaviour) MarkPeerAsGood(peer Peer) { | ||
spb.sw.MarkPeerAsGood(peer) | ||
} | ||
|
||
func NewSwitchedPeerBehaviour(sw *Switch) *SwitchedPeerBehaviour { | ||
return &SwitchedPeerBehaviour{ | ||
sw: sw, | ||
} | ||
} | ||
``` | ||
|
||
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 StorePeerBehaviour struct { | ||
pe PeerErrors | ||
gp GoodPeers | ||
} | ||
|
||
func NewStorePeerBehaviour() *StorePeerBehaviour{ | ||
return &StorePeerBehaviour{ | ||
pe: make(PeerErrors), | ||
gp: GoodPeers{}, | ||
} | ||
} | ||
|
||
func (spb StorePeerBehaviour) Errored(peer Peer, reason ErrPeer) { | ||
if _, ok := spb.pe[peer]; !ok { | ||
spb.pe[peer] = []ErrPeer{reason} | ||
} else { | ||
spb.pe[peer] = append(spb.pe[peer], reason) | ||
} | ||
} | ||
|
||
func (mpb *StorePeerBehaviour) GetPeerErrors() PeerErrors { | ||
return mpb.pe | ||
} | ||
|
||
func (spb *StorePeerBehaviour) MarkPeerAsGood(peer Peer) { | ||
if _, ok := spb.gp[peer]; !ok { | ||
spb.gp[peer] = true | ||
} | ||
} | ||
|
||
func (spb *StorePeerBehaviour) GetGoodPeers() GoodPeers { | ||
return spb.gp | ||
} | ||
``` | ||
|
||
## Status | ||
|
||
Proposed | ||
|
||
## Consequences | ||
|
||
### Positive | ||
|
||
* De-couple signaling from acting upon peer behaviour. | ||
* Reduce the coupling of reactors and the Switch and the network | ||
stack | ||
* The responsibility of managing peer behaviour can be migrated to | ||
a single component instead of split between the switch and the | ||
address book. | ||
|
||
### Negative | ||
|
||
* The first iteration will simply wrap the Switch and introduce a | ||
level of indirection. | ||
|
||
### Neutral | ||
|
||
## References | ||
|
||
1. Issue [#2067](https://github.com/tendermint/tendermint/issues/2067): P2P Refactor | ||
2. PR: [#3506](https://github.com/tendermint/tendermint/pull/3506): ADR 036: Blockchain Reactor Refactor |