fix(network-fabric): propagate ledger info failures instead of rescanning from genesis

Signed-off-by: AkramBitar <akram@il.ibm.com>

Author: AkramBitar

docs/services/network-fabric.md

@@ -294,6 +294,20 @@ sequenceDiagram
 - LRU cache for recent transactions
 - Automatic retry on connection failures
 
+#### Choosing the Starting Block
+
+The scan resumes at the peer's current ledger height, read via `GetLedgerInfo`
+([`delivery.go`](../../token/services/network/fabric/finality/delivery.go)). Because that
+one RPC decides the starting block, a failure is retried with an exponentially growing
+delay (`LedgerInfoAttempts`, `LedgerInfoRetryDelay`; 3 attempts over ~1.5s by default) and
+aborts early if the context is cancelled.
+
+If the height is still unavailable after the last attempt, `ScanBlock` returns the error
+rather than defaulting to block 0. Starting at genesis would rescan the entire chain and
+replay finality notifications for every historical transaction, while the caller would have
+no way to tell a transient RPC failure from a genuinely fresh chain. Block 0 is used only
+when no ledger is configured at all.
+
 ### Notification Mode
 
 Uses asynchronous event notifications from the FSC layer:

token/services/network/fabric/finality/delivery.go

@@ -8,27 +8,113 @@ package finality
 
 import (
 	"context"
+	"time"
 
 	"github.com/LFDT-Panurus/panurus/token/services/logging"
+	"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
 	"github.com/hyperledger-labs/fabric-smart-client/platform/fabric"
 )
 
+const (
+	// defaultLedgerInfoAttempts is how many times ScanBlock asks the ledger for
+	// its current height before giving up. A transient RPC hiccup must not
+	// decide the starting block, so the call is retried rather than trusted once.
+	defaultLedgerInfoAttempts = 3
+	// defaultLedgerInfoRetryDelay is the pause before the first retry. It doubles
+	// on each further attempt, so the defaults span ~1.5s in total.
+	defaultLedgerInfoRetryDelay = 500 * time.Millisecond
+)
+
+// blockFromScanner is the subset of fabric.Delivery used by Delivery: the block
+// scan, parameterized by the block to start from.
+type blockFromScanner interface {
+	ScanBlockFrom(ctx context.Context, block uint64, callback fabric.BlockCallback) error
+}
+
+// ledgerHeightProvider is the subset of fabric.Ledger used by Delivery: the
+// current ledger height, which is where a fresh block scan resumes.
+type ledgerHeightProvider interface {
+	GetLedgerInfo() (*fabric.LedgerInfo, error)
+}
+
 type Delivery struct {
-	*fabric.Delivery
-	*fabric.Ledger
-	Logger logging.Logger
+	Delivery blockFromScanner
+	Ledger   ledgerHeightProvider
+	Logger   logging.Logger
+
+	// LedgerInfoAttempts bounds the number of GetLedgerInfo attempts made by
+	// ScanBlock. Zero or negative selects defaultLedgerInfoAttempts.
+	LedgerInfoAttempts int
+	// LedgerInfoRetryDelay is the pause before the first GetLedgerInfo retry; it
+	// doubles on each further attempt. Zero or negative selects
+	// defaultLedgerInfoRetryDelay.
+	LedgerInfoRetryDelay time.Duration
 }
 
+// ScanBlock scans blocks starting from the current ledger height.
+//
+// The height is read from the ledger, retried a bounded number of times so a
+// transient RPC failure does not decide where the scan starts. If the height
+// remains unavailable, the error is returned instead of falling back to block 0:
+// on a chain with history that fallback silently turns a passing RPC hiccup into
+// a full rescan from genesis, with duplicate finality notifications and no error
+// for the caller to distinguish it from a genuinely fresh chain.
+//
+// A nil Ledger is the one case that still starts at block 0 — there is no height
+// to read, so the whole chain is the intended range.
 func (d *Delivery) ScanBlock(background context.Context, callback fabric.BlockCallback) error {
 	startingBlock := uint64(0)
 	if d.Ledger != nil {
-		info, err := d.GetLedgerInfo()
-		if err == nil {
-			startingBlock = info.Height
-		} else {
-			d.Logger.ErrorfContext(background, "failed to get ledger info: %s", err)
+		height, err := d.ledgerHeight(background)
+		if err != nil {
+			return err
+		}
+		startingBlock = height
+	}
+
+	return d.Delivery.ScanBlockFrom(background, startingBlock, callback)
+}
+
+// ledgerHeight returns the current ledger height, retrying transient failures
+// with an exponentially growing delay. It gives up as soon as the context is
+// cancelled, and reports the last observed failure once the attempts run out.
+func (d *Delivery) ledgerHeight(ctx context.Context) (uint64, error) {
+	attempts := d.LedgerInfoAttempts
+	if attempts <= 0 {
+		attempts = defaultLedgerInfoAttempts
+	}
+	delay := d.LedgerInfoRetryDelay
+	if delay <= 0 {
+		delay = defaultLedgerInfoRetryDelay
+	}
+
+	var lastErr error
+	for attempt := 1; attempt <= attempts; attempt++ {
+		info, err := d.Ledger.GetLedgerInfo()
+		switch {
+		case err != nil:
+			lastErr = err
+		case info == nil:
+			// A driver that reports neither info nor error would otherwise nil
+			// deref below; treat it as a failure to read the height.
+			lastErr = errors.New("ledger returned no info")
+		default:
+			return info.Height, nil
+		}
+
+		d.Logger.ErrorfContext(ctx, "failed to get ledger info (attempt %d/%d): %s", attempt, attempts, lastErr)
+
+		if attempt == attempts {
+			break
+		}
+
+		select {
+		case <-ctx.Done():
+			return 0, errors.Wrapf(ctx.Err(), "cancelled while getting ledger info, last error [%v]", lastErr)
+		case <-time.After(delay):
 		}
+		delay *= 2
 	}
 
-	return d.ScanBlockFrom(background, startingBlock, callback)
+	return 0, errors.Wrapf(lastErr, "failed to get ledger info after %d attempt(s), refusing to rescan from genesis", attempts)
 }

token/services/network/fabric/finality/delivery_test.go

@@ -0,0 +1,210 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package finality_test
+
+import (
+	"context"
+	"errors"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/LFDT-Panurus/panurus/token/services/logging"
+	"github.com/LFDT-Panurus/panurus/token/services/network/fabric/finality"
+	"github.com/hyperledger-labs/fabric-smart-client/platform/fabric"
+	"github.com/hyperledger/fabric-protos-go-apiv2/common"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// heightLedgerResult is one scripted GetLedgerInfo outcome.
+type heightLedgerResult struct {
+	info *fabric.LedgerInfo
+	err  error
+}
+
+// fakeHeightLedger returns scripted GetLedgerInfo results, replaying the last
+// one once the script is exhausted, and counts how often it was called.
+type fakeHeightLedger struct {
+	results []heightLedgerResult
+	calls   atomic.Int32
+}
+
+func (l *fakeHeightLedger) GetLedgerInfo() (*fabric.LedgerInfo, error) {
+	n := int(l.calls.Add(1))
+	r := l.results[min(n, len(l.results))-1]
+
+	return r.info, r.err
+}
+
+// recordingBlockDelivery records the block ScanBlockFrom was asked to start
+// from, and whether it was called at all — the observable difference between
+// "resumed at the current height", "rescanned from genesis", and "did not scan".
+type recordingBlockDelivery struct {
+	called        bool
+	startingBlock uint64
+	callback      fabric.BlockCallback
+	err           error
+}
+
+func (d *recordingBlockDelivery) ScanBlockFrom(_ context.Context, block uint64, callback fabric.BlockCallback) error {
+	d.called = true
+	d.startingBlock = block
+	d.callback = callback
+
+	return d.err
+}
+
+// newTestDelivery builds a Delivery over the two fakes, with a retry delay short
+// enough to keep the tests fast.
+func newTestDelivery(l *fakeHeightLedger, d *recordingBlockDelivery) *finality.Delivery {
+	return &finality.Delivery{
+		Delivery:             d,
+		Ledger:               l,
+		Logger:               logging.MustGetLogger(),
+		LedgerInfoRetryDelay: time.Millisecond,
+	}
+}
+
+// TestScanBlock_StartsFromCurrentLedgerHeight covers the happy path: the scan
+// resumes at the height reported by the ledger, with the caller's callback
+// passed through untouched.
+func TestScanBlock_StartsFromCurrentLedgerHeight(t *testing.T) {
+	ledger := &fakeHeightLedger{results: []heightLedgerResult{{info: &fabric.LedgerInfo{Height: 42}}}}
+	delivery := &recordingBlockDelivery{}
+
+	callbackInvoked := false
+	callback := func(context.Context, *common.Block) (bool, error) {
+		callbackInvoked = true
+
+		return false, nil
+	}
+
+	require.NoError(t, newTestDelivery(ledger, delivery).ScanBlock(context.Background(), callback))
+
+	assert.True(t, delivery.called)
+	assert.Equal(t, uint64(42), delivery.startingBlock)
+	assert.Equal(t, int32(1), ledger.calls.Load(), "a successful GetLedgerInfo must not be retried")
+
+	require.NotNil(t, delivery.callback)
+	_, err := delivery.callback(context.Background(), nil)
+	require.NoError(t, err)
+	assert.True(t, callbackInvoked, "the caller's callback must be the one handed to ScanBlockFrom")
+}
+
+// TestScanBlock_PropagatesLedgerInfoErrorInsteadOfRescanningFromGenesis is the
+// regression test for issue #2058: a persistent GetLedgerInfo failure used to be
+// logged and swallowed, leaving startingBlock at 0 and silently triggering a full
+// rescan from genesis. The error must now reach the caller, and no scan must start.
+func TestScanBlock_PropagatesLedgerInfoErrorInsteadOfRescanningFromGenesis(t *testing.T) {
+	transient := errors.New("peer connection reset")
+	ledger := &fakeHeightLedger{results: []heightLedgerResult{{err: transient}}}
+	delivery := &recordingBlockDelivery{}
+
+	err := newTestDelivery(ledger, delivery).ScanBlock(context.Background(), nil)
+
+	require.Error(t, err, "GetLedgerInfo failure must be propagated, not swallowed after a log line")
+	require.ErrorIs(t, err, transient, "the underlying ledger error must stay inspectable")
+	assert.Contains(t, err.Error(), "refusing to rescan from genesis")
+	assert.False(t, delivery.called, "no scan may start from genesis when the ledger height is unknown")
+}
+
+// TestScanBlock_RetriesTransientLedgerInfoFailure verifies the transient case the
+// issue describes — an ordinary RPC hiccup — is absorbed by a retry and resumes at
+// the real height, rather than either failing or rescanning from block 0.
+func TestScanBlock_RetriesTransientLedgerInfoFailure(t *testing.T) {
+	ledger := &fakeHeightLedger{results: []heightLedgerResult{
+		{err: errors.New("peer connection reset")},
+		{info: &fabric.LedgerInfo{Height: 7}},
+	}}
+	delivery := &recordingBlockDelivery{}
+
+	require.NoError(t, newTestDelivery(ledger, delivery).ScanBlock(context.Background(), nil))
+
+	assert.Equal(t, int32(2), ledger.calls.Load())
+	assert.True(t, delivery.called)
+	assert.Equal(t, uint64(7), delivery.startingBlock, "must resume at the height read on the retry, not at genesis")
+}
+
+// TestScanBlock_RetriesAreBounded verifies the retry loop honours the configured
+// attempt budget instead of spinning on a permanently failing ledger.
+func TestScanBlock_RetriesAreBounded(t *testing.T) {
+	ledger := &fakeHeightLedger{results: []heightLedgerResult{{err: errors.New("peer connection reset")}}}
+	delivery := &recordingBlockDelivery{}
+
+	d := newTestDelivery(ledger, delivery)
+	d.LedgerInfoAttempts = 5
+
+	require.Error(t, d.ScanBlock(context.Background(), nil))
+	assert.Equal(t, int32(5), ledger.calls.Load())
+	assert.False(t, delivery.called)
+}
+
+// TestScanBlock_NilLedgerInfoIsReportedNotDereferenced covers a driver that
+// reports neither info nor error: that used to be a nil dereference waiting on
+// info.Height, and must instead surface as an error.
+func TestScanBlock_NilLedgerInfoIsReportedNotDereferenced(t *testing.T) {
+	ledger := &fakeHeightLedger{results: []heightLedgerResult{{}}}
+	delivery := &recordingBlockDelivery{}
+
+	err := newTestDelivery(ledger, delivery).ScanBlock(context.Background(), nil)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "ledger returned no info")
+	assert.False(t, delivery.called)
+}
+
+// TestScanBlock_CancelledContextStopsRetrying verifies shutdown does not have to
+// wait out the retry budget: a cancelled context ends the loop with its own error.
+func TestScanBlock_CancelledContextStopsRetrying(t *testing.T) {
+	ledger := &fakeHeightLedger{results: []heightLedgerResult{{err: errors.New("peer connection reset")}}}
+	delivery := &recordingBlockDelivery{}
+
+	d := newTestDelivery(ledger, delivery)
+	d.LedgerInfoAttempts = 100
+	d.LedgerInfoRetryDelay = time.Hour // only cancellation can end this loop in time
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	done := make(chan error, 1)
+	go func() { done <- d.ScanBlock(ctx, nil) }()
+
+	select {
+	case err := <-done:
+		require.ErrorIs(t, err, context.Canceled)
+		assert.Equal(t, int32(1), ledger.calls.Load(), "must not keep retrying after cancellation")
+		assert.False(t, delivery.called)
+	case <-time.After(10 * time.Second):
+		t.Fatal("ScanBlock kept retrying after its context was cancelled")
+	}
+}
+
+// TestScanBlock_NilLedgerScansFromGenesis pins the one remaining path that
+// legitimately starts at block 0: no ledger is configured, so there is no height
+// to resume from and the whole chain is the intended range.
+func TestScanBlock_NilLedgerScansFromGenesis(t *testing.T) {
+	delivery := &recordingBlockDelivery{}
+	d := &finality.Delivery{Delivery: delivery, Logger: logging.MustGetLogger()}
+
+	require.NoError(t, d.ScanBlock(context.Background(), nil))
+	assert.True(t, delivery.called)
+	assert.Equal(t, uint64(0), delivery.startingBlock)
+}
+
+// TestScanBlock_PropagatesScanError verifies the scan's own error is returned
+// unchanged once the starting block has been resolved.
+func TestScanBlock_PropagatesScanError(t *testing.T) {
+	scanErr := errors.New("delivery stream broken")
+	ledger := &fakeHeightLedger{results: []heightLedgerResult{{info: &fabric.LedgerInfo{Height: 3}}}}
+	delivery := &recordingBlockDelivery{err: scanErr}
+
+	err := newTestDelivery(ledger, delivery).ScanBlock(context.Background(), nil)
+
+	require.ErrorIs(t, err, scanErr)
+	assert.Equal(t, uint64(3), delivery.startingBlock)
+}