diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index fe31e4671b..10b938b888 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.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 diff --git a/.golangci.yml b/.golangci.yml index 979ef6c0bc..4b422298a1 100644 --- a/.golangci.yml +++ b/.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$ diff --git a/go.mod b/go.mod index 35fbdf317a..94eb927769 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,6 @@ require ( github.com/adjust/goautoneg v0.0.0-20150426214442-d788f35a0315 github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 github.com/aws/aws-sdk-go v1.45.27 - github.com/creachadair/jrpc2 v1.2.0 github.com/djherbis/fscache v0.10.1 github.com/elazarl/go-bindata-assetfs v1.0.1 github.com/getsentry/raven-go v0.2.0 @@ -58,6 +57,7 @@ require ( require ( github.com/cenkalti/backoff/v4 v4.3.0 + github.com/creachadair/jrpc2 v1.2.0 github.com/dgryski/go-farm v0.0.0-20240924180020-3414d57e47da github.com/docker/docker v27.3.1+incompatible github.com/docker/go-connections v0.5.0 diff --git a/ingest/ledgerbackend/rpc_backend.go b/ingest/ledgerbackend/rpc_backend.go index e5afdcf378..e623890d41 100644 --- a/ingest/ledgerbackend/rpc_backend.go +++ b/ingest/ledgerbackend/rpc_backend.go @@ -25,18 +25,16 @@ func (e *RPCLedgerMissingError) Error() string { 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. type RPCLedgerGetter interface { GetLedgers(ctx context.Context, req protocol.GetLedgersRequest) (protocol.GetLedgersResponse, error) + GetHealth(ctx context.Context) (protocol.GetHealthResponse, error) // <-- Added } type RPCLedgerBackend struct { @@ -146,8 +144,8 @@ func (b *RPCLedgerBackend) GetLedger(ctx context.Context, sequence uint32) (xdr. return lcm, nil } - _, isBeyondErr := err.(*RPCLedgerBeyondLatestError) - if !isBeyondErr { + var beyondErr *rpcLedgerBeyondLatestError + if !(errors.As(err, &beyondErr)) { return xdr.LedgerCloseMeta{}, err } @@ -182,7 +180,7 @@ func (b *RPCLedgerBackend) PrepareRange(ctx context.Context, ledgerRange Range) _, err := b.getBufferedLedger(ctx, ledgerRange.from) if err != nil { // beyond latest is handled later in GetLedger - var beyondErr *RPCLedgerBeyondLatestError + var beyondErr *rpcLedgerBeyondLatestError if !(errors.As(err, &beyondErr)) { return err } @@ -240,7 +238,16 @@ func (b *RPCLedgerBackend) getBufferedLedger(ctx context.Context, sequence uint3 return lcm, nil } - // Ledger not in buffer, fetch a small batch from RPC starting from the requested sequence + // Check if requested ledger is beyond the RPC retention window using GetHealth + health, err := b.client.GetHealth(ctx) + if err != nil { + return xdr.LedgerCloseMeta{}, fmt.Errorf("failed to get health from RPC: %w", err) + } + if sequence > health.LatestLedger { + return xdr.LedgerCloseMeta{}, &rpcLedgerBeyondLatestError{} + } + + // attempt to fetch a small batch from RPC starting from the requested sequence req := protocol.GetLedgersRequest{ StartLedger: sequence, Pagination: &protocol.LedgerPaginationOptions{ @@ -250,19 +257,11 @@ func (b *RPCLedgerBackend) getBufferedLedger(ctx context.Context, sequence uint3 ledgers, err := b.client.GetLedgers(ctx, req) if err != nil { - return xdr.LedgerCloseMeta{}, fmt.Errorf("failed to get ledgers starting from %d: %w", sequence, err) + return xdr.LedgerCloseMeta{}, 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, - } - } - // Populate buffer with new ledgers for _, ledger := range ledgers.Ledgers { var lcm xdr.LedgerCloseMeta diff --git a/ingest/ledgerbackend/rpc_backend_test.go b/ingest/ledgerbackend/rpc_backend_test.go index ca7c3d04ad..a883ee51de 100644 --- a/ingest/ledgerbackend/rpc_backend_test.go +++ b/ingest/ledgerbackend/rpc_backend_test.go @@ -22,6 +22,11 @@ func (m *MockRPCClient) GetLedgers(ctx context.Context, req protocol.GetLedgersR return args.Get(0).(protocol.GetLedgersResponse), args.Error(1) } +func (m *MockRPCClient) GetHealth(ctx context.Context) (protocol.GetHealthResponse, error) { + args := m.Called(ctx) + return args.Get(0).(protocol.GetHealthResponse), args.Error(1) +} + func setupRPCTest(t *testing.T) (*RPCLedgerBackend, *MockRPCClient) { mockClient := new(MockRPCClient) backend := &RPCLedgerBackend{ @@ -38,6 +43,10 @@ func TestRPCGetLedger(t *testing.T) { ctx := context.Background() sequence := uint32(12345) + mockClient.On("GetHealth", ctx).Return(protocol.GetHealthResponse{ + LatestLedger: sequence + 10, + }, nil) + lcm := xdr.LedgerCloseMeta{ V: 0, V0: &xdr.LedgerCloseMetaV0{ @@ -88,12 +97,12 @@ func TestRPCGetLedger(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "requested ledger 12345 is not the expected ledger 12346") - // Test requesteed ledger is outside of prepared range + // Test requested ledger is outside of prepared range _, err = rpcBackend.GetLedger(ctx, sequence+50) assert.Error(t, err) assert.Contains(t, err.Error(), "requested ledger 12395 is outside prepared range") - // Test requested ledger is after oldest range in rpc but, was missing from response + // Test requested ledger is in valid range of rpc but was missing from response notFoundSequnce := sequence + 1 expectedReq.StartLedger = notFoundSequnce mockClient.On("GetLedgers", ctx, expectedReq).Return(mockMissingLedgerResponse, nil).Once() @@ -103,10 +112,9 @@ func TestRPCGetLedger(t *testing.T) { assert.Equal(t, notFoundSequnce, missingErr.Sequence) // Test rpc error response - expectedErr := fmt.Errorf("rpc connection error") + expectedErr := fmt.Errorf("rpc error") mockClient.On("GetLedgers", ctx, expectedReq).Return(protocol.GetLedgersResponse{}, expectedErr).Once() _, err = rpcBackend.GetLedger(ctx, sequence+1) - assert.Error(t, err) assert.Contains(t, err.Error(), expectedErr.Error()) // Verify Closed backend @@ -116,7 +124,6 @@ func TestRPCGetLedger(t *testing.T) { _, err = rpcBackend.GetLedger(ctx, sequence) assert.Error(t, err) assert.Contains(t, err.Error(), "RPCLedgerBackend is closed") - } func TestRPCBackendImplementsInterface(t *testing.T) { @@ -148,29 +155,26 @@ func TestNewRPCLedgerBackend(t *testing.T) { }) } -func TestGetLedgerBeyondLatest(t *testing.T) { +func TestGetLedgerWaitsForLatest(t *testing.T) { rpcBackend, mockClient := setupRPCTest(t) ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() requestedSequence := uint32(100) - latestLedger := requestedSequence - 1 // Latest ledger is 1 behind requested - rpcGetLedgersRequest := protocol.GetLedgersRequest{ StartLedger: requestedSequence, Pagination: &protocol.LedgerPaginationOptions{ Limit: uint(rpcBackendDefaultBufferSize), }, } - // Setup first response indicating ledger is beyond latest - firstResponse := protocol.GetLedgersResponse{ - LatestLedger: latestLedger, - Ledgers: []protocol.LedgerInfo{}, // Empty ledgers array - } - // gets called by preparerange, and test expects it to get called by GetLedger first time - mockClient.On("GetLedgers", ctx, rpcGetLedgersRequest).Return(firstResponse, nil).Twice() - // Setup second call to return the requested ledger + // this gets used on Prepared Range call and first call to GetLedger + // indicates requested ledger is beyond rpc latest + mockClient.On("GetHealth", ctx).Return(protocol.GetHealthResponse{ + LatestLedger: requestedSequence - 1, + }, nil).Twice() + + // Setup call to GetLedger return the requested ledger lcm := xdr.LedgerCloseMeta{ V: 0, V0: &xdr.LedgerCloseMetaV0{ @@ -184,7 +188,7 @@ func TestGetLedgerBeyondLatest(t *testing.T) { encodedLCM, err := xdr.MarshalBase64(lcm) assert.NoError(t, err) - secondResponse := protocol.GetLedgersResponse{ + getResponse := protocol.GetLedgersResponse{ LatestLedger: requestedSequence, Ledgers: []protocol.LedgerInfo{ { @@ -193,24 +197,30 @@ func TestGetLedgerBeyondLatest(t *testing.T) { }, }, } - mockClient.On("GetLedgers", ctx, rpcGetLedgersRequest).Return(secondResponse, nil).Once() + // called by GetLedger on second try, indicates rpc latest has advanced beyond requested + mockClient.On("GetHealth", ctx).Return(protocol.GetHealthResponse{ + LatestLedger: requestedSequence + 10, + }, nil).Once() + // now it attempts GetLedgers call + mockClient.On("GetLedgers", ctx, rpcGetLedgersRequest).Return(getResponse, nil).Once() preparedRange := Range{from: requestedSequence, to: requestedSequence + 10, bounded: true} - rpcBackend.PrepareRange(ctx, preparedRange) + assert.NoError(t, rpcBackend.PrepareRange(ctx, preparedRange)) startTime := time.Now() + // first call to GetLedger gets health status indicating requested ledger is beyond roc latest + // triggers retry logic 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 + // Verify timing - GetLedger should have waited one retry interval and then refetched ledgers from rpc on second call assert.GreaterOrEqual(t, duration.Seconds(), float64(rpcBackendDefaultWaitIntervalSeconds)) - } -func TestGetLedgerContextTimeout(t *testing.T) { +func TestGetLedgerContextTimeoutInterrupt(t *testing.T) { rpcBackend, mockClient := setupRPCTest(t) sequence := uint32(100) background := context.Background() @@ -218,19 +228,9 @@ func TestGetLedgerContextTimeout(t *testing.T) { ctx, cancel := context.WithTimeout(background, 100*time.Millisecond) defer cancel() - expectedReq := protocol.GetLedgersRequest{ - StartLedger: sequence, - Pagination: &protocol.LedgerPaginationOptions{ - Limit: uint(rpcBackendDefaultBufferSize), - }, - } - // represents request that is beyond latest on rpc - // in GetLLedger, this will trigger retry request after context deadline exceeded - mockResponse := protocol.GetLedgersResponse{ - LatestLedger: sequence - 2, - Ledgers: []protocol.LedgerInfo{}, - } - mockClient.On("GetLedgers", mock.Anything, expectedReq).Return(mockResponse, nil) + mockClient.On("GetHealth", mock.Anything).Return(protocol.GetHealthResponse{ + LatestLedger: sequence - 1, + }, nil).Twice() // Prepare the range first, it doesn't mind the beyond latest response preparedRange := Range{from: sequence, to: sequence + 10, bounded: true} @@ -242,34 +242,30 @@ func TestGetLedgerContextTimeout(t *testing.T) { assert.ErrorIs(t, err, context.DeadlineExceeded) } -func TestGetLedgerWhileClosed(t *testing.T) { +func TestGetLedgerClosedInterrupt(t *testing.T) { rpcBackend, mockClient := setupRPCTest(t) sequence := uint32(100) ctx := context.Background() - expectedReq := protocol.GetLedgersRequest{ - StartLedger: sequence, - Pagination: &protocol.LedgerPaginationOptions{ - Limit: uint(rpcBackendDefaultBufferSize), - }, - } - // represents request that is beyond latest on rpc - // allows it to check for retry which will fail due to closed backend - mockResponse := protocol.GetLedgersResponse{ - LatestLedger: sequence - 2, - Ledgers: []protocol.LedgerInfo{}, + healthResponse := protocol.GetHealthResponse{ + LatestLedger: sequence - 1, } - mockClient.On("GetLedgers", ctx, expectedReq). + // prepare range calls it + mockClient.On("GetHealth", ctx).Return(healthResponse, nil).Once() + + // get ledger calls it again, + // and artificallly simulate backedn being closed by caller + mockClient.On("GetHealth", ctx). Run(func(args mock.Arguments) { assert.NoError(t, rpcBackend.Close()) - }).Return(mockResponse, nil).Once() + }).Return(healthResponse, nil).Once() // Prepare the range first, it doesn't mind the beyond latest response preparedRange := Range{from: sequence, to: sequence + 10, bounded: true} rpcBackend.PrepareRange(ctx, preparedRange) - // Call GetLedger it will attempt to retry due to beyond latest, and detect closed + // Call GetLedger it will attempt roc retry and detect closed after _, err := rpcBackend.GetLedger(ctx, sequence) assert.ErrorContains(t, err, "RPCLedgerBackend is closed") } @@ -280,6 +276,9 @@ func TestPrepareRange(t *testing.T) { ctx := context.Background() start := uint32(150) + mockClient.On("GetHealth", ctx).Return(protocol.GetHealthResponse{ + LatestLedger: start + 10, + }, nil).Once() expectedReq := protocol.GetLedgersRequest{ StartLedger: start, Pagination: &protocol.LedgerPaginationOptions{ @@ -295,6 +294,11 @@ func TestPrepareRange(t *testing.T) { err := backend.PrepareRange(ctx, Range{from: start, to: start + 30, bounded: true}) assert.NoError(t, err) assert.Equal(t, uint32(150), backend.nextLedger) + + // Second prepare should fail + err = backend.PrepareRange(ctx, Range{from: start, to: start + 30, bounded: true}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "already prepared") }) t.Run("unbounded range", func(t *testing.T) { @@ -302,6 +306,10 @@ func TestPrepareRange(t *testing.T) { ctx := context.Background() start := uint32(150) + mockClient.On("GetHealth", ctx).Return(protocol.GetHealthResponse{ + LatestLedger: start + 10, + }, nil).Once() + expectedReq := protocol.GetLedgersRequest{ StartLedger: start, Pagination: &protocol.LedgerPaginationOptions{ @@ -319,33 +327,6 @@ func TestPrepareRange(t *testing.T) { assert.Equal(t, uint32(150), backend.nextLedger) }) - t.Run("error when already prepared", func(t *testing.T) { - backend, mockClient := setupRPCTest(t) - ctx := context.Background() - start := uint32(150) - - expectedReq := protocol.GetLedgersRequest{ - StartLedger: start, - Pagination: &protocol.LedgerPaginationOptions{ - Limit: uint(rpcBackendDefaultBufferSize), - }, - } - mockResponse := protocol.GetLedgersResponse{ - LatestLedger: start + 10, - Ledgers: []protocol.LedgerInfo{generateRPCInfo(start)}, - } - mockClient.On("GetLedgers", ctx, expectedReq).Return(mockResponse, nil).Once() - - // First prepare should succeed - err := backend.PrepareRange(ctx, Range{from: start, to: start + 30, bounded: true}) - assert.NoError(t, err) - - // Second prepare should fail - err = backend.PrepareRange(ctx, Range{from: start + 10, to: start + 40, bounded: true}) - assert.Error(t, err) - assert.Contains(t, err.Error(), "already prepared") - }) - t.Run("error when RPC returns error", func(t *testing.T) { backend, mockClient := setupRPCTest(t) ctx := context.Background() @@ -353,6 +334,10 @@ func TestPrepareRange(t *testing.T) { expectedErr := fmt.Errorf("rpc server side reported error") start := uint32(150) + mockClient.On("GetHealth", ctx).Return(protocol.GetHealthResponse{ + LatestLedger: start + 10, + }, nil).Once() + expectedReq := protocol.GetLedgersRequest{ StartLedger: start, Pagination: &protocol.LedgerPaginationOptions{ @@ -468,20 +453,11 @@ func TestRPCBackendGetLatestLedgerSequence(t *testing.T) { ctx := context.Background() start := uint32(150) - expectedReq := protocol.GetLedgersRequest{ - StartLedger: start, - Pagination: &protocol.LedgerPaginationOptions{ - Limit: uint(rpcBackendDefaultBufferSize), - }, - } - // Setup first response indicating ledger is beyond latest - mockResponse := protocol.GetLedgersResponse{ + mockClient.On("GetHealth", ctx).Return(protocol.GetHealthResponse{ LatestLedger: start - 1, - Ledgers: []protocol.LedgerInfo{}, // Empty ledgers array - } - mockClient.On("GetLedgers", ctx, expectedReq).Return(mockResponse, nil) + }, nil).Once() - // establish a prepared, but empty buffer state + // establish a prepared, but was beyond rpc latest, so empty buffer state err := backend.PrepareRange(ctx, Range{from: start, to: start + 10, bounded: true}) assert.NoError(t, err) @@ -498,6 +474,10 @@ func TestRPCBackendGetLatestLedgerSequence(t *testing.T) { ctx := context.Background() start := uint32(150) + mockClient.On("GetHealth", ctx).Return(protocol.GetHealthResponse{ + LatestLedger: start + 10, + }, nil).Once() + expectedReq := protocol.GetLedgersRequest{ StartLedger: start, Pagination: &protocol.LedgerPaginationOptions{