From ce8681864e96fad1ae6608ae365b1230f0ee2d8e Mon Sep 17 00:00:00 2001 From: derrandz Date: Wed, 5 Apr 2023 00:47:03 +0000 Subject: [PATCH 01/16] doc: initial commit --- .../adr-013-bootstrap-from-previous-peers.md | 151 ++++++++++++++++++ nodebuilder/header/flags.go | 2 +- 2 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 docs/adr/adr-013-bootstrap-from-previous-peers.md diff --git a/docs/adr/adr-013-bootstrap-from-previous-peers.md b/docs/adr/adr-013-bootstrap-from-previous-peers.md new file mode 100644 index 0000000000..cd751e005b --- /dev/null +++ b/docs/adr/adr-013-bootstrap-from-previous-peers.md @@ -0,0 +1,151 @@ +# ADR {ADR-NUMBER}: {TITLE} + +## Changelog + +* 2023-05-04: initial draft + +## Context + +Celestia node relies on a set or peer to peer protocols that assume a decentralized network, where nodes are connected to each other and can exchange data. However, in the case of a new node joining the network, the node does not have any peers to connect to (_apart from the hardcoded bootstrappers_), and thus cannot bootstrap itself. This is a problem for it makes the network centralized and reliant on the few hardcoded nodes. + +In this ADR, we wish to aleviate this problem by allowing nodes to bootstrap from previously seen peers. This is a simple solution that allows the network to gain more decentralization, where nodes can join the network without the need for hardcoded bootstrappers, as long as they have already seen at least one peer. + +_Original issue available in references_ + +## Decision + +Periodically store the addresses of previously seen peers in a new key-value database, and allow the node to bootstrap from these peers on startup. + +## Detailed Design + +### What systems will be affected? + +- `peerTracker` and `libhead.Exchange` from `go-header` +- `newInitStore` in `nodebuilder/header.go` + + +### Database + +We will use a badgerDB datastore to store the addresses of previously seen peers. The database will be stored in the `data` directory, and will be named `peers.db`. The database will have a single key-value pair, where the key is the peer ID, and the value is the peer multiaddress. + +The peer store will implement the following interface: + +```go +type PeerStore interface { + // Put adds a peer to the store. + Put(peer.ID, ma.Multiaddr) error + // Get returns a peer from the store. + Get(peer.ID) (ma.Multiaddr, error) + // Delete removes a peer from the store. + Delete(peer.ID) error +} +``` + +And we expect the underlying implementation to use a badgerDB datastore. Example: +```go +type peerStore struct { + db datastore.Batching +} +``` + +### Peer Selection + +Since bootstrappers are primarily used as trustedPeers to kick off the header exchange process, we suggest to only select trustedPeers that have answered with a header in the last 24 hours. This will ensure that the node is only bootstrapping from peers that are still active. + +This means initially that on node startup, specifically on store initialization, we should have a mechanism that informs us about which trustedPeers have successfully answered the `Get` request to retrieve the trusted hash (_to initialize the store_). In the current state of affairs, with `go-header` being a separate repository, we suggest to extend internal functionality of `go-header` such that the `libhead.Exchange`'s `peerTracker` is able to select peers based on the criteria mentioned above. + +To achieve this, we will first extend `peerStat` to include a new attribute `isTrustedPeer`: +```go +type peerStat struct { + // ... + isTrustedPeer bool +} +``` +and add a new method to `peerTracker` named `rewardTrustedPeer` such that it increases the score of a tracked peer (_that is a trusted peer_) by 10(_TODO: rethink this value_) points to mark it as a good _bootstrapping_ peer: +```go +func (pt *peerTracker) rewardTrustedPeer(id peer.ID) { + pt.mu.Lock() + defer pt.mu.Unlock() + + if stat, ok := pt.trackedPeers[id]; ok && stat.isTrustedPeer { + stat.peerScore +=10 + } +} +``` + +Then, we would update `libhead.Exchange`'s private method `performRequest` to explicitly return the peers that answered with the header, then in `Get` call `rewardTrustedPeer` on the `peerTracker` when a request to a trusted peer was successful. + +This will allow a tracking of which trusted peers were successful in answering the `Get` request, and thus can be used to bootstrap from. + +Similarly, we will add a new method to `peerTracker` named `GetTrustedPeers` that returns a list of trusted peers that have answered with a header: +```go +func (pt *peerTracker) GetTrustedPeers() []peer.ID { + pt.mu.Lock() + defer pt.mu.Unlock() + + var peers []peer.ID + for id, stat := range pt.tracedPeers { + if stat.isTrustedPeer && stat.peerScore >= 10 { + peers = append(peers, id) + } + } + return peers +} +``` + +Then in `store.Init`, we pass an instance of the peer store/database mentioned above, and perform a call to this method. We then iterate over this list and add each peer to the peer store/database. + +(_MISSING: TODO: Add design for logic to only store peers that were active since the last 24 hours_) + +### Node Startup + +When the node starts up, it will first check if the `peers.db` database exists. If it does not, the node will bootstrap from the hardcoded bootstrappers. If it does, the node will bootstrap from the peers in the database. + +## Status + +Proposed + +## Consequences + +### Positive + +* Allows nodes to bootstrap from previously seen peers, which allows the network to gain more decentralization. + + +### Negative + +* peerTracker scoring logic might be impacted + + + + +## References + +- [Original Issue](https://github.com/celestiaorg/celestia-node/issues/1851) diff --git a/nodebuilder/header/flags.go b/nodebuilder/header/flags.go index aa4cb093a1..5c7368fa57 100644 --- a/nodebuilder/header/flags.go +++ b/nodebuilder/header/flags.go @@ -38,7 +38,7 @@ func TrustedPeersFlags() *flag.FlagSet { flags.StringSlice( headersTrustedPeersFlag, nil, - "Multiaddresses of a reliable peers to fetch headers from. (Format: multiformats.io/multiaddr)", + "Multiaddresses of reliable peers to fetch headers from. (Format: multiformats.io/multiaddr)", ) return flags } From 10c10390e937f73f0917100030704f447bfb0d8c Mon Sep 17 00:00:00 2001 From: derrandz Date: Wed, 5 Apr 2023 00:53:49 +0000 Subject: [PATCH 02/16] doc: add periodic logic --- .../adr-013-bootstrap-from-previous-peers.md | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/docs/adr/adr-013-bootstrap-from-previous-peers.md b/docs/adr/adr-013-bootstrap-from-previous-peers.md index cd751e005b..f21706507a 100644 --- a/docs/adr/adr-013-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-013-bootstrap-from-previous-peers.md @@ -94,6 +94,65 @@ func (pt *peerTracker) GetTrustedPeers() []peer.ID { ``` Then in `store.Init`, we pass an instance of the peer store/database mentioned above, and perform a call to this method. We then iterate over this list and add each peer to the peer store/database. +```go +// Init ensures a Store is initialized. If it is not already initialized, +// it initializes the Store by requesting the header with the given hash. +func Init[H header.Header](ctx context.Context, store header.Store[H], ex header.Exchange[H], hash header.Hash, pstore store.PeerStore) error { + _, err := store.Head(ctx) + switch err { + default: + return err + case header.ErrNoHead: + initial, err := ex.Get(ctx, hash) + if err != nil { + return err + } + + err := store.Init(ctx, initial) + if err != nil { + return err + } + + peers := ex.PeerTracker().GetTrustedPeers() + for _, peer := range peers { + p := ex.PeerTracker().Get(peer) + err = pstore.Put(peer, p.Multiaddr) + if err != nil { + return err + } + } + + return err + } +} +``` + +Also, we will add a routine that periodically checks for new peers that have answered with a header, and adds them to the peer store/database. This routine will be started in `store.Init`: +```go + + fx.Provide( + // ... + func(lc fx.Lifecycle, ex header.Exchange[H], pstore store.PeerStore) { + lc.Append(fx.Hook{ + OnStart: func(ctx context.Context) error { + go func() { + for { + peers := ex.PeerTracker().GetTrustedPeers() + for _, peer := range peers { + p := ex.PeerTracker().Get(peer) + err = pstore.Put(peer, p.Multiaddr) + if err != nil { + return err + } + } + time.Sleep(1 * time.Hour) + } + }() + return nil + }, + }) + }, +``` (_MISSING: TODO: Add design for logic to only store peers that were active since the last 24 hours_) From a09e9b25feefbc76dc9fbec001add1833f315863 Mon Sep 17 00:00:00 2001 From: derrandz Date: Wed, 5 Apr 2023 15:11:54 +0000 Subject: [PATCH 03/16] fix: adr title and make linter happy --- docs/adr/adr-013-bootstrap-from-previous-peers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/adr-013-bootstrap-from-previous-peers.md b/docs/adr/adr-013-bootstrap-from-previous-peers.md index f21706507a..cb2ad3e623 100644 --- a/docs/adr/adr-013-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-013-bootstrap-from-previous-peers.md @@ -1,4 +1,4 @@ -# ADR {ADR-NUMBER}: {TITLE} +# ADR 013: Bootstrap from previous peers ## Changelog From 59f07c138b2aad6e04464c8a0f687dcdeff02cff Mon Sep 17 00:00:00 2001 From: derrandz Date: Wed, 5 Apr 2023 15:16:44 +0000 Subject: [PATCH 04/16] fix: language --- .../adr-013-bootstrap-from-previous-peers.md | 58 +++++++++++-------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/docs/adr/adr-013-bootstrap-from-previous-peers.md b/docs/adr/adr-013-bootstrap-from-previous-peers.md index cb2ad3e623..8c24a03fc0 100644 --- a/docs/adr/adr-013-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-013-bootstrap-from-previous-peers.md @@ -6,23 +6,30 @@ ## Context -Celestia node relies on a set or peer to peer protocols that assume a decentralized network, where nodes are connected to each other and can exchange data. However, in the case of a new node joining the network, the node does not have any peers to connect to (_apart from the hardcoded bootstrappers_), and thus cannot bootstrap itself. This is a problem for it makes the network centralized and reliant on the few hardcoded nodes. +Celestia node relies on a set of peer to peer protocols that assume a decentralized network, where nodes are connected to each other and can exchange data. -In this ADR, we wish to aleviate this problem by allowing nodes to bootstrap from previously seen peers. This is a simple solution that allows the network to gain more decentralization, where nodes can join the network without the need for hardcoded bootstrappers, as long as they have already seen at least one peer. +Currently, when a node joins/comes up on the network, it relies, by default, on hardcoded bootstrappers for 3 things: -_Original issue available in references_ +1. initializing its store with the first (genesis) block +2. requesting the network head to which it sets its initial sync target, and +3. bootstrapping itself with some other peers on the network which it can utilise for its operations. + +This is centralisation bottleneck as it happens both during initial start-up and for any re-starts. + +In this ADR, we wish to aleviate this problem by allowing nodes to bootstrap from previously seen peers. This is a simple solution that allows the network alleviate the start-up centralisation bottleneck such that nodes can join the network without the need for hardcoded bootstrappers up re-start + +(_Original issue available in references_) ## Decision -Periodically store the addresses of previously seen peers in a new key-value database, and allow the node to bootstrap from these peers on startup. +Periodically store the addresses of previously seen peers in a key-value database, and allow the node to bootstrap from these peers on start-up. ## Detailed Design ### What systems will be affected? -- `peerTracker` and `libhead.Exchange` from `go-header` -- `newInitStore` in `nodebuilder/header.go` - +* `peerTracker` and `libhead.Exchange` from `go-header` +* `newInitStore` in `nodebuilder/header.go` ### Database @@ -42,6 +49,7 @@ type PeerStore interface { ``` And we expect the underlying implementation to use a badgerDB datastore. Example: + ```go type peerStore struct { db datastore.Batching @@ -55,13 +63,16 @@ Since bootstrappers are primarily used as trustedPeers to kick off the header ex This means initially that on node startup, specifically on store initialization, we should have a mechanism that informs us about which trustedPeers have successfully answered the `Get` request to retrieve the trusted hash (_to initialize the store_). In the current state of affairs, with `go-header` being a separate repository, we suggest to extend internal functionality of `go-header` such that the `libhead.Exchange`'s `peerTracker` is able to select peers based on the criteria mentioned above. To achieve this, we will first extend `peerStat` to include a new attribute `isTrustedPeer`: + ```go type peerStat struct { // ... isTrustedPeer bool } ``` + and add a new method to `peerTracker` named `rewardTrustedPeer` such that it increases the score of a tracked peer (_that is a trusted peer_) by 10(_TODO: rethink this value_) points to mark it as a good _bootstrapping_ peer: + ```go func (pt *peerTracker) rewardTrustedPeer(id peer.ID) { pt.mu.Lock() @@ -78,6 +89,7 @@ Then, we would update `libhead.Exchange`'s private method `performRequest` to ex This will allow a tracking of which trusted peers were successful in answering the `Get` request, and thus can be used to bootstrap from. Similarly, we will add a new method to `peerTracker` named `GetTrustedPeers` that returns a list of trusted peers that have answered with a header: + ```go func (pt *peerTracker) GetTrustedPeers() []peer.ID { pt.mu.Lock() @@ -94,21 +106,22 @@ func (pt *peerTracker) GetTrustedPeers() []peer.ID { ``` Then in `store.Init`, we pass an instance of the peer store/database mentioned above, and perform a call to this method. We then iterate over this list and add each peer to the peer store/database. + ```go // Init ensures a Store is initialized. If it is not already initialized, // it initializes the Store by requesting the header with the given hash. func Init[H header.Header](ctx context.Context, store header.Store[H], ex header.Exchange[H], hash header.Hash, pstore store.PeerStore) error { - _, err := store.Head(ctx) - switch err { - default: - return err - case header.ErrNoHead: - initial, err := ex.Get(ctx, hash) - if err != nil { - return err - } - - err := store.Init(ctx, initial) + _, err := store.Head(ctx) + switch err { + default: + return err + case header.ErrNoHead: + initial, err := ex.Get(ctx, hash) + if err != nil { + return err + } + + err := store.Init(ctx, initial) if err != nil { return err } @@ -123,13 +136,13 @@ func Init[H header.Header](ctx context.Context, store header.Store[H], ex header } return err - } + } } ``` Also, we will add a routine that periodically checks for new peers that have answered with a header, and adds them to the peer store/database. This routine will be started in `store.Init`: -```go +```go fx.Provide( // ... func(lc fx.Lifecycle, ex header.Exchange[H], pstore store.PeerStore) { @@ -152,6 +165,7 @@ Also, we will add a routine that periodically checks for new peers that have ans }, }) }, + ) ``` (_MISSING: TODO: Add design for logic to only store peers that were active since the last 24 hours_) @@ -170,12 +184,10 @@ Proposed * Allows nodes to bootstrap from previously seen peers, which allows the network to gain more decentralization. - ### Negative * peerTracker scoring logic might be impacted - - ## References * [Original Issue](https://github.com/celestiaorg/celestia-node/issues/1851) From 171895b57479495643f42f5cf71428326a9166bc Mon Sep 17 00:00:00 2001 From: derrandz Date: Wed, 12 Apr 2023 00:24:16 +0000 Subject: [PATCH 10/16] doc: last touches --- .../adr-014-bootstrap-from-previous-peers.md | 135 ++++++++++++++---- 1 file changed, 107 insertions(+), 28 deletions(-) diff --git a/docs/adr/adr-014-bootstrap-from-previous-peers.md b/docs/adr/adr-014-bootstrap-from-previous-peers.md index 7b75473700..068360c9ec 100644 --- a/docs/adr/adr-014-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-014-bootstrap-from-previous-peers.md @@ -17,7 +17,7 @@ Currently, when a node joins/comes up on the network, it relies, by default, on 2. requesting the network head to which it sets its initial sync target, and 3. bootstrapping itself with some other peers on the network which it can utilise for its operations. -This is centralisation bottleneck as it happens both during initial start-up and for any re-starts. +This is a centralization bottleneck as it happens both during initial start-up and for any re-starts. In this ADR, we wish to aleviate this problem by allowing nodes to bootstrap from previously seen peers. This is a simple solution that allows the network alleviate the start-up centralisation bottleneck such that nodes can join the network without the need for hardcoded bootstrappers up re-start @@ -33,7 +33,7 @@ Periodically store the addresses of previously seen peers in a key-value databas
-In this approach, we will track on two important internal events from `peerTracker` in `libhead.Exchange` for bootstrapper peers selection and persistence purposes, and that is by proposing a design that allows access to `peerTracker`'s list of good peers and bad peers, and opening the design space to integrate peer selection and storage logic. +In this approach, we will rely on `peerTracker`'s scoring mechanism that tracks the fastest connected nodes, and blocks malicious ones. To make use of this functionality, we track two important internal events from `peerTracker` in `libhead.Exchange` for peers selection and persistence purposes, and that is by proposing a design that allows access to `peerTracker`'s list of good peers and bad peers, and opening the design space to integrate peer selection and storage logic. ## Implementation @@ -46,7 +46,7 @@ In this approach, we will track on two important internal events from `peerTrack ### Storage -We will use a badgerDB datastore to store the addresses of previously seen peers. The database will be stored in the `data` directory, and will be named `good_peers`. The database will have a single key-value pair, where the key is `peers` and the value is a `[]PeerInfo` +We will use a badgerDB datastore to store the addresses of previously seen good peers. The database will be stored in the `data` directory, and will have a store prefix `good_peers`. The database will have a single key-value pair, where the key is `peers` and the value is a `[]PeerInfo` The peer store will implement the following interface: @@ -75,14 +75,12 @@ type peerStore struct { To periodically select the _"good peers"_ that we're connected to and store them in the peer store, we will rely on the internals of `go-header`, specifically the `peerTracker` and `libhead.Exchange` structs. -`peerTracker` has internal logic that continuiously tracks and garbage collects peers, so that if new peers connect to the node, peer tracker keeps track of them in an internal list, as well as it marks disconnected peers for removal. `peerTracker` also blocks peers that behave maliciously. +`peerTracker` has internal logic that continuiously tracks and garbage collects peers based on whether they connected to the node, disconnected from the node, or responded to the node while taking into consideration response speed. All connected peers are kept track of in a list, and disconnected peers are marked for removal, all as a part of a garbage collection routine. `peerTracker` also blocks peers that behave maliciously, but not in a routine, instead, it does so on `blockPeer` call. -We would like to use this internal logic to periodically select peers that are "good" and store them in the peer store. To do this, we will "hook" into the internals of `peerTracker` and `libhead.Exchange` by defining new "event handlers" intended to handle two main events from `peerTracker`: +We intend to use this internal logic to periodically select peers that are "good" and store them in the peer store mentioned in the storage section. To do this, we will "hook" into the internals of `peerTracker` and `libhead.Exchange` by defining new "event handlers" intended to handle two main events from `peerTracker`: -1. `OnUpdatedPeers` -2. `OnBlockedPeer` - -_**Example of required changes to: go-header/p2p/peer_tracker.go**_ +1. `OnUpdatedPeers`: _This event is triggered on every garbage collection cycle from `peerTracker`. Before the triggering of this event, the list would have underwent changes from other routines as well and thus the most final version will be available at the time of triggering this event ._ +2. `OnBlockedPeer`: _This event is triggered every time the `blockPeer` action is called_ ```diff type peerTracker struct { @@ -97,7 +95,9 @@ type peerTracker struct { // done is used to gracefully stop the peerTracker. ``` -`OnUpdatedPeers` will be called whenever `peerTracker` updates its internal list of peers on its `gc` routine, which updates every 30 minutes (_value is configurable_): +(_Code Snippet 1.a: Example of required changes to: go-header/p2p/peer_tracker.go_) + +as explained above, `OnUpdatedPeers` will be called whenever `peerTracker` updates its internal list of peers on its `gc` routine, which updates every 30 minutes (_value is configurable_): ```diff func (p *peerTracker) gc() { @@ -125,6 +125,10 @@ func (p *peerTracker) gc() { } ``` +(_Code Snippet 1.b: Example of required changes to: go-header/p2p/peer_tracker.go_) + +In _Code Snippet 1.b_, we can choose to order the slice by `peer.peerScore` to allow the event handler to know which peers are best in connectivity, without leaking the `peerStat` type into the event handler callbacks. + where as `OnBlockedPeer` will be called whenever `peerTracker` blocks a peer through its method `blockPeer`. ```diff @@ -149,9 +153,11 @@ where as `OnBlockedPeer` will be called whenever `peerTracker` blocks a peer thr delete(p.trackedPeers, pID) delete(p.disconnectedPeers, pID) + p.onBlockedPeer(PIDToAddrInfo(pID)) - } +} ``` +(_Code Snippet 1.c: Example of required changes to: go-header/p2p/peer_tracker.go_) + The `PIDToAddrInfo` converts a `peer.ID` to a `peer.AddrInfo` by retrieving such info from the host's peer store (_`host.Peerstore().PeerInfo(peerId)`_). The `peerTracker`'s constructor should be updated to accept these new event handlers: @@ -166,6 +172,8 @@ The `peerTracker`'s constructor should be updated to accept these new event hand ) *peerTracker { ``` +(_Code Snippet 1.d: Example of required changes to: go-header/p2p/peer_tracker.go_) + as well as the `libhead.Exchange`'s options and construction: ```diff @@ -184,9 +192,11 @@ as well as the `libhead.Exchange`'s options and construction: + + OnUpdatedPeers func([]peer.AddrInfo) + OnBlockedPeer func(peer.AddrInfo) - } +} ``` +(_Code Snippet 2.a: Example of required changes to: go-header/p2p/options.go_) + ```diff func NewExchange[H header.Header]( peers peer.IDSlice, @@ -219,11 +229,12 @@ as well as the `libhead.Exchange`'s options and construction: return shufflePeers(peers) } return ex, nil - } +} ``` +(_Code Snippet 2.b: Example of required changes to: go-header/p2p/exchange.go_) + And then define `libhead.Exchange` options to set the event handlers on the `peerTracker`: -_**Example of required changes to: go-header/p2p/go-header/p2p/options.go**_ ```go func WithOnUpdatedPeers(f func([]]peer.ID)) Option[ClientParameters] { @@ -245,6 +256,8 @@ func WithOnBlockedPeer(f func([]]peer.ID)) Option[ClientParameters] { } ``` +(_Code Snippet 2.c: Example of required changes to: go-header/p2p/go-header/p2p/options.go_) + The event handlers to be supplied are callbacks that either: 1. Put the new peer list into the peer store @@ -259,8 +272,9 @@ func newPeerStore(cfg Config) func(ds datastore.Datastore) (PeerStore, error) { } ``` +(_Code Snippet 3.a: Example of required changes to: celestia-node/nodebuilder/p2p/misc.go_) + ```diff -+++ celestia-node/nodebuilder/header/module.go + fx.Provide(newPeerStore), fx.Provide(newHeaderService) @@ -272,8 +286,13 @@ func newPeerStore(cfg Config) func(ds datastore.Datastore) (PeerStore, error) { ... p2p.WithChainID(network.String()), + p2p.WithOnUpdatedPeers(func(peers []peer.ID) { -+ topPeers := getTopPeers(peers, cfg.BootstrappersLimit) -+ peerStore.Put(datastore.NewKey("topPeers"), topPeers) ++ var topTen []peer.ID ++ if len(peers) >= 10 { ++ topTen = peers[:9] ++ } else { ++ topTen = peers ++ } ++ peerStore.Put(topTen) + }), } }, @@ -281,14 +300,14 @@ func newPeerStore(cfg Config) func(ds datastore.Datastore) (PeerStore, error) { fx.Provide(newP2PExchange(*cfg)), ``` -*:_for requests are that are performed through `session.go`_ +(_Code Snippet 3.b: Example of required changes to celestia-node/nodebuilder/header/module.go_) + +As explained for _Code Snippet 1.b_ the event handler can handle how to deal with the ordered peer list, in this, we are portraying an example of situation where we are always interested in the top 10 peers if 10 or more peers are available. ### Node Startup When the node starts up, it will first check if the `peers` database exists. If it does not, the node will bootstrap from the hardcoded bootstrappers. If it does, the node will bootstrap from the peers in the database. -_**Example of required changes to: celestia-node/nodebuilder/header/constructors.go**_ - ```diff // newP2PExchange constructs a new Exchange for headers. func newP2PExchange(cfg Config) func( @@ -339,11 +358,13 @@ _**Example of required changes to: celestia-node/nodebuilder/header/constructors } ``` +(_Code Snippet 4.a: Example of required changes to: celestia-node/nodebuilder/header/constructors.go_) +
## Approach B: Periodic sampling of peers from the peerstore for "good peers" selection using liveness buckets -In this approach, we will periodically sample peers from the peerstore and select the "good peers", with the "good peers" being the ones that have been connected to the longest possible amount of time. +In this approach, we will periodically sample peers from the host's `Peerstore` and select the "good peers", with the "good peers" being the ones that have been connected to the longest possible period of time. Taking inspiration from [this proposition](https://github.com/libp2p/go-libp2p-kad-dht/issues/254#issuecomment-633806123) we suggest to look for peers that have been continuously online, such that, for the given buckets of connection durations: @@ -352,9 +373,9 @@ Taking inspiration from [this proposition](https://github.com/libp2p/go-libp2p-k * < 1 week ago * < 1 month ago. -The "good enough peers" to select should be present in most recent buckets, and should be peers we've seen often. For example, if a peer has been connected to the node for 1 day, then it should figure up in the buckets: "< 1 hour" and , "< 1 day", hence making it a "good enough peer" to select for persistence. +the "good enough peers" to select should be present in most recent buckets, and should be peers we've seen often. For example, if a peer has been connected to the node for 1 day, then it should figure up in the buckets: "< 1 hour" and , "< 1 day", hence making it a "good enough peer" to select for persistence. -The bucketing logic that triages peers into the mentioned buckets is executed periodically, with a new list of "good enough peers" being selected and persisted also periodically, and at the moment of node shutdown. +The bucketing logic that triages peers into the mentioned buckets is executed periodically, with a new list of "good enough peers" being selected and persisted at that same time, and also at the moment of node shutdown. ## Implementation @@ -374,9 +395,9 @@ A list of 1 timestamp means it's been seen once, and if the timestamp was 3 hour Depending on how many peers the node will be connected to, this could be the best peer it can get, or the worst. A better example of what a very good peer would be is: -A peer with list of timestamps whose length is equal to 10, and the oldest timestamp was a week ago. This will make the peer fall into "< 1 week ago" bucket. +A peer that has a list of timestamps whose length is equal to 10, and the oldest timestamp was a week ago and the newest from an hour ago. This will make the peer fall into "< 1 week ago" bucket but also all the other recent buckets. -We suggest to also turn the "amount of times seen" into buckets, such that we sorted peers into buckets of "how many times" they were seen, modifying the selection criteria for good peers to become: +We suggest to also turn the "amount of times seen" into buckets, such that we sort peers into buckets of "how many times" they were seen, modifying the selection criteria for good peers to become: * Peers that fall in most recent buckets + Peers that fall in highest seen count buckets @@ -399,6 +420,7 @@ The implementer of this interface should rely on golang maps as the native abstr type bucketer struct { timeBuckets map[BucketDuration][]peer.ID seenBuckets map[int][]peer.ID + peersSeen map[peer.ID][]time.Time } ``` @@ -417,13 +439,56 @@ The `AddPeer` or `AddPeers`, and the `Sort` methods are intended to be called fr `fetchSortPersistPeers` will also be called on node shutdown. +`fetchSortPersistPeers` will be as follows: + +```go +func fetchSortPersistPeers(bcktr Bucketer, store PeerStore, host host.Host) { + goodPeers := make([]peer.PeerInfo, 0) + peers := host.Peerstore().PeersWithAddrs() + bcktr.AddPeers(peers) + bcktr.Sort() + bestPeers = bcktr.BestPeers() + for _, bestpeer := range bestPeers { + goodPeers = append(goodPeers, host.Peerstore().PeerInfo(bestpeer)) + } + store.Put(goodPeers) +} +``` + ### Node Startup -In node startup, we propose to use the same mechanism that is proposed in approach A. +In node startup, we propose to use the same mechanism that is proposed in approach A, but also add changes to register the `fetchSortPersistPeers` routine to trigger periodically every other `filterBootstrappersInterval`. + +```diff + baseComponents = fx.Options( + ... + fx.Provide(newModule), ++ fx.Invoke(func (ctx context.Context, lc fx.Lifecycle, cfg Config, bcktr Bucketer, pstore PeerStore, host host.Host) { ++ ch := make(chan struct{}) ++ ticker := time.NewTicker(cfg.filterBootstrappersInterval * time.Millisecond) ++ go func() { ++ select { ++ case <-ctx.Done(): ++ case <-ch: ++ case <-ticker.C: ++ fetchSortPersistPeers(bcktr, pstore, host) ++ } ++ ++ lc.Append(fx.Hook{ ++ onStop: func(ctx context.Context) error) { ++ ch <- struct{}{} ++ }, ++ }) ++ }) +), + +``` + +(_Code Snippet 5.a: Example of required changes to celestia-node/nodebuilder/p2p/module.go_) ## Approach C: Periodic sampling of peers from the peerstore for randomized "good peers" selection -In this approach, we will periodically sample peers from the peer store and select random peers from the peer store, similar to approach B, only without the bucketing logic. We suggest to do away with the bucketing logic as it will introduce maintenance costs, and will likely end up in go-libp2p-kad-dht sooner than we anticipate. +In this approach, we will periodically sample peers from the hosts's `Peerstore` and select random peers from the peer store, similar to approach B, only without the bucketing logic. We suggest to do away with the bucketing logic as it will introduce maintenance costs, and will likely end up in go-libp2p-kad-dht sooner than we anticipate. Thus, approach C consist basically of approach B minus the bucketing logic. @@ -443,7 +508,21 @@ and on node shutdown, this routine should: 2.a If the fetched peer list's length is less than the required number, we simply select all available peers, as there wouldn't be enough peers to perform randomized selection 3. Upsert the new list in place of the old one. -This will result in periodic updates of the stored list of "good peers" on disk. The same should happen on node shutdown. +```go +func fetchPersistPeers(store PeerStore, host host.Host) { + goodPeers := make([]peer.PeerInfo, 0) + peers := host.Peerstore().PeersWithAddrs() + bestPeers := getRandomPeers(peers) + for _, bestpeer := range bestPeers { + goodPeers = append(goodPeers, host.Peerstore().PeerInfo(bestpeer)) + } + store.Put(goodPeers) +} +``` + +_The selection of random peers is done through a `getRandomPeers` function that returns a slice of randomly selected peers from an original slice_ + +This will result in periodic updates to the stored list of "good peers" on disk. The same should happen on node shutdown. If the node list does not change, then we will be upserting the same list again which should cause no problems. From 21cd75231931f46e7a2d3bc251a1c0c7afb537d9 Mon Sep 17 00:00:00 2001 From: hrt/derrandz Date: Wed, 12 Apr 2023 23:16:18 +0000 Subject: [PATCH 11/16] fix(language): ryan correct language Co-authored-by: Ryan --- docs/adr/adr-014-bootstrap-from-previous-peers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/adr/adr-014-bootstrap-from-previous-peers.md b/docs/adr/adr-014-bootstrap-from-previous-peers.md index 068360c9ec..ac736131df 100644 --- a/docs/adr/adr-014-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-014-bootstrap-from-previous-peers.md @@ -79,7 +79,7 @@ To periodically select the _"good peers"_ that we're connected to and store them We intend to use this internal logic to periodically select peers that are "good" and store them in the peer store mentioned in the storage section. To do this, we will "hook" into the internals of `peerTracker` and `libhead.Exchange` by defining new "event handlers" intended to handle two main events from `peerTracker`: -1. `OnUpdatedPeers`: _This event is triggered on every garbage collection cycle from `peerTracker`. Before the triggering of this event, the list would have underwent changes from other routines as well and thus the most final version will be available at the time of triggering this event ._ +1. `OnUpdatedPeers`: _This event is triggered on every garbage collection cycle from `peerTracker`. The list will have undergone changes from the other routines before this event is triggered._ 2. `OnBlockedPeer`: _This event is triggered every time the `blockPeer` action is called_ ```diff From 93c76ffa5117e1f34886e7a43b2dc4897b4dd6de Mon Sep 17 00:00:00 2001 From: derrandz Date: Wed, 12 Apr 2023 23:48:54 +0000 Subject: [PATCH 12/16] doc: address review comments --- .../adr-014-bootstrap-from-previous-peers.md | 97 ++++++------------- 1 file changed, 27 insertions(+), 70 deletions(-) diff --git a/docs/adr/adr-014-bootstrap-from-previous-peers.md b/docs/adr/adr-014-bootstrap-from-previous-peers.md index ac736131df..08b63d213b 100644 --- a/docs/adr/adr-014-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-014-bootstrap-from-previous-peers.md @@ -1,3 +1,4 @@ + # ADR 014: Bootstrap from previous peers ## Changelog @@ -127,7 +128,7 @@ func (p *peerTracker) gc() { (_Code Snippet 1.b: Example of required changes to: go-header/p2p/peer_tracker.go_) -In _Code Snippet 1.b_, we can choose to order the slice by `peer.peerScore` to allow the event handler to know which peers are best in connectivity, without leaking the `peerStat` type into the event handler callbacks. +In _Code Snippet 1.b_, we can choose to order the slice by `peer.peerScore` to allow the event handler to pick the top 10 peers (_for example_) in connectivity, without leaking the `peerStat` type into the event handler callbacks. where as `OnBlockedPeer` will be called whenever `peerTracker` blocks a peer through its method `blockPeer`. @@ -234,30 +235,6 @@ as well as the `libhead.Exchange`'s options and construction: (_Code Snippet 2.b: Example of required changes to: go-header/p2p/exchange.go_) -And then define `libhead.Exchange` options to set the event handlers on the `peerTracker`: - -```go -func WithOnUpdatedPeers(f func([]]peer.ID)) Option[ClientParameters] { - return func(p *ClientParameters) { - switch t := any(p).(type) { //nolint:gocritic - case *PeerTrackerParameters: - p.OnUpdatedPeers = f - } - } -} - -func WithOnBlockedPeer(f func([]]peer.ID)) Option[ClientParameters] { - return func(p *ClientParameters) { - switch t := any(p).(type) { //nolint:gocritic - case *PeerTrackerParameters: - p.OnBlockedPeer = f - } - } -} -``` - -(_Code Snippet 2.c: Example of required changes to: go-header/p2p/go-header/p2p/options.go_) - The event handlers to be supplied are callbacks that either: 1. Put the new peer list into the peer store @@ -265,21 +242,10 @@ The event handlers to be supplied are callbacks that either: Such callbacks are easily passable as options at header module construction time, example: -```go -// newP2PExchange constructs a new Exchange for headers. -func newPeerStore(cfg Config) func(ds datastore.Datastore) (PeerStore, error) { - return PeerStore(namespace.Wrap(ds, datastore.NewKey("peer_list"))) -} -``` - -(_Code Snippet 3.a: Example of required changes to: celestia-node/nodebuilder/p2p/misc.go_) - ```diff + fx.Provide(newPeerStore), fx.Provide(newHeaderService) - ... - - func(cfg Config, network modp2p.Network) []p2p.Option[p2p.ClientParameters] { + func(cfg Config, network modp2p.Network, peerStore datastore.Datastore) []p2p.Option[p2p.ClientParameters] { return []p2p.Option[p2p.ClientParameters]{ @@ -294,6 +260,9 @@ func newPeerStore(cfg Config) func(ds datastore.Datastore) (PeerStore, error) { + } + peerStore.Put(topTen) + }), ++ p2p.WithOnBlockedPeers(func(p peer.ID { ++ peerSTore.Delete(p) ++ }) } }, ), @@ -312,19 +281,15 @@ When the node starts up, it will first check if the `peers` database exists. If // newP2PExchange constructs a new Exchange for headers. func newP2PExchange(cfg Config) func( fx.Lifecycle, - modp2p.Bootstrappers, - modp2p.Network, - host.Host, - *conngater.BasicConnectionGater, + ... + ... []p2p.Option[p2p.ClientParameters], + pstore peerstore.Peerstore, ) (libhead.Exchange[*header.ExtendedHeader], error) { return func( lc fx.Lifecycle, - bpeers modp2p.Bootstrappers, - network modp2p.Network, - host host.Host, - conngater *conngater.BasicConnectionGater, + ... + ..., opts []p2p.Option[p2p.ClientParameters], ) (libhead.Exchange[*header.ExtendedHeader], error) { peers, err := cfg.trustedPeers(bpeers) @@ -342,17 +307,7 @@ When the node starts up, it will first check if the `peers` database exists. If host.Peerstore().AddAddrs(peer.ID, peer.Addrs, peerstore.PermanentAddrTTL) } exchange, err := p2p.NewExchange[*header.ExtendedHeader](host, ids, conngater, opts...) - if err != nil { - return nil, err - } - lc.Append(fx.Hook{ - OnStart: func(ctx context.Context) error { - return exchange.Start(ctx) - }, - OnStop: func(ctx context.Context) error { - return exchange.Stop(ctx) - }, - }) + ... return exchange, nil } } @@ -383,6 +338,8 @@ The bucketing logic that triages peers into the mentioned buckets is executed pe We suggest to use the same storage mechanism proposed in approach A. +Note: In terms of write frequency, we intend for the periodicity of writes to be configurable. + ### Peer Selection To achieve bucket-powered peer selection, we propose to create a standalone cache like component to manage buckets, such that it tracks peers by their IDs (_`peer.ID`_) and puts them in the correct bucket every time a `Sort` method is called on the component. The component, thus, maps each peer ID to a list of timestamps, each timestamp marking when did the bucketing component see this peer. @@ -463,22 +420,22 @@ In node startup, we propose to use the same mechanism that is proposed in approa baseComponents = fx.Options( ... fx.Provide(newModule), -+ fx.Invoke(func (ctx context.Context, lc fx.Lifecycle, cfg Config, bcktr Bucketer, pstore PeerStore, host host.Host) { -+ ch := make(chan struct{}) -+ ticker := time.NewTicker(cfg.filterBootstrappersInterval * time.Millisecond) -+ go func() { -+ select { -+ case <-ctx.Done(): -+ case <-ch: -+ case <-ticker.C: -+ fetchSortPersistPeers(bcktr, pstore, host) -+ } ++ fx.Invoke(func (ctx context.Context, lc fx.Lifecycle, cfg Config, bcktr Bucketer, pstore PeerStore, host host.Host) { ++ ch := make(chan struct{}) ++ ticker := time.NewTicker(cfg.filterBootstrappersInterval * time.Millisecond) ++ go func() { ++ select { ++ case <-ctx.Done(): ++ case <-ch: ++ case <-ticker.C: ++ fetchSortPersistPeers(bcktr, pstore, host) ++ } + -+ lc.Append(fx.Hook{ -+ onStop: func(ctx context.Context) error) { -+ ch <- struct{}{} -+ }, -+ }) ++ lc.Append(fx.Hook{ ++ onStop: func(ctx context.Context) error) { ++ ch <- struct{}{} ++ }, ++ }) + }) ), From 670fe646a919949f5b94b35842c87ba4942bf19d Mon Sep 17 00:00:00 2001 From: derrandz Date: Fri, 14 Apr 2023 15:14:42 +0000 Subject: [PATCH 13/16] doc: document design --- .../adr-014-bootstrap-from-previous-peers.md | 58 +++---------------- 1 file changed, 9 insertions(+), 49 deletions(-) diff --git a/docs/adr/adr-014-bootstrap-from-previous-peers.md b/docs/adr/adr-014-bootstrap-from-previous-peers.md index 08b63d213b..6d02648f21 100644 --- a/docs/adr/adr-014-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-014-bootstrap-from-previous-peers.md @@ -1,4 +1,3 @@ - # ADR 014: Bootstrap from previous peers ## Changelog @@ -7,6 +6,7 @@ * 2023-05-04: update peer selection with new simpler solution + fix language * 2023-10-04: propose a second approach * 2023-11-04: propose a third approach +* 2023-14-04: document decision ## Context @@ -26,7 +26,11 @@ In this ADR, we wish to aleviate this problem by allowing nodes to bootstrap fro ## Decision -Periodically store the addresses of previously seen peers in a key-value database, and allow the node to bootstrap from these peers on start-up. +Periodically store the addresses of previously seen peers in a key-value database, and allow the node to bootstrap from these peers on start-up, by following the methodology of: + +* Approach A. + +Caveats: In regards to the storage section of approach A, we decided to factorize the store implementation under `das/store.go` to be used by both approach A and the checkpoint store in `daser`. ## Design @@ -57,13 +61,9 @@ type PeerStore interface { Put([]peer.AddrInfo) error // Get returns a peer from the store. Load() ([]peer.AddrInfo, error) - // Delete removes a peer from the store. - Delete(peer.ID) error } ``` -\*: _Delete will be used in in the special case where we store a peer and then it gets blocked by `peerTracker`_ - And we expect the underlying implementation to use a badgerDB datastore. Example: ```go @@ -78,10 +78,9 @@ To periodically select the _"good peers"_ that we're connected to and store them `peerTracker` has internal logic that continuiously tracks and garbage collects peers based on whether they connected to the node, disconnected from the node, or responded to the node while taking into consideration response speed. All connected peers are kept track of in a list, and disconnected peers are marked for removal, all as a part of a garbage collection routine. `peerTracker` also blocks peers that behave maliciously, but not in a routine, instead, it does so on `blockPeer` call. -We intend to use this internal logic to periodically select peers that are "good" and store them in the peer store mentioned in the storage section. To do this, we will "hook" into the internals of `peerTracker` and `libhead.Exchange` by defining new "event handlers" intended to handle two main events from `peerTracker`: +We intend to use this internal logic to periodically select peers that are "good" and store them in the peer store mentioned in the storage section. To do this, we will "hook" into the internals of `peerTracker` and `libhead.Exchange` by defining new "event handlers" intended to handle the following events from `peerTracker`: 1. `OnUpdatedPeers`: _This event is triggered on every garbage collection cycle from `peerTracker`. The list will have undergone changes from the other routines before this event is triggered._ -2. `OnBlockedPeer`: _This event is triggered every time the `blockPeer` action is called_ ```diff type peerTracker struct { @@ -89,7 +88,6 @@ type peerTracker struct { disconnectedPeers map[peer.ID]*peerStat + onUpdatedPeers func([]peer.AddrInfo) -+ onBlockedPeer func(peer.AddrInfo) + ctx context.Context cancel context.CancelFunc @@ -130,46 +128,13 @@ func (p *peerTracker) gc() { In _Code Snippet 1.b_, we can choose to order the slice by `peer.peerScore` to allow the event handler to pick the top 10 peers (_for example_) in connectivity, without leaking the `peerStat` type into the event handler callbacks. -where as `OnBlockedPeer` will be called whenever `peerTracker` blocks a peer through its method `blockPeer`. - -```diff - // blockPeer blocks a peer on the networking level and removes it from the local cache. - func (p *peerTracker) blockPeer(pID peer.ID, reason error) { - // add peer to the blacklist, so we can't connect to it in the future. - err := p.connGater.BlockPeer(pID) - if err != nil { - log.Errorw("header/p2p: blocking peer failed", "pID", pID, "err", err) - } - // close connections to peer. - err = p.host.Network().ClosePeer(pID) - if err != nil { - log.Errorw("header/p2p: closing connection with peer failed", "pID", pID, "err", err) - } - - log.Warnw("header/p2p: blocked peer", "pID", pID, "reason", reason) - - p.peerLk.Lock() - defer p.peerLk.Unlock() - // remove peer from cache. - delete(p.trackedPeers, pID) - delete(p.disconnectedPeers, pID) -+ p.onBlockedPeer(PIDToAddrInfo(pID)) -} -``` - -(_Code Snippet 1.c: Example of required changes to: go-header/p2p/peer_tracker.go_) - -The `PIDToAddrInfo` converts a `peer.ID` to a `peer.AddrInfo` by retrieving such info from the host's peer store (_`host.Peerstore().PeerInfo(peerId)`_). - The `peerTracker`'s constructor should be updated to accept these new event handlers: ```diff - type peerTracker struct { - func newPeerTracker( +func newPeerTracker( h host.Host, connGater *conngater.BasicConnectionGater, + onUpdatedPeers func([]peer.ID), -+ onBlockedPeer func(peer.ID), ) *peerTracker { ``` @@ -192,7 +157,6 @@ as well as the `libhead.Exchange`'s options and construction: chainID string + + OnUpdatedPeers func([]peer.AddrInfo) -+ OnBlockedPeer func(peer.AddrInfo) } ``` @@ -221,7 +185,6 @@ as well as the `libhead.Exchange`'s options and construction: host, connGater, + params.OnUpdatedPeers -+ params.OnBlockedPeer, ), Params: params, } @@ -260,9 +223,6 @@ Such callbacks are easily passable as options at header module construction time + } + peerStore.Put(topTen) + }), -+ p2p.WithOnBlockedPeers(func(p peer.ID { -+ peerSTore.Delete(p) -+ }) } }, ), @@ -501,7 +461,7 @@ Both suggested approaches, A and B, do not conflict with using an on-disk peer s ## Status -Proposed +Accepted ## Consequences From b6b49cb5f6b24b2b517521c802f311d6a8acedee Mon Sep 17 00:00:00 2001 From: derrandz Date: Fri, 14 Apr 2023 15:37:45 +0000 Subject: [PATCH 14/16] doc: address ryan's comments --- .../adr-014-bootstrap-from-previous-peers.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/adr/adr-014-bootstrap-from-previous-peers.md b/docs/adr/adr-014-bootstrap-from-previous-peers.md index 6d02648f21..b9e8278eae 100644 --- a/docs/adr/adr-014-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-014-bootstrap-from-previous-peers.md @@ -80,14 +80,14 @@ To periodically select the _"good peers"_ that we're connected to and store them We intend to use this internal logic to periodically select peers that are "good" and store them in the peer store mentioned in the storage section. To do this, we will "hook" into the internals of `peerTracker` and `libhead.Exchange` by defining new "event handlers" intended to handle the following events from `peerTracker`: -1. `OnUpdatedPeers`: _This event is triggered on every garbage collection cycle from `peerTracker`. The list will have undergone changes from the other routines before this event is triggered._ +1. `OnPeersGC`: _This event is triggered on every garbage collection cycle from `peerTracker`. The list will have undergone changes from the other routines before this event is triggered._ ```diff type peerTracker struct { // online until pruneDeadline, it will be removed and its score will be lost. disconnectedPeers map[peer.ID]*peerStat -+ onUpdatedPeers func([]peer.AddrInfo) ++ onPeersGC func([]peer.AddrInfo) + ctx context.Context cancel context.CancelFunc @@ -96,7 +96,7 @@ type peerTracker struct { (_Code Snippet 1.a: Example of required changes to: go-header/p2p/peer_tracker.go_) -as explained above, `OnUpdatedPeers` will be called whenever `peerTracker` updates its internal list of peers on its `gc` routine, which updates every 30 minutes (_value is configurable_): +as explained above, `OnPeersGC` will be called whenever `peerTracker` updates its internal list of peers on its `gc` routine, which updates every 30 minutes (_value is configurable_): ```diff func (p *peerTracker) gc() { @@ -118,7 +118,7 @@ func (p *peerTracker) gc() { + p.peerLk.Unlock() + -+ p.onUpdatedPeers(updatedPeerList) ++ p.onPeersGC(updatedPeerList) } } } @@ -134,7 +134,7 @@ The `peerTracker`'s constructor should be updated to accept these new event hand func newPeerTracker( h host.Host, connGater *conngater.BasicConnectionGater, -+ onUpdatedPeers func([]peer.ID), ++ onPeersGC func([]peer.ID), ) *peerTracker { ``` @@ -156,7 +156,7 @@ as well as the `libhead.Exchange`'s options and construction: // chainID is an identifier of the chain. chainID string + -+ OnUpdatedPeers func([]peer.AddrInfo) ++ OnPeersGC func([]peer.AddrInfo) } ``` @@ -184,7 +184,7 @@ as well as the `libhead.Exchange`'s options and construction: peerTracker: newPeerTracker( host, connGater, -+ params.OnUpdatedPeers ++ params.OnPeersGC ), Params: params, } @@ -214,7 +214,7 @@ Such callbacks are easily passable as options at header module construction time return []p2p.Option[p2p.ClientParameters]{ ... p2p.WithChainID(network.String()), -+ p2p.WithOnUpdatedPeers(func(peers []peer.ID) { ++ p2p.WithOnPeersGC(func(peers []peer.ID) { + var topTen []peer.ID + if len(peers) >= 10 { + topTen = peers[:9] @@ -288,7 +288,7 @@ Taking inspiration from [this proposition](https://github.com/libp2p/go-libp2p-k * < 1 week ago * < 1 month ago. -the "good enough peers" to select should be present in most recent buckets, and should be peers we've seen often. For example, if a peer has been connected to the node for 1 day, then it should figure up in the buckets: "< 1 hour" and , "< 1 day", hence making it a "good enough peer" to select for persistence. +the "good enough peers" to select should be present in most recent buckets, and should be peers we've seen often. For example, if a peer has been connected to the node for 1 day, then it should show up in the buckets: "< 1 hour" and , "< 1 day", hence making it a "good enough peer" to select for persistence. The bucketing logic that triages peers into the mentioned buckets is executed periodically, with a new list of "good enough peers" being selected and persisted at that same time, and also at the moment of node shutdown. From c8cd4da6dca99f47ab3691f5f1d60e78f89c3a41 Mon Sep 17 00:00:00 2001 From: derrandz Date: Tue, 25 Apr 2023 18:01:12 +0100 Subject: [PATCH 15/16] doc: update approach A to resolve security concerns --- .../adr-014-bootstrap-from-previous-peers.md | 144 +++++++++--------- 1 file changed, 70 insertions(+), 74 deletions(-) diff --git a/docs/adr/adr-014-bootstrap-from-previous-peers.md b/docs/adr/adr-014-bootstrap-from-previous-peers.md index b9e8278eae..ac41b7f12c 100644 --- a/docs/adr/adr-014-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-014-bootstrap-from-previous-peers.md @@ -7,6 +7,7 @@ * 2023-10-04: propose a second approach * 2023-11-04: propose a third approach * 2023-14-04: document decision +* 2023-25-04: update approach A to resolve security concerns ## Context @@ -20,7 +21,7 @@ Currently, when a node joins/comes up on the network, it relies, by default, on This is a centralization bottleneck as it happens both during initial start-up and for any re-starts. -In this ADR, we wish to aleviate this problem by allowing nodes to bootstrap from previously seen peers. This is a simple solution that allows the network alleviate the start-up centralisation bottleneck such that nodes can join the network without the need for hardcoded bootstrappers up re-start +In this ADR, we wish to aleviate this problem by allowing nodes to bootstrap from previously seen peers. This is a simple solution that allows the network alleviate the start-up centralisation bottleneck such that nodes can join the network without the need for hardcoded bootstrappers being up on re-start (_Original issue available in references_) @@ -30,15 +31,13 @@ Periodically store the addresses of previously seen peers in a key-value databas * Approach A. -Caveats: In regards to the storage section of approach A, we decided to factorize the store implementation under `das/store.go` to be used by both approach A and the checkpoint store in `daser`. - ## Design ## Approach A: Hook into `peerTracker` from `libhead.Exchange` and react to its internal events
-In this approach, we will rely on `peerTracker`'s scoring mechanism that tracks the fastest connected nodes, and blocks malicious ones. To make use of this functionality, we track two important internal events from `peerTracker` in `libhead.Exchange` for peers selection and persistence purposes, and that is by proposing a design that allows access to `peerTracker`'s list of good peers and bad peers, and opening the design space to integrate peer selection and storage logic. +In this approach, we will rely on `peerTracker`'s scoring mechanism that tracks the fastest connected nodes, and blocks malicious ones. To make use of this functionality, we rely on the internal mechanisms of `peerTracker` for peers selection and persistence, and that is by proposing a design that allows storing `peerTracker`'s list of good peers, and opening the design space to integrate peer selection and storage logic. ## Implementation @@ -51,7 +50,7 @@ In this approach, we will rely on `peerTracker`'s scoring mechanism that tracks ### Storage -We will use a badgerDB datastore to store the addresses of previously seen good peers. The database will be stored in the `data` directory, and will have a store prefix `good_peers`. The database will have a single key-value pair, where the key is `peers` and the value is a `[]PeerInfo` +We will use a badgerDB datastore to store the addresses of previously seen good peers. The database will be stored in the `data` directory, and will have a store prefix `good_peers`. The database will have a single key-value pair, where the key is `peers` and the value is a `[]AddrInfo` The peer store will implement the following interface: @@ -72,22 +71,27 @@ type peerStore struct { } ``` +Ultimately, we will use the same pattern that is used in the store implementation under `das/store.go` for the peer store. + ### Peer Selection & Persistence -To periodically select the _"good peers"_ that we're connected to and store them in the peer store, we will rely on the internals of `go-header`, specifically the `peerTracker` and `libhead.Exchange` structs. +To periodically select the _"good peers"_ that we're connected to and store them a in "peer store", we will rely on the internals of `go-header`, specifically the `peerTracker` and `libhead.Exchange` structs. -`peerTracker` has internal logic that continuiously tracks and garbage collects peers based on whether they connected to the node, disconnected from the node, or responded to the node while taking into consideration response speed. All connected peers are kept track of in a list, and disconnected peers are marked for removal, all as a part of a garbage collection routine. `peerTracker` also blocks peers that behave maliciously, but not in a routine, instead, it does so on `blockPeer` call. +`peerTracker` has internal logic that continuously tracks and garbage collects peers based on whether they've connected to the node, disconnected from the node, or responded to the node while taking into consideration response speed. All connected peers are kept track of in a list, and disconnected peers are marked for removal, all as a part of a garbage collection routine. `peerTracker` also blocks peers that behave maliciously, but not in a routine, instead, it does so on `blockPeer` call. Ultimately, the blocked peers will be first disconnected from before the block happens, thus the garbage collection routine will take care of cleaning them up. -We intend to use this internal logic to periodically select peers that are "good" and store them in the peer store mentioned in the storage section. To do this, we will "hook" into the internals of `peerTracker` and `libhead.Exchange` by defining new "event handlers" intended to handle the following events from `peerTracker`: +We intend to use this internal logic to periodically select peers that are "good" and store them in the peer store mentioned in the storage section. To do this, we will perform the storing of the "good peers" every time `peerTracker` performs its internal garbage collection, since that's when `peerTracker` updates its peer list by removing peers marked for removal and cleans peers with low scores. (_i.e: the "bad peers" are filtered_) -1. `OnPeersGC`: _This event is triggered on every garbage collection cycle from `peerTracker`. The list will have undergone changes from the other routines before this event is triggered._ +To enable this, we suggest that: + +1.`peerTracker` to have a reference of the `Peerstore` +2.`peerTracker` to directly call the `Peerstore`'s methods for storage ```diff type peerTracker struct { // online until pruneDeadline, it will be removed and its score will be lost. disconnectedPeers map[peer.ID]*peerStat - -+ onPeersGC func([]peer.AddrInfo) + ++ peerstore Peerstore + ctx context.Context cancel context.CancelFunc @@ -96,7 +100,7 @@ type peerTracker struct { (_Code Snippet 1.a: Example of required changes to: go-header/p2p/peer_tracker.go_) -as explained above, `OnPeersGC` will be called whenever `peerTracker` updates its internal list of peers on its `gc` routine, which updates every 30 minutes (_value is configurable_): +as explained above, we will store the new updated list on every new garbage collection cycle (_the gc interval value is configurable_): ```diff func (p *peerTracker) gc() { @@ -116,9 +120,8 @@ func (p *peerTracker) gc() { + updatedPeerList = append(updatedPeerList, PIDtoAddrInfo(peer.peerID)) + } + ++ p.peerstore.Put(updatedPeerList) p.peerLk.Unlock() -+ -+ p.onPeersGC(updatedPeerList) } } } @@ -126,15 +129,13 @@ func (p *peerTracker) gc() { (_Code Snippet 1.b: Example of required changes to: go-header/p2p/peer_tracker.go_) -In _Code Snippet 1.b_, we can choose to order the slice by `peer.peerScore` to allow the event handler to pick the top 10 peers (_for example_) in connectivity, without leaking the `peerStat` type into the event handler callbacks. - -The `peerTracker`'s constructor should be updated to accept these new event handlers: +The `peerTracker`'s constructor should be updated to accept the peer store: ```diff func newPeerTracker( h host.Host, connGater *conngater.BasicConnectionGater, -+ onPeersGC func([]peer.ID), ++ peerstore Peerstore, ) *peerTracker { ``` @@ -156,7 +157,7 @@ as well as the `libhead.Exchange`'s options and construction: // chainID is an identifier of the chain. chainID string + -+ OnPeersGC func([]peer.AddrInfo) ++ peerstore Peerstore } ``` @@ -184,9 +185,9 @@ as well as the `libhead.Exchange`'s options and construction: peerTracker: newPeerTracker( host, connGater, -+ params.OnPeersGC - ), - Params: params, ++ params.peerstore + ), + Params: params, } ex.trustedPeers = func() peer.IDSlice { @@ -198,12 +199,7 @@ as well as the `libhead.Exchange`'s options and construction: (_Code Snippet 2.b: Example of required changes to: go-header/p2p/exchange.go_) -The event handlers to be supplied are callbacks that either: - -1. Put the new peer list into the peer store -2. Remove a blocked peer from the peer store - -Such callbacks are easily passable as options at header module construction time, example: +To pass the peer store to the `Exchange` (_and subsequently to the `peerTracker`_), we can rely on `WithPeerPersistence` option as follows: ```diff + fx.Provide(newPeerStore), @@ -214,67 +210,67 @@ Such callbacks are easily passable as options at header module construction time return []p2p.Option[p2p.ClientParameters]{ ... p2p.WithChainID(network.String()), -+ p2p.WithOnPeersGC(func(peers []peer.ID) { -+ var topTen []peer.ID -+ if len(peers) >= 10 { -+ topTen = peers[:9] -+ } else { -+ topTen = peers -+ } -+ peerStore.Put(topTen) -+ }), ++ p2p.WithPeerPersistence(peerStore), } }, ), - fx.Provide(newP2PExchange(*cfg)), + fx.Provide(newP2PExchange(*cfg)), ``` (_Code Snippet 3.b: Example of required changes to celestia-node/nodebuilder/header/module.go_) -As explained for _Code Snippet 1.b_ the event handler can handle how to deal with the ordered peer list, in this, we are portraying an example of situation where we are always interested in the top 10 peers if 10 or more peers are available. - -### Node Startup +### Security Concerns: Long Range Attack -When the node starts up, it will first check if the `peers` database exists. If it does not, the node will bootstrap from the hardcoded bootstrappers. If it does, the node will bootstrap from the peers in the database. +With the ability to retrieve previously seen good peers, we should distinguish between those and the trusted peers for cases when we have expired local sync target (_i.e: subjective head_). On such cases, we would like to strictly sync and retrieve the subjective head from our trusted peers and no other, and for other syncing tasks, include the other stored good peers. To achieve this, we need to first include information about the expiry of the local sync target: ```diff + // newP2PExchange constructs a new Exchange for headers. - func newP2PExchange(cfg Config) func( - fx.Lifecycle, - ... + func newP2PExchange( + lc fx.Lifecycle, ++ storeChainHead *header.ExtendedHeader, + bpeers modp2p.Bootstrappers, + network modp2p.Network, + host host.Host, ... - []p2p.Option[p2p.ClientParameters], -+ pstore peerstore.Peerstore, - ) (libhead.Exchange[*header.ExtendedHeader], error) { - return func( - lc fx.Lifecycle, - ... - ..., - opts []p2p.Option[p2p.ClientParameters], - ) (libhead.Exchange[*header.ExtendedHeader], error) { - peers, err := cfg.trustedPeers(bpeers) - if err != nil { - return nil, err - } -+ list, err := pstore.Load() -+ if err != nil { -+ return nil, err -+ } -+ peers := append(peers, list...) - ids := make([]peer.ID, len(peers)) - for index, peer := range peers { - ids[index] = peer.ID - host.Peerstore().AddAddrs(peer.ID, peer.Addrs, peerstore.PermanentAddrTTL) - } - exchange, err := p2p.NewExchange[*header.ExtendedHeader](host, ids, conngater, opts...) - ... - return exchange, nil - } - } + exchange, err := p2p.NewExchange(..., + p2p.WithParams(cfg.Client), + p2p.WithNetworkID[p2p.ClientParameters](network.String()), + p2p.WithChainID(network.String()), ++ p2p.WithSubjectiveInitialization(storeChainHead.IsExpired), + ) + if err != nil { + return nil, err ``` -(_Code Snippet 4.a: Example of required changes to: celestia-node/nodebuilder/header/constructors.go_) +(_Code Snippet 3.c: Example of required changes to celestia-node/nodebuilder/header/constructors.go_) + +This piece information will allow the `Exchange` to distinguish between the two cases and use the correct peer set for each case: + +```diff + func (ex *Exchange[H]) Head(ctx context.Context) (H, error) { + log.Debug("requesting head") ++ // pool of peers to request from is determined by whether the head of the store is expired or not ++ if ex.Params.subjectiveInitialisation { ++ // if the local chain head is outside the unbonding period, only use TRUSTED PEERS for determining the head ++ peerset = ex.trustedPeers ++ } else { ++ // otherwise, node has a chain head that is NOT outdated so we can actually request random peers in addition to trusted ++ peerset = append(ex.trustedPeers, ex.params.peerstore.Get()) ++ } ++ + reqCtx := ctx + if deadline, ok := ctx.Deadline(); ok { + // allocate 90% of caller's set deadline for requests + ... +- for _, from := range trustedPeers { ++ for _, from := range peerset { + go func(from peer.ID) { + headers, err := ex.request(reqCtx, from, headerReq) + if err != nil { +``` +Other side-effect changes will have to be done to methods that use `trustedPeers` restrictively for requests (_such as `performRequest`_) to allow requesting to previously seen good peers as well (_Reference: [Issue #25](https://github.com/celestiaorg/go-header/issues/25)_)
## Approach B: Periodic sampling of peers from the peerstore for "good peers" selection using liveness buckets From bf5d30daf0624905a2b5e9a9a5b9c16108a7689d Mon Sep 17 00:00:00 2001 From: derrandz Date: Thu, 11 May 2023 13:42:26 +0100 Subject: [PATCH 16/16] fix: implement latest requested changes fix: implement latest requested changes --- .../adr-014-bootstrap-from-previous-peers.md | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/docs/adr/adr-014-bootstrap-from-previous-peers.md b/docs/adr/adr-014-bootstrap-from-previous-peers.md index ac41b7f12c..fc0386b450 100644 --- a/docs/adr/adr-014-bootstrap-from-previous-peers.md +++ b/docs/adr/adr-014-bootstrap-from-previous-peers.md @@ -221,37 +221,35 @@ To pass the peer store to the `Exchange` (_and subsequently to the `peerTracker` ### Security Concerns: Long Range Attack -With the ability to retrieve previously seen good peers, we should distinguish between those and the trusted peers for cases when we have expired local sync target (_i.e: subjective head_). On such cases, we would like to strictly sync and retrieve the subjective head from our trusted peers and no other, and for other syncing tasks, include the other stored good peers. To achieve this, we need to first include information about the expiry of the local sync target: +With the ability to retrieve previously seen good peers, we should distinguish between those and the trusted peers for cases when we have local chain head and have to perform subjective initialisation. (_i.e: subjective head_). On such cases, we would like to strictly sync and retrieve the subjective head from our trusted peers and no other, and for other syncing tasks, include the other stored good peers. We also need need to make sure that we have runtime detection for subjective initialization, so to achieve this, we suggest to extend `exchange.Head` with options that allow us to distinguish between the two cases: ```diff + -func (ex *Exchange[H]) Head(ctx context.Context) (H, error) { + +func (ex *Exchange[H]) Head(ctx context.Context, opts ....header.RequestOptions) (H, error) { - // newP2PExchange constructs a new Exchange for headers. - func newP2PExchange( - lc fx.Lifecycle, -+ storeChainHead *header.ExtendedHeader, - bpeers modp2p.Bootstrappers, - network modp2p.Network, - host host.Host, - ... - exchange, err := p2p.NewExchange(..., - p2p.WithParams(cfg.Client), - p2p.WithNetworkID[p2p.ClientParameters](network.String()), - p2p.WithChainID(network.String()), -+ p2p.WithSubjectiveInitialization(storeChainHead.IsExpired), - ) - if err != nil { - return nil, err ``` +And define a `RequestOptions` as follows: + +```diff ++ func WithSubjectiveInitilization(bool) +``` + +such that the `exchange.Head` method could perform subjective initialization against trusted peers when the option is set to `true`. + (_Code Snippet 3.c: Example of required changes to celestia-node/nodebuilder/header/constructors.go_) This piece information will allow the `Exchange` to distinguish between the two cases and use the correct peer set for each case: ```diff - func (ex *Exchange[H]) Head(ctx context.Context) (H, error) { + func (ex *Exchange[H]) Head(ctx context.Context, opts ...header.RequestOptions) (H, error) { log.Debug("requesting head") ++ var req reqOpts ++ for _, opt := range opts { ++ opt(&req) ++ } + // pool of peers to request from is determined by whether the head of the store is expired or not -+ if ex.Params.subjectiveInitialisation { ++ if req.subjectiveInitialisation { + // if the local chain head is outside the unbonding period, only use TRUSTED PEERS for determining the head + peerset = ex.trustedPeers + } else { @@ -270,7 +268,7 @@ This piece information will allow the `Exchange` to distinguish between the two if err != nil { ``` -Other side-effect changes will have to be done to methods that use `trustedPeers` restrictively for requests (_such as `performRequest`_) to allow requesting to previously seen good peers as well (_Reference: [Issue #25](https://github.com/celestiaorg/go-header/issues/25)_) +Other side-effect changes will have to be done to methods that use `trustedPeers` restrictively for requests (_such as `performRequest`_) to allow requesting to use previously seen good peers as well (_Reference: [Issue #25](https://github.com/celestiaorg/go-header/issues/25)_)
## Approach B: Periodic sampling of peers from the peerstore for "good peers" selection using liveness buckets