Skip to content

Commit

Permalink
fix(nodebuilder/share): Pass light.Window to shrex getter construct…
Browse files Browse the repository at this point in the history
…ion regardless of node type / pruning mode (celestiaorg#3466)

Since it is a network constant, shrex getter request routing should be
based on the hardcoded light.Window rather than the avail window that is
passed to it. `light.Window` should be used network-wide to determine
which peer pool (`full` or `archival`) would be most likely to serve the
request.

This also fixes a bug where archival nodes would not route historic EDS
requests to the archival pool due to AvailabilityWindow being 0.
  • Loading branch information
renaynay authored Jun 6, 2024
1 parent 8254438 commit bdac1d7
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
8 changes: 2 additions & 6 deletions nodebuilder/share/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

"github.com/celestiaorg/celestia-node/nodebuilder/node"
modp2p "github.com/celestiaorg/celestia-node/nodebuilder/p2p"
"github.com/celestiaorg/celestia-node/pruner"
lightprune "github.com/celestiaorg/celestia-node/pruner/light"
"github.com/celestiaorg/celestia-node/share"
"github.com/celestiaorg/celestia-node/share/availability/full"
"github.com/celestiaorg/celestia-node/share/availability/light"
Expand Down Expand Up @@ -92,17 +92,13 @@ func shrexComponents(tp node.Type, cfg *Config) fx.Option {
edsClient *shrexeds.Client,
ndClient *shrexnd.Client,
managers map[string]*peers.Manager,
window pruner.AvailabilityWindow,
) *getters.ShrexGetter {
return getters.NewShrexGetter(
edsClient,
ndClient,
managers[fullNodesTag],
managers[archivalNodesTag],
// TODO @renaynay: Pruned FNs should pass `light.Window` to shrex getter
// best route requests (as full.Window serves as a buffer for serving data while
// the request router itself should just stick to light.Window)
window,
lightprune.Window,
)
},
fx.OnStart(func(ctx context.Context, getter *getters.ShrexGetter) error {
Expand Down
18 changes: 18 additions & 0 deletions nodebuilder/tests/prune_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,17 @@ import (
"go.uber.org/fx"

"github.com/celestiaorg/celestia-node/blob"
"github.com/celestiaorg/celestia-node/libs/fxutil"
"github.com/celestiaorg/celestia-node/nodebuilder"
"github.com/celestiaorg/celestia-node/nodebuilder/das"
"github.com/celestiaorg/celestia-node/nodebuilder/node"
"github.com/celestiaorg/celestia-node/nodebuilder/tests/swamp"
"github.com/celestiaorg/celestia-node/pruner"
"github.com/celestiaorg/celestia-node/share"
"github.com/celestiaorg/celestia-node/share/getters"
"github.com/celestiaorg/celestia-node/share/p2p/peers"
"github.com/celestiaorg/celestia-node/share/p2p/shrexeds"
"github.com/celestiaorg/celestia-node/share/p2p/shrexnd"
)

// TestArchivalBlobSync tests whether a LN is able to sync historical blobs from
Expand Down Expand Up @@ -62,6 +67,19 @@ func TestArchivalBlobSync(t *testing.T) {
testAvailWindow := pruner.AvailabilityWindow(time.Millisecond)
prunerOpts := fx.Options(
fx.Replace(testAvailWindow),
fxutil.ReplaceAs(func(
edsClient *shrexeds.Client,
ndClient *shrexnd.Client,
managers map[string]*peers.Manager,
) *getters.ShrexGetter {
return getters.NewShrexGetter(
edsClient,
ndClient,
managers["full"],
managers["archival"],
testAvailWindow,
)
}, new(getters.ShrexGetter)),
)

// stop the archival BN to force LN to have to discover
Expand Down
3 changes: 2 additions & 1 deletion share/getters/shrex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/celestiaorg/celestia-node/header"
"github.com/celestiaorg/celestia-node/header/headertest"
"github.com/celestiaorg/celestia-node/pruner/full"
"github.com/celestiaorg/celestia-node/pruner/light"
"github.com/celestiaorg/celestia-node/share"
"github.com/celestiaorg/celestia-node/share/eds"
"github.com/celestiaorg/celestia-node/share/eds/edstest"
Expand Down Expand Up @@ -59,7 +60,7 @@ func TestShrexGetter(t *testing.T) {
archivalPeerManager, err := testManager(ctx, clHost, sub)
require.NoError(t, err)

getter := NewShrexGetter(edsClient, ndClient, fullPeerManager, archivalPeerManager, full.Window)
getter := NewShrexGetter(edsClient, ndClient, fullPeerManager, archivalPeerManager, light.Window)
require.NoError(t, getter.Start(ctx))

t.Run("ND_Available, total data size > 1mb", func(t *testing.T) {
Expand Down

0 comments on commit bdac1d7

Please sign in to comment.