Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: fix remaining golangci-lint issues #10972

Merged
merged 1 commit into from
Oct 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion core/chains/cosmos/cosmostxm/txm.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"cmp"
"context"
"encoding/hex"
"fmt"
"slices"
"strings"
"time"
Expand Down Expand Up @@ -327,7 +328,13 @@ func (txm *Txm) sendMsgBatchFromAddress(ctx context.Context, gasPrice sdk.DecCoi
// Assume transient api issue and retry.
return err
}
timeoutHeight := uint64(lb.Block.Header.Height) + uint64(txm.cfg.BlocksUntilTxTimeout())
header, timeout := lb.SdkBlock.Header.Height, txm.cfg.BlocksUntilTxTimeout()
if header < 0 {
return fmt.Errorf("invalid negative header height: %d", header)
} else if timeout < 0 {
return fmt.Errorf("invalid negative blocks until tx timeout: %d", timeout)
}
timeoutHeight := uint64(header) + uint64(timeout)
signedTx, err := tc.CreateAndSign(simResults.Succeeded.GetMsgs(), an, sn, gasLimit, txm.cfg.GasLimitMultiplier(),
gasPrice, NewKeyWrapper(txm.keystoreAdapter, sender.String()), timeoutHeight)
if err != nil {
Expand Down
17 changes: 8 additions & 9 deletions core/chains/cosmos/cosmostxm/txm_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"time"

wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
tmtypes "github.com/cometbft/cometbft/proto/tendermint/types"
tmservicetypes "github.com/cosmos/cosmos-sdk/client/grpc/tmservice"
cosmostypes "github.com/cosmos/cosmos-sdk/types"
txtypes "github.com/cosmos/cosmos-sdk/types/tx"
Expand Down Expand Up @@ -112,8 +111,8 @@ func TestTxm(t *testing.T) {
tc.On("SimulateUnsigned", mock.Anything, mock.Anything).Return(&txtypes.SimulateResponse{GasInfo: &cosmostypes.GasInfo{
GasUsed: 1_000_000,
}}, nil)
tc.On("LatestBlock").Return(&tmservicetypes.GetLatestBlockResponse{Block: &tmtypes.Block{
Header: tmtypes.Header{Height: 1},
tc.On("LatestBlock").Return(&tmservicetypes.GetLatestBlockResponse{SdkBlock: &tmservicetypes.Block{
Header: tmservicetypes.Header{Height: 1},
}}, nil)
tc.On("CreateAndSign", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte{0x01}, nil)

Expand Down Expand Up @@ -168,8 +167,8 @@ func TestTxm(t *testing.T) {
tc.On("SimulateUnsigned", mock.Anything, mock.Anything).Return(&txtypes.SimulateResponse{GasInfo: &cosmostypes.GasInfo{
GasUsed: 1_000_000,
}}, nil).Once()
tc.On("LatestBlock").Return(&tmservicetypes.GetLatestBlockResponse{Block: &tmtypes.Block{
Header: tmtypes.Header{Height: 1},
tc.On("LatestBlock").Return(&tmservicetypes.GetLatestBlockResponse{SdkBlock: &tmservicetypes.Block{
Header: tmservicetypes.Header{Height: 1},
}}, nil).Once()
tc.On("CreateAndSign", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte{0x01}, nil).Once()
txResp := &cosmostypes.TxResponse{TxHash: "4BF5122F344554C53BDE2EBB8CD2B7E3D1600AD631C385A5D7CCE23C7785459A"}
Expand Down Expand Up @@ -227,8 +226,8 @@ func TestTxm(t *testing.T) {
tc.On("SimulateUnsigned", mock.Anything, mock.Anything).Return(&txtypes.SimulateResponse{GasInfo: &cosmostypes.GasInfo{
GasUsed: 1_000_000,
}}, nil).Once()
tc.On("LatestBlock").Return(&tmservicetypes.GetLatestBlockResponse{Block: &tmtypes.Block{
Header: tmtypes.Header{Height: 1},
tc.On("LatestBlock").Return(&tmservicetypes.GetLatestBlockResponse{SdkBlock: &tmservicetypes.Block{
Header: tmservicetypes.Header{Height: 1},
}}, nil).Once()
tc.On("CreateAndSign", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte{0x01}, nil).Once()
}
Expand Down Expand Up @@ -360,8 +359,8 @@ func TestTxm(t *testing.T) {
tc.On("SimulateUnsigned", mock.Anything, mock.Anything).Return(&txtypes.SimulateResponse{GasInfo: &cosmostypes.GasInfo{
GasUsed: 1_000_000,
}}, nil)
tc.On("LatestBlock").Return(&tmservicetypes.GetLatestBlockResponse{Block: &tmtypes.Block{
Header: tmtypes.Header{Height: 1},
tc.On("LatestBlock").Return(&tmservicetypes.GetLatestBlockResponse{SdkBlock: &tmservicetypes.Block{
Header: tmservicetypes.Header{Height: 1},
}}, nil)
tc.On("CreateAndSign", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]byte{0x01}, nil)
txResp := &cosmostypes.TxResponse{TxHash: "4BF5122F344554C53BDE2EBB8CD2B7E3D1600AD631C385A5D7CCE23C7785459A"}
Expand Down
4 changes: 2 additions & 2 deletions core/chains/evm/log/broadcaster.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func NewBroadcaster(orm ORM, ethClient evmclient.Client, config Config, lggr log

func (b *broadcaster) Start(context.Context) error {
return b.StartOnce("LogBroadcaster", func() error {
b.wgDone.Add(2)
b.wgDone.Add(1)
go b.awaitInitialSubscribers()
b.mailMon.Monitor(b.changeSubscriberStatus, "LogBroadcaster", "ChangeSubscriber", b.evmChainID.String())
return nil
Expand Down Expand Up @@ -234,11 +234,11 @@ func (b *broadcaster) awaitInitialSubscribers() {
case <-b.DependentAwaiter.AwaitDependents():
// ensure that any queued dependent subscriptions are registered first
b.onChangeSubscriberStatus()
b.wgDone.Add(1)
go b.startResubscribeLoop()
return

case <-b.chStop:
b.wgDone.Done() // because startResubscribeLoop won't be called
return
}
}
Expand Down
39 changes: 11 additions & 28 deletions core/chains/evm/log/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,10 @@ type broadcasterHelper struct {
pipelineHelper cltest.JobPipelineV2TestHelper
}

func newBroadcasterHelper(t *testing.T, blockHeight int64, timesSubscribe int, overridesFn func(*chainlink.Config, *chainlink.Secrets)) *broadcasterHelper {
return broadcasterHelperCfg{}.new(t, blockHeight, timesSubscribe, nil, overridesFn)
}

type broadcasterHelperCfg struct {
highestSeenHead *evmtypes.Head
db *sqlx.DB
}
Comment on lines -65 to -68
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was awkward and barely used any longer, so I simplified it away. The db field was no cost, but the tradeoff for highestSeenHead is to pass some nils instead 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't love required args with nil values, but the surface is small and only in tests so maybe ok.

l like XXXcfg.New() (*XXX) even less...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I thought the same but didn't stress since it's not even exported

func newBroadcasterHelper(t *testing.T, blockHeight int64, timesSubscribe int, filterLogsResult []types.Log, overridesFn func(*chainlink.Config, *chainlink.Secrets)) *broadcasterHelper {
// ensure we check before registering any mock Cleanup assertions
testutils.SkipShortDB(t)

func (c broadcasterHelperCfg) new(t *testing.T, blockHeight int64, timesSubscribe int, filterLogsResult []types.Log, overridesFn func(*chainlink.Config, *chainlink.Secrets)) *broadcasterHelper {
if c.db == nil {
// ensure we check before registering any mock Cleanup assertions
testutils.SkipShortDB(t)
}
expectedCalls := mockEthClientExpectedCalls{
SubscribeFilterLogs: timesSubscribe,
HeaderByNumber: 1,
Expand All @@ -81,21 +71,13 @@ func (c broadcasterHelperCfg) new(t *testing.T, blockHeight int64, timesSubscrib

chchRawLogs := make(chan evmtest.RawSub[types.Log], timesSubscribe)
mockEth := newMockEthClient(t, chchRawLogs, blockHeight, expectedCalls)
helper := c.newWithEthClient(t, mockEth.EthClient, overridesFn)
helper := newBroadcasterHelperWithEthClient(t, mockEth.EthClient, nil, overridesFn)
helper.chchRawLogs = chchRawLogs
helper.mockEth = mockEth
return helper
}

func newBroadcasterHelperWithEthClient(t *testing.T, ethClient evmclient.Client, highestSeenHead *evmtypes.Head, overridesFn func(*chainlink.Config, *chainlink.Secrets)) *broadcasterHelper {
return broadcasterHelperCfg{highestSeenHead: highestSeenHead}.newWithEthClient(t, ethClient, overridesFn)
}

func (c broadcasterHelperCfg) newWithEthClient(t *testing.T, ethClient evmclient.Client, overridesFn func(*chainlink.Config, *chainlink.Secrets)) *broadcasterHelper {
if c.db == nil {
c.db = pgtest.NewSqlxDB(t)
}

globalConfig := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) {
c.Database.LogQueries = ptr(true)
finality := uint32(10)
Expand All @@ -109,25 +91,26 @@ func (c broadcasterHelperCfg) newWithEthClient(t *testing.T, ethClient evmclient
lggr := logger.TestLogger(t)
mailMon := srvctest.Start(t, utils.NewMailboxMonitor(t.Name()))

orm := log.NewORM(c.db, lggr, config.Database(), cltest.FixtureChainID)
lb := log.NewTestBroadcaster(orm, ethClient, config.EVM(), lggr, c.highestSeenHead, mailMon)
kst := cltest.NewKeyStore(t, c.db, globalConfig.Database())
db := pgtest.NewSqlxDB(t)
orm := log.NewORM(db, lggr, config.Database(), cltest.FixtureChainID)
lb := log.NewTestBroadcaster(orm, ethClient, config.EVM(), lggr, highestSeenHead, mailMon)
kst := cltest.NewKeyStore(t, db, globalConfig.Database())

cc := evmtest.NewChainRelayExtenders(t, evmtest.TestChainOpts{
Client: ethClient,
GeneralConfig: globalConfig,
DB: c.db,
DB: db,
KeyStore: kst.Eth(),
LogBroadcaster: &log.NullBroadcaster{},
MailMon: mailMon,
})
legacyChains := evmrelay.NewLegacyChainsFromRelayerExtenders(cc)
pipelineHelper := cltest.NewJobPipelineV2(t, config.WebServer(), config.JobPipeline(), config.Database(), legacyChains, c.db, kst, nil, nil)
pipelineHelper := cltest.NewJobPipelineV2(t, config.WebServer(), config.JobPipeline(), config.Database(), legacyChains, db, kst, nil, nil)

return &broadcasterHelper{
t: t,
lb: lb,
db: c.db,
db: db,
globalConfig: globalConfig,
config: config,
pipelineHelper: pipelineHelper,
Expand Down
Loading
Loading