From 039d625d0dbccd5bb3edd833054282c7f995d0a2 Mon Sep 17 00:00:00 2001 From: Awbrey Hughlett Date: Tue, 24 Dec 2024 11:26:00 -0600 Subject: [PATCH] fix linter issues --- .github/workflows/golangci-lint.yml | 4 ++ .golangci.yml | 2 +- pkg/solana/config_tracker.go | 9 ++-- pkg/solana/fees/block_history_test.go | 70 +++++++++++++-------------- pkg/solana/fees/fixed_price.go | 6 +-- pkg/solana/fees/utils.go | 12 ++--- pkg/solana/logpoller/parser.go | 4 +- pkg/solana/logpoller/worker.go | 4 +- pkg/solana/report_test.go | 15 +++--- pkg/solana/txm/pendingtx_test.go | 11 ++--- 10 files changed, 69 insertions(+), 68 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index c47f83c63..674d5cc4f 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -14,6 +14,8 @@ jobs: uses: cachix/install-nix-action@3715ab1a11cac9e991980d7b4a28d80c7ebdd8f9 # nix:v2.24.6 with: nix_path: nixpkgs=channel:nixos-unstable + - name: Print golangci-lint Version + run: nix develop -c golangci-lint --version - name: golangci-lint run: nix develop -c make lint-go-integration-tests - name: Print lint report artifact @@ -37,6 +39,8 @@ jobs: uses: cachix/install-nix-action@3715ab1a11cac9e991980d7b4a28d80c7ebdd8f9 # nix:v2.24.6 with: nix_path: nixpkgs=channel:nixos-unstable + - name: Print golangci-lint Version + run: nix develop -c golangci-lint --version - name: golangci-lint run: nix develop -c make lint-go-relay - name: Print lint report artifact diff --git a/.golangci.yml b/.golangci.yml index d0542f400..6744196f9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,7 +3,7 @@ run: linters: enable: - exhaustive - - exportloopref + - copyloopvar - revive - goimports - gosec diff --git a/pkg/solana/config_tracker.go b/pkg/solana/config_tracker.go index 3ddff2715..96185bf5b 100644 --- a/pkg/solana/config_tracker.go +++ b/pkg/solana/config_tracker.go @@ -26,14 +26,15 @@ func (c *ConfigTracker) LatestConfigDetails(ctx context.Context) (changedInBlock func ConfigFromState(ctx context.Context, state State) (types.ContractConfig, error) { pubKeys := []types.OnchainPublicKey{} accounts := []types.Account{} + oracles, err := state.Oracles.Data() if err != nil { return types.ContractConfig{}, err } - for _, o := range oracles { - o := o // https://github.com/golang/go/wiki/CommonMistakes#using-reference-to-loop-iterator-variable - pubKeys = append(pubKeys, o.Signer.Key[:]) - accounts = append(accounts, types.Account(o.Transmitter.String())) + + for idx := range oracles { + pubKeys = append(pubKeys, oracles[idx].Signer.Key[:]) + accounts = append(accounts, types.Account(oracles[idx].Transmitter.String())) } onchainConfigStruct := median.OnchainConfig{ diff --git a/pkg/solana/fees/block_history_test.go b/pkg/solana/fees/block_history_test.go index 03ab41b63..2cb92cc72 100644 --- a/pkg/solana/fees/block_history_test.go +++ b/pkg/solana/fees/block_history_test.go @@ -41,8 +41,8 @@ func TestBlockHistoryEstimator_InvalidBlockHistorySize(t *testing.T) { func TestBlockHistoryEstimator_LatestBlock(t *testing.T) { // Helper variables for tests - min := uint64(10) - max := uint64(100_000) + minPrice := uint64(10) + maxPrice := uint64(100_000) defaultPrice := uint64(100) depth := uint64(1) // 1 is LatestBlockEstimator pollPeriod := 100 * time.Millisecond @@ -63,13 +63,13 @@ func TestBlockHistoryEstimator_LatestBlock(t *testing.T) { t.Run("Successful Estimation", func(t *testing.T) { // Setup cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Assert the computed price matches the expected price require.NoError(t, estimator.calculatePrice(ctx), "Failed to calculate price") - cfg.On("ComputeUnitPriceMin").Return(min) - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMin").Return(minPrice) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, uint64(lastBlockMedianPrice), estimator.BaseComputeUnitPrice()) }) @@ -77,7 +77,7 @@ func TestBlockHistoryEstimator_LatestBlock(t *testing.T) { // Setup cfg := cfgmock.NewConfig(t) tmpMin := uint64(lastBlockMedianPrice) + 100 // Set min higher than the median price - setupConfigMock(cfg, defaultPrice, tmpMin, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, tmpMin, pollPeriod, depth) estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Call calculatePrice and ensure no error @@ -91,14 +91,14 @@ func TestBlockHistoryEstimator_LatestBlock(t *testing.T) { // Setup cfg := cfgmock.NewConfig(t) tmpMax := uint64(lastBlockMedianPrice) - 100 // Set max lower than the median price - setupConfigMock(cfg, defaultPrice, min, tmpMax, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Call calculatePrice and ensure no error // Assert the compute unit price is capped at max require.NoError(t, estimator.calculatePrice(ctx), "Failed to calculate price with price above max") cfg.On("ComputeUnitPriceMax").Return(tmpMax) - cfg.On("ComputeUnitPriceMin").Return(min) + cfg.On("ComputeUnitPriceMin").Return(minPrice) assert.Equal(t, tmpMax, estimator.BaseComputeUnitPrice(), "Price should be capped at max") }) @@ -109,13 +109,13 @@ func TestBlockHistoryEstimator_LatestBlock(t *testing.T) { return rw, nil }) cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) rw.On("GetLatestBlock", mock.Anything).Return(nil, fmt.Errorf("fail rpc call")) // Mock GetLatestBlock returning error estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Ensure the price remains unchanged require.Error(t, estimator.calculatePrice(ctx), "Expected error when GetLatestBlock fails") - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, uint64(100), estimator.BaseComputeUnitPrice(), "Price should not change when GetLatestBlock fails") }) @@ -126,13 +126,13 @@ func TestBlockHistoryEstimator_LatestBlock(t *testing.T) { return rw, nil }) cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) rw.On("GetLatestBlock", mock.Anything).Return(nil, nil) // Mock GetLatestBlock returning nil estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Ensure the price remains unchanged require.Error(t, estimator.calculatePrice(ctx), "Expected error when parsing fails") - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, uint64(100), estimator.BaseComputeUnitPrice(), "Price should not change when parsing fails") }) @@ -143,13 +143,13 @@ func TestBlockHistoryEstimator_LatestBlock(t *testing.T) { return rw, nil }) cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) rw.On("GetLatestBlock", mock.Anything).Return(&rpc.GetBlockResult{}, nil) // Mock GetLatestBlock returning empty block estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Ensure the price remains unchanged require.EqualError(t, estimator.calculatePrice(ctx), errNoComputeUnitPriceCollected.Error(), "Expected error when no compute unit prices are collected") - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, uint64(100), estimator.BaseComputeUnitPrice(), "Price should not change when median calculation fails") }) @@ -160,21 +160,21 @@ func TestBlockHistoryEstimator_LatestBlock(t *testing.T) { return nil, fmt.Errorf("fail client load") }) cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) estimator := initializeEstimator(ctx, t, rwFailLoader, cfg, logger.Test(t)) // Call calculatePrice and expect an error // Ensure the price remains unchanged require.Error(t, estimator.calculatePrice(ctx), "Expected error when getting client fails") - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, defaultPrice, estimator.BaseComputeUnitPrice(), "Price should remain at default when client fails") }) } func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { // helpers vars for tests - min := uint64(100) - max := uint64(100_000) + minPrice := uint64(100) + maxPrice := uint64(100_000) depth := uint64(3) defaultPrice := uint64(100) pollPeriod := 3 * time.Second @@ -220,12 +220,12 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { t.Run("Successful Estimation", func(t *testing.T) { // Setup cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Calculated avg price should be equal to the one extracted manually from the blocks. require.NoError(t, estimator.calculatePrice(ctx)) - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, uint64(multipleBlocksAvg), estimator.BaseComputeUnitPrice()) }) @@ -233,7 +233,7 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { // Setup cfg := cfgmock.NewConfig(t) tmpMin := uint64(multipleBlocksAvg) + 100 // Set min higher than the avg price - setupConfigMock(cfg, defaultPrice, tmpMin, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, tmpMin, pollPeriod, depth) estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Compute unit price should be floored at min @@ -246,13 +246,13 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { // Setup cfg := cfgmock.NewConfig(t) tmpMax := uint64(multipleBlocksAvg) - 100 // Set tmpMax lower than the avg price - setupConfigMock(cfg, defaultPrice, min, tmpMax, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Compute unit price should be capped at max require.NoError(t, estimator.calculatePrice(ctx), "Failed to calculate price with price above max") cfg.On("ComputeUnitPriceMax").Return(tmpMax) - cfg.On("ComputeUnitPriceMin").Return(min) + cfg.On("ComputeUnitPriceMin").Return(minPrice) assert.Equal(t, tmpMax, estimator.BaseComputeUnitPrice(), "Price should be capped at max") }) @@ -264,12 +264,12 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { return nil, fmt.Errorf("fail client load") }) cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) estimator := initializeEstimator(ctx, t, rwFailLoader, cfg, logger.Test(t)) // Price should remain unchanged require.Error(t, estimator.calculatePrice(ctx), "Expected error when getting client fails") - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, defaultPrice, estimator.BaseComputeUnitPrice()) }) @@ -280,13 +280,13 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { return rw, nil }) cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) rw.On("SlotHeight", mock.Anything).Return(uint64(0), fmt.Errorf("failed to get current slot")) // Mock SlotHeight returning error estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Price should remain unchanged require.Error(t, estimator.calculatePrice(ctx), "Expected error when getting current slot fails") - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, defaultPrice, estimator.BaseComputeUnitPrice()) }) @@ -297,13 +297,13 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { return rw, nil }) cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) rw.On("SlotHeight", mock.Anything).Return(depth-1, nil) // Mock SlotHeight returning less than desiredBlockCount estimator := initializeEstimator(ctx, t, rwLoader, cfg, logger.Test(t)) // Price should remain unchanged require.Error(t, estimator.calculatePrice(ctx), "Expected error when current slot is less than desired block count") - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, defaultPrice, estimator.BaseComputeUnitPrice()) }) @@ -314,7 +314,7 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { return rw, nil }) cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) rw.On("SlotHeight", mock.Anything).Return(testSlots[len(testSlots)-1], nil) rw.On("GetBlocksWithLimit", mock.Anything, mock.Anything, mock.Anything). Return(nil, fmt.Errorf("failed to get blocks with limit")) // Mock GetBlocksWithLimit returning error @@ -322,7 +322,7 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { // Price should remain unchanged require.Error(t, estimator.calculatePrice(ctx), "Expected error when getting blocks with limit fails") - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, defaultPrice, estimator.BaseComputeUnitPrice()) }) @@ -333,7 +333,7 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { return rw, nil }) cfg := cfgmock.NewConfig(t) - setupConfigMock(cfg, defaultPrice, min, max, pollPeriod, depth) + setupConfigMock(cfg, defaultPrice, minPrice, pollPeriod, depth) rw.On("SlotHeight", mock.Anything).Return(testSlots[len(testSlots)-1], nil) emptyBlocks := rpc.BlocksResult{} // No blocks with compute unit prices rw.On("GetBlocksWithLimit", mock.Anything, mock.Anything, mock.Anything). @@ -342,15 +342,15 @@ func TestBlockHistoryEstimator_MultipleBlocks(t *testing.T) { // Price should remain unchanged require.EqualError(t, estimator.calculatePrice(ctx), errNoComputeUnitPriceCollected.Error(), "Expected error when no compute unit prices are collected") - cfg.On("ComputeUnitPriceMax").Return(max) + cfg.On("ComputeUnitPriceMax").Return(maxPrice) assert.Equal(t, defaultPrice, estimator.BaseComputeUnitPrice()) }) } // setupConfigMock configures the Config mock with necessary return values. -func setupConfigMock(cfg *cfgmock.Config, defaultPrice uint64, min, max uint64, pollPeriod time.Duration, depth uint64) { +func setupConfigMock(cfg *cfgmock.Config, defaultPrice uint64, minPrice uint64, pollPeriod time.Duration, depth uint64) { cfg.On("ComputeUnitPriceDefault").Return(defaultPrice).Once() - cfg.On("ComputeUnitPriceMin").Return(min).Once() + cfg.On("ComputeUnitPriceMin").Return(minPrice).Once() cfg.On("BlockHistoryPollPeriod").Return(pollPeriod).Once() cfg.On("BlockHistorySize").Return(depth) } diff --git a/pkg/solana/fees/fixed_price.go b/pkg/solana/fees/fixed_price.go index 3a748c301..a8c4cdee3 100644 --- a/pkg/solana/fees/fixed_price.go +++ b/pkg/solana/fees/fixed_price.go @@ -14,10 +14,10 @@ type fixedPriceEstimator struct { } func NewFixedPriceEstimator(cfg config.Config) (Estimator, error) { - defaultPrice, min, max := cfg.ComputeUnitPriceDefault(), cfg.ComputeUnitPriceMin(), cfg.ComputeUnitPriceMax() + defaultPrice, minPrice, maxPrice := cfg.ComputeUnitPriceDefault(), cfg.ComputeUnitPriceMin(), cfg.ComputeUnitPriceMax() - if defaultPrice < min || defaultPrice > max { - return nil, fmt.Errorf("default price (%d) is not within the min (%d) and max (%d) price bounds", defaultPrice, min, max) + if defaultPrice < minPrice || defaultPrice > maxPrice { + return nil, fmt.Errorf("default price (%d) is not within the min (%d) and max (%d) price bounds", defaultPrice, minPrice, maxPrice) } return &fixedPriceEstimator{ diff --git a/pkg/solana/fees/utils.go b/pkg/solana/fees/utils.go index 652fc5039..487928231 100644 --- a/pkg/solana/fees/utils.go +++ b/pkg/solana/fees/utils.go @@ -8,7 +8,7 @@ import ( ) // returns new fee based on number of times bumped -func CalculateFee(base, max, min uint64, count uint) uint64 { +func CalculateFee(base, maxFee, minFee uint64, count uint) uint64 { amount := base for i := uint(0); i < count; i++ { @@ -18,7 +18,7 @@ func CalculateFee(base, max, min uint64, count uint) uint64 { next := amount + amount if next <= amount { // overflowed - amount = max + amount = maxFee break } amount = next @@ -26,11 +26,11 @@ func CalculateFee(base, max, min uint64, count uint) uint64 { } // respect bounds - if amount < min { - return min + if amount < minFee { + return minFee } - if amount > max { - return max + if amount > maxFee { + return maxFee } return amount } diff --git a/pkg/solana/logpoller/parser.go b/pkg/solana/logpoller/parser.go index dc9a36e29..aad71a875 100644 --- a/pkg/solana/logpoller/parser.go +++ b/pkg/solana/logpoller/parser.go @@ -90,11 +90,9 @@ func (v *pgDSLParser) Timestamp(prim primitives.Timestamp) { return } - var tm int64 + tm := int64(prim.Timestamp) //nolint:gosec // disable G115 if prim.Timestamp > math.MaxInt64 { tm = 0 - } else { - tm = int64(prim.Timestamp) } v.expression = fmt.Sprintf( diff --git a/pkg/solana/logpoller/worker.go b/pkg/solana/logpoller/worker.go index 0e7d31df0..98226452f 100644 --- a/pkg/solana/logpoller/worker.go +++ b/pkg/solana/logpoller/worker.go @@ -300,8 +300,8 @@ type queue[T any] struct { values []T } -func newQueue[T any](len uint) *queue[T] { - values := make([]T, len) +func newQueue[T any](maxLen uint) *queue[T] { + values := make([]T, maxLen) return &queue[T]{ values: values, diff --git a/pkg/solana/report_test.go b/pkg/solana/report_test.go index 84452dada..0c419eba3 100644 --- a/pkg/solana/report_test.go +++ b/pkg/solana/report_test.go @@ -142,12 +142,11 @@ func TestMedianFromReport(t *testing.T) { tt = append(tt, test) } - for _, tc := range tt { - tc := tc - t.Run(tc.name, func(t *testing.T) { + for idx := range tt { + t.Run(tt[idx].name, func(t *testing.T) { ctx := tests.Context(t) var pos []median.ParsedAttributedObservation - for i, obs := range tc.obs { + for i, obs := range tt[idx].obs { pos = append(pos, median.ParsedAttributedObservation{ Value: obs, JuelsPerFeeCoin: obs, @@ -156,15 +155,15 @@ func TestMedianFromReport(t *testing.T) { } report, err := cdc.BuildReport(ctx, pos) require.NoError(t, err) - max, err := cdc.MaxReportLength(ctx, len(tc.obs)) + maxLen, err := cdc.MaxReportLength(ctx, len(tt[idx].obs)) require.NoError(t, err) - assert.Equal(t, len(report), max) + assert.Equal(t, len(report), maxLen) med, err := cdc.MedianFromReport(ctx, report) require.NoError(t, err) - assert.Equal(t, tc.expectedMedian.String(), med.String()) + assert.Equal(t, tt[idx].expectedMedian.String(), med.String()) count, err := cdc.ObserversCountFromReport(report) require.NoError(t, err) - assert.Equal(t, len(tc.obs), int(count)) + assert.Equal(t, len(tt[idx].obs), int(count)) }) } } diff --git a/pkg/solana/txm/pendingtx_test.go b/pkg/solana/txm/pendingtx_test.go index a79f9f7aa..d04cf883f 100644 --- a/pkg/solana/txm/pendingtx_test.go +++ b/pkg/solana/txm/pendingtx_test.go @@ -1303,17 +1303,16 @@ func TestPendingTxContext_ListAllExpiredBroadcastedTxs(t *testing.T) { }, } - for _, tt := range tests { - tt := tt // capture range variable - t.Run(tt.name, func(t *testing.T) { + for idx := range tests { + t.Run(tests[idx].name, func(t *testing.T) { // Initialize a new PendingTxContext ctx := newPendingTxContext() // Setup the test case - tt.setup(t, ctx) + tests[idx].setup(t, ctx) // Execute the function under test - result := ctx.ListAllExpiredBroadcastedTxs(tt.currBlockHeight) + result := ctx.ListAllExpiredBroadcastedTxs(tests[idx].currBlockHeight) // Extract the IDs from the result var resultIDs []string @@ -1322,7 +1321,7 @@ func TestPendingTxContext_ListAllExpiredBroadcastedTxs(t *testing.T) { } // Assert that the expected IDs match the result IDs (order does not matter) - assert.ElementsMatch(t, tt.expectedTxIDs, resultIDs) + assert.ElementsMatch(t, tests[idx].expectedTxIDs, resultIDs) }) } }