ingest/ledgerbackend: fix rpc ledger backend retries

.github/workflows/golangci-lint.yml

@@ -15,16 +15,15 @@ jobs:
     runs-on: ubuntu-22.04
     steps:
       - name: Checkout
-        uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # version v3.0.2
+        uses: actions/checkout@v4
         with:
           fetch-depth: 0 # required for new-from-rev option in .golangci.yml
-      - name: Setup GO
-        uses: actions/setup-go@268d8c0ca0432bb2cf416faae41297df9d262d7f # version v3.3.0
+      - uses: ./.github/actions/setup-go
       - name: Run golangci-lint
-        uses: golangci/golangci-lint-action@537aa1903e5d359d0b27dbc19ddd22c5087f3fbc # version v3.2.0
+        uses: golangci/golangci-lint-action@v7
         with:
-          version: v1.52.2 # this is the golangci-lint version
-          args: --issues-exit-code=0 # exit without errors for now - won't fail the build
+          version: v2.1.6
+          args: -v --issues-exit-code=0 --config=.golangci.yml # exit without errors for now - won't fail the build
           github-token: ${{ secrets.GITHUB_TOKEN }}
           only-new-issues: true
 

.golangci.yml

@@ -1,129 +1,95 @@
-linters-settings:
-  depguard:
-    list-type: denylist
-    packages:
-      # logging is allowed only by logutils.Log, logrus
-      # is allowed to use only in logutils package
-      - github.com/sirupsen/logrus
-    packages-with-error-message:
-      - github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
-  dupl:
-    threshold: 100
-  funlen:
-    lines: 100
-    statements: 50
-  goconst:
-    min-len: 2
-    min-occurrences: 3
-  gocritic:
-    enabled-tags:
-      - diagnostic
-      - experimental
-      - opinionated
-      - performance
-      - style
-    disabled-checks:
-      - dupImport # https://github.com/go-critic/go-critic/issues/845
-      - ifElseChain
-      - octalLiteral
-      - whyNoLint
-  gocyclo:
-    min-complexity: 15
-  goimports:
-    local-prefixes: github.com/golangci/golangci-lint
-  gomnd:
-    # don't include the "operation" and "assign"
-    checks:
-      - argument
-      - case
-      - condition
-      - return
-    ignored-numbers:
-      - '0'
-      - '1'
-      - '2'
-      - '3'
-    ignored-functions:
-      - strings.SplitN
-
-  govet:
-    check-shadowing: true
-    settings:
-      printf:
-        funcs:
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
-          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
-  lll:
-    line-length: 140
-  misspell:
-    locale: US
-  nolintlint:
-    allow-unused: false # report any unused nolint directives
-    require-explanation: false # don't require an explanation for nolint directives
-    require-specific: false # don't require nolint directives to be specific about which linter is being skipped
-
+version: "2"
 linters:
-  disable-all: true
+  default: none
   enable:
     - bodyclose
     - depguard
     - dogsled
     - dupl
     - errcheck
-    - exportloopref
     - funlen
     - gochecknoinits
     - goconst
-    #- gocritic
     - gocyclo
-    - gofmt
-    - goimports
-    - gomnd
     - goprintffuncname
     - gosec
-    - gosimple
     - govet
     - ineffassign
     - lll
     - misspell
+    - mnd
     - nakedret
     - noctx
     - nolintlint
     - staticcheck
-    - stylecheck
-    - typecheck
     - unconvert
     - unparam
     - unused
-    #- whitespace
-
-  # don't enable:
-  # - asciicheck
-  # - scopelint
-  # - gochecknoglobals
-  # - gocognit
-  # - godot
-  # - godox
-  # - goerr113
-  # - interfacer
-  # - maligned
-  # - nestif
-  # - prealloc
-  # - testpackage
-  # - revive
-  # - wsl
-
-issues:
-  # Excluding configuration per-path, per-linter, per-text and per-source
-  exclude-rules:
-    - path: _test\.go
-      linters:
-        - govet
-
-run:
-  timeout: 5m
-  skip-dirs:
-    - docs
-    - vendor
+  settings:
+    dupl:
+      threshold: 100
+    funlen:
+      lines: 100
+      statements: 50
+    goconst:
+      min-len: 2
+      min-occurrences: 3
+    gocritic:
+      disabled-checks:
+        - dupImport
+        - ifElseChain
+        - octalLiteral
+        - whyNoLint
+      enabled-tags:
+        - diagnostic
+        - experimental
+        - opinionated
+        - performance
+        - style
+    gocyclo:
+      min-complexity: 15
+    govet:
+      settings:
+        printf:
+          funcs:
+            - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
+            - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
+            - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
+            - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
+    lll:
+      line-length: 140
+    misspell:
+      locale: US
+    nolintlint:
+      require-explanation: false
+      require-specific: false
+      allow-unused: false
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      - linters:
+          - govet
+        path: _test\.go
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
+  enable:
+    - gofmt
+    - goimports
+  settings:
+    goimports:
+      local-prefixes:
+        - github.com/golangci/golangci-lint
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

ingest/ledgerbackend/rpc_backend.go

@@ -9,6 +9,7 @@
 	"sync/atomic"
 	"time"
 
+	"github.com/creachadair/jrpc2"
 	"github.com/stellar/go/xdr"
 	rpc "github.com/stellar/stellar-rpc/client"
 	"github.com/stellar/stellar-rpc/protocol"
@@ -25,13 +26,10 @@
 	return fmt.Sprintf("ledger %d was not present on rpc", e.Sequence)
 }
 
-type RPCLedgerBeyondLatestError struct {
-	Sequence     uint32
-	LatestLedger uint32
-}
+type RPCLedgerBeyondLatestError struct{}
 
-func (e *RPCLedgerBeyondLatestError) Error() string {
-	return fmt.Sprintf("ledger %d is beyond the RPC latest ledger is %d", e.Sequence, e.LatestLedger)
+func (e RPCLedgerBeyondLatestError) Error() string {
+	return "ledger is not available on the RPC server yet"
 }
 
 // The minimum required RPC client methods used by RPCLedgerBackend.
@@ -146,8 +144,8 @@
 			return lcm, nil
 		}
 
-		_, isBeyondErr := err.(*RPCLedgerBeyondLatestError)
-		if !isBeyondErr {
+		var beyondErr *RPCLedgerBeyondLatestError
+		if !(errors.As(err, &beyondErr)) {
 			return xdr.LedgerCloseMeta{}, err
 		}
 
@@ -250,17 +248,21 @@
 
 	ledgers, err := b.client.GetLedgers(ctx, req)
 	if err != nil {
+		// InvalidRequest code is the most specific error code provided for invalid range requests.
+		// https://github.com/stellar/stellar-rpc/pull/407/
+		// if received, assume it's range problem to enable retry.
+		var rpcErr *jrpc2.Error
+		if errors.As(err, &rpcErr) && rpcErr.Code == jrpc2.InvalidRequest {
+			return xdr.LedgerCloseMeta{}, &RPCLedgerBeyondLatestError{}
+		}
 		return xdr.LedgerCloseMeta{}, fmt.Errorf("failed to get ledgers starting from %d: %w", sequence, err)
 	}
 
 	b.initBuffer()
 
 	// Check if requested ledger is beyond the RPC retention window
 	if sequence > ledgers.LatestLedger {
-		return xdr.LedgerCloseMeta{}, &RPCLedgerBeyondLatestError{
-			Sequence:     sequence,
-			LatestLedger: ledgers.LatestLedger,
-		}
+		return xdr.LedgerCloseMeta{}, &RPCLedgerBeyondLatestError{}
 	}
 
 	// Populate buffer with new ledgers

ingest/ledgerbackend/rpc_backend_test.go

@@ -6,6 +6,7 @@
 	"testing"
 	"time"
 
+	"github.com/creachadair/jrpc2"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 
@@ -148,7 +149,7 @@
 	})
 }
 
-func TestGetLedgerBeyondLatest(t *testing.T) {
+func TestGetLedgerBeyondLatestBasedOnEmptyResults(t *testing.T) {
 	rpcBackend, mockClient := setupRPCTest(t)
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
@@ -207,7 +208,68 @@
 
 	// Verify timing - GetLedger should have waited one interval and then refetched ledgers from rpc on second call
 	assert.GreaterOrEqual(t, duration.Seconds(), float64(rpcBackendDefaultWaitIntervalSeconds))
+}
+
+func TestGetLedgerBeyondLatestBasedOnErr(t *testing.T) {
+	rpcBackend, mockClient := setupRPCTest(t)
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	requestedSequence := uint32(100)
+
+	rpcGetLedgersRequest := protocol.GetLedgersRequest{
+		StartLedger: requestedSequence,
+		Pagination: &protocol.LedgerPaginationOptions{
+			Limit: uint(rpcBackendDefaultBufferSize),
+		},
+	}
+
+	invalidRequestErr := &jrpc2.Error{
+		Code: jrpc2.InvalidRequest,
+	}
+
+	// called by PrepareRange
+	mockClient.On("GetLedgers", ctx, rpcGetLedgersRequest).Return(protocol.GetLedgersResponse{}, invalidRequestErr).Once()
+	// called by GetLedger first time
+	mockClient.On("GetLedgers", ctx, rpcGetLedgersRequest).Return(protocol.GetLedgersResponse{}, invalidRequestErr).Once()
+
+	// Setup second call to return the requested ledger
+	lcm := xdr.LedgerCloseMeta{
+		V: 0,
+		V0: &xdr.LedgerCloseMetaV0{
+			LedgerHeader: xdr.LedgerHeaderHistoryEntry{
+				Header: xdr.LedgerHeader{
+					LedgerSeq: xdr.Uint32(requestedSequence),
+				},
+			},
+		},
+	}
+	encodedLCM, err := xdr.MarshalBase64(lcm)
+	assert.NoError(t, err)
+
+	secondResponse := protocol.GetLedgersResponse{
+		LatestLedger: requestedSequence,
+		Ledgers: []protocol.LedgerInfo{
+			{
+				Sequence:       requestedSequence,
+				LedgerMetadata: encodedLCM,
+			},
+		},
+	}
+	// called by GetLedger second time
+	mockClient.On("GetLedgers", ctx, rpcGetLedgersRequest).Return(secondResponse, nil).Once()
+
+	preparedRange := Range{from: requestedSequence, to: requestedSequence + 10, bounded: true}
+	assert.NoError(t, rpcBackend.PrepareRange(ctx, preparedRange))
 
+	startTime := time.Now()
+	actualLCM, err := rpcBackend.GetLedger(ctx, requestedSequence)
+	duration := time.Since(startTime)
+
+	assert.NoError(t, err)
+	assert.Equal(t, requestedSequence, uint32(actualLCM.V0.LedgerHeader.Header.LedgerSeq))
+
+	// Verify timing - GetLedger should have waited one interval and then refetched ledgers from rpc on second call
+	assert.GreaterOrEqual(t, duration.Seconds(), float64(rpcBackendDefaultWaitIntervalSeconds))
 }
 
 func TestGetLedgerContextTimeout(t *testing.T) {