From bdac1d701c0ef0a048daa7072c29f4bcdc70dda7 Mon Sep 17 00:00:00 2001 From: rene <41963722+renaynay@users.noreply.github.com> Date: Thu, 6 Jun 2024 16:12:57 +0200 Subject: [PATCH] fix(nodebuilder/share): Pass `light.Window` to shrex getter construction regardless of node type / pruning mode (#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. --- nodebuilder/share/module.go | 8 ++------ nodebuilder/tests/prune_test.go | 18 ++++++++++++++++++ share/getters/shrex_test.go | 3 ++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/nodebuilder/share/module.go b/nodebuilder/share/module.go index 06dd5c1d2b..5d0dbbd096 100644 --- a/nodebuilder/share/module.go +++ b/nodebuilder/share/module.go @@ -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" @@ -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 { diff --git a/nodebuilder/tests/prune_test.go b/nodebuilder/tests/prune_test.go index 1bc23b6299..79a20ef1f9 100644 --- a/nodebuilder/tests/prune_test.go +++ b/nodebuilder/tests/prune_test.go @@ -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 @@ -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 diff --git a/share/getters/shrex_test.go b/share/getters/shrex_test.go index 86ec775b4c..a474e1e618 100644 --- a/share/getters/shrex_test.go +++ b/share/getters/shrex_test.go @@ -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" @@ -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) {