Skip to content

Commit

Permalink
adr: Peer Behaviour (tendermint#3539)
Browse files Browse the repository at this point in the history
* [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
brapse authored and Stumble committed Jul 25, 2019
1 parent 49ff449 commit 720f876
Showing 1 changed file with 145 additions and 0 deletions.
145 changes: 145 additions & 0 deletions docs/architecture/adr-037-peer-behaviour.md
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

0 comments on commit 720f876

Please sign in to comment.