Skip to content

Commit

Permalink
Make load and save backup peers functions private
Browse files Browse the repository at this point in the history
  • Loading branch information
gammazero committed Sep 9, 2023
1 parent 4ace928 commit 98d4895
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 33 deletions.
31 changes: 21 additions & 10 deletions core/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ type BootstrapConfig struct {
// as backup bootstrap peers.
MaxBackupBootstrapSize int

SaveBackupBootstrapPeers func(context.Context, []peer.AddrInfo)
LoadBackupBootstrapPeers func(context.Context) []peer.AddrInfo
saveBackupBootstrapPeers func(context.Context, []peer.AddrInfo)
loadBackupBootstrapPeers func(context.Context) []peer.AddrInfo
}

// DefaultBootstrapConfig specifies default sane parameters for bootstrapping.
Expand Down Expand Up @@ -91,11 +91,22 @@ func WithBackupPeers(load func(context.Context) []peer.AddrInfo, save func(conte
panic("both load and save backup bootstrap peers functions must be defined")
}
return func(cfg *BootstrapConfig) {
cfg.LoadBackupBootstrapPeers = load
cfg.SaveBackupBootstrapPeers = save
cfg.loadBackupBootstrapPeers = load
cfg.saveBackupBootstrapPeers = save
}
}

// BackupPeers returns the load and save backup peers functions.
func (cfg *BootstrapConfig) BackupPeers() (func(context.Context) []peer.AddrInfo, func(context.Context, []peer.AddrInfo)) {
return cfg.loadBackupBootstrapPeers, cfg.saveBackupBootstrapPeers
}

// SetBackupPeers sets the load and save backup peers functions.
func (cfg *BootstrapConfig) SetBackupPeers(load func(context.Context) []peer.AddrInfo, save func(context.Context, []peer.AddrInfo)) {
opt := WithBackupPeers(load, save)
opt(cfg)
}

// Bootstrap kicks off IpfsNode bootstrapping. This function will periodically
// check the number of open connections and -- if there are too few -- initiate
// connections to well-known bootstrap peers. It also kicks off subsystem
Expand Down Expand Up @@ -140,9 +151,9 @@ func Bootstrap(id peer.ID, host host.Host, rt routing.Routing, cfg BootstrapConf
doneWithRound <- struct{}{}
close(doneWithRound) // it no longer blocks periodic

// If LoadBackupBootstrapPeers is not nil then SaveBackupBootstrapPeers
// If loadBackupBootstrapPeers is not nil then saveBackupBootstrapPeers
// must also not be nil.
if cfg.LoadBackupBootstrapPeers != nil {
if cfg.loadBackupBootstrapPeers != nil {
startSavePeersAsTemporaryBootstrapProc(cfg, host, proc)
}

Expand Down Expand Up @@ -205,7 +216,7 @@ func saveConnectedPeersAsTemporaryBootstrap(ctx context.Context, host host.Host,

// If we didn't reach the target number use previously stored connected peers.
if len(backupPeers) < cfg.MaxBackupBootstrapSize {
oldSavedPeers := cfg.LoadBackupBootstrapPeers(ctx)
oldSavedPeers := cfg.loadBackupBootstrapPeers(ctx)
log.Debugf("missing %d peers to reach backup bootstrap target of %d, trying from previous list of %d saved peers",
cfg.MaxBackupBootstrapSize-len(backupPeers), cfg.MaxBackupBootstrapSize, len(oldSavedPeers))

Expand All @@ -229,7 +240,7 @@ func saveConnectedPeersAsTemporaryBootstrap(ctx context.Context, host host.Host,
}
}

cfg.SaveBackupBootstrapPeers(ctx, backupPeers)
cfg.saveBackupBootstrapPeers(ctx, backupPeers)
log.Debugf("saved %d peers (of %d target) as bootstrap backup in the config", len(backupPeers), cfg.MaxBackupBootstrapSize)
return nil
}
Expand Down Expand Up @@ -261,14 +272,14 @@ func bootstrapRound(ctx context.Context, host host.Host, cfg BootstrapConfig) er
}
}

if cfg.LoadBackupBootstrapPeers == nil {
if cfg.loadBackupBootstrapPeers == nil {
log.Debugf("not enough bootstrap peers to fill the remaining target of %d connections", numToDial)
return ErrNotEnoughBootstrapPeers
}

log.Debugf("not enough bootstrap peers to fill the remaining target of %d connections, trying backup list", numToDial)

tempBootstrapPeers := cfg.LoadBackupBootstrapPeers(ctx)
tempBootstrapPeers := cfg.loadBackupBootstrapPeers(ctx)
if len(tempBootstrapPeers) > 0 {
numToDial -= int(peersConnect(ctx, host, tempBootstrapPeers, numToDial, false))
if numToDial <= 0 {
Expand Down
65 changes: 47 additions & 18 deletions core/bootstrap/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package bootstrap
import (
"context"
"crypto/rand"
"reflect"
"testing"
"time"

Expand Down Expand Up @@ -34,12 +35,19 @@ func TestLoadAndSaveOptions(t *testing.T) {
saveFunc := func(_ context.Context, _ []peer.AddrInfo) {}

bootCfg := BootstrapConfigWithPeers(nil, WithBackupPeers(loadFunc, saveFunc))
if bootCfg.LoadBackupBootstrapPeers == nil {
load, save := bootCfg.BackupPeers()
if load == nil {
t.Fatal("load function not assigned")
}
if bootCfg.SaveBackupBootstrapPeers == nil {
if reflect.ValueOf(load).Pointer() != reflect.ValueOf(loadFunc).Pointer() {
t.Fatal("load not assigned correct function")
}
if save == nil {
t.Fatal("save function not assigned")
}
if reflect.ValueOf(save).Pointer() != reflect.ValueOf(saveFunc).Pointer() {
t.Fatal("save not assigned correct function")
}

assertPanics(t, "with only load func", func() {
BootstrapConfigWithPeers(nil, WithBackupPeers(loadFunc, nil))
Expand All @@ -50,7 +58,43 @@ func TestLoadAndSaveOptions(t *testing.T) {
})

bootCfg = BootstrapConfigWithPeers(nil, WithBackupPeers(nil, nil))
if bootCfg.LoadBackupBootstrapPeers != nil || bootCfg.SaveBackupBootstrapPeers != nil {
load, save = bootCfg.BackupPeers()
if load != nil || save != nil {
t.Fatal("load and save functions should both be nil")
}
}

func TestSetBackupPeers(t *testing.T) {
loadFunc := func(_ context.Context) []peer.AddrInfo { return nil }
saveFunc := func(_ context.Context, _ []peer.AddrInfo) {}

bootCfg := DefaultBootstrapConfig
bootCfg.SetBackupPeers(loadFunc, saveFunc)
load, save := bootCfg.BackupPeers()
if load == nil {
t.Fatal("load function not assigned")
}
if reflect.ValueOf(load).Pointer() != reflect.ValueOf(loadFunc).Pointer() {
t.Fatal("load not assigned correct function")
}
if save == nil {
t.Fatal("save function not assigned")
}
if reflect.ValueOf(save).Pointer() != reflect.ValueOf(saveFunc).Pointer() {
t.Fatal("save not assigned correct function")
}

assertPanics(t, "with only load func", func() {
bootCfg.SetBackupPeers(loadFunc, nil)
})

assertPanics(t, "with only save func", func() {
bootCfg.SetBackupPeers(nil, saveFunc)
})

bootCfg.SetBackupPeers(nil, nil)
load, save = bootCfg.BackupPeers()
if load != nil || save != nil {
t.Fatal("load and save functions should both be nil")
}
}
Expand Down Expand Up @@ -82,21 +126,6 @@ func TestNoTempPeersLoadAndSave(t *testing.T) {
time.Sleep(4 * period)
bootstrapper.Close()

// Test for error is only Load or Save function defined.
bootCfg.LoadBackupBootstrapPeers = func(_ context.Context) []peer.AddrInfo { return nil }

_, err = Bootstrap(peerID, p2pHost, nil, bootCfg)
if err == nil {
t.Fatal("expected error")
}

bootCfg.LoadBackupBootstrapPeers = nil
bootCfg.SaveBackupBootstrapPeers = func(_ context.Context, _ []peer.AddrInfo) {}

_, err = Bootstrap(peerID, p2pHost, nil, bootCfg)
if err == nil {
t.Fatal("expected error")
}
}

func assertPanics(t *testing.T, name string, f func()) {
Expand Down
9 changes: 4 additions & 5 deletions core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,24 +168,23 @@ func (n *IpfsNode) Bootstrap(cfg bootstrap.BootstrapConfig) error {
return ps
}
}
if cfg.SaveBackupBootstrapPeers == nil {
cfg.SaveBackupBootstrapPeers = func(ctx context.Context, peerList []peer.AddrInfo) {
if load, save := cfg.BackupPeers(); load == nil {
save = func(ctx context.Context, peerList []peer.AddrInfo) {
err := n.saveTempBootstrapPeers(ctx, peerList)
if err != nil {
log.Warnf("saveTempBootstrapPeers failed: %s", err)
return
}
}
}
if cfg.LoadBackupBootstrapPeers == nil {
cfg.LoadBackupBootstrapPeers = func(ctx context.Context) []peer.AddrInfo {
load = func(ctx context.Context) []peer.AddrInfo {
peerList, err := n.loadTempBootstrapPeers(ctx)
if err != nil {
log.Warnf("loadTempBootstrapPeers failed: %s", err)
return nil
}
return peerList
}
cfg.SetBackupPeers(load, save)
}

repoConf, err := n.Repo.Config()
Expand Down

0 comments on commit 98d4895

Please sign in to comment.