From e0ddf427ae77d09b382144c83d1ab73d20e50452 Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Fri, 13 Jan 2023 04:25:52 +0700 Subject: [PATCH 1/6] lint v5 --- .github/workflows/golangci.yaml | 27 ++++++++++++++ .golangci.yml | 66 +++++++++++++++++++++++++++++++++ router/keeper/genesis.go | 2 + router/keeper/keeper.go | 2 +- router/module_test.go | 17 ++++----- router/types/params.go | 1 - test/setup.go | 6 +-- 7 files changed, 107 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/golangci.yaml create mode 100644 .golangci.yml diff --git a/.github/workflows/golangci.yaml b/.github/workflows/golangci.yaml new file mode 100644 index 0000000..28daa82 --- /dev/null +++ b/.github/workflows/golangci.yaml @@ -0,0 +1,27 @@ +name: golangci-lint +on: + push: + tags: + - v* + branches: + - master + - main + pull_request: +permissions: + contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + # pull-requests: read +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/setup-go@v3 + with: + go-version: 1.19 + - uses: actions/checkout@v3 + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version + version: latest diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..e5b8c80 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,66 @@ +run: + tests: true + timeout: 10m + sort-results: true + allow-parallel-runners: true + exclude-dir: testutil/testdata_pulsar + concurrency: 4 + +linters: + disable-all: true + enable: + - depguard + - dogsled + - exportloopref + - goconst + - gocritic + - gofumpt + - gosec + - gosimple + - govet + - ineffassign + - misspell + - nakedret + - nolintlint + - staticcheck + - revive + - stylecheck + - typecheck + - unconvert + - unused + +issues: + exclude-rules: + - text: "Use of weak random number generator" + linters: + - gosec + - text: "ST1003:" + linters: + - stylecheck + # FIXME: Disabled until golangci-lint updates stylecheck with this fix: + # https://github.com/dominikh/go-tools/issues/389 + - text: "ST1016:" + linters: + - stylecheck + - path: "migrations" + text: "SA1019:" + linters: + - staticcheck + - text: "leading space" + linters: + - nolintlint + + max-issues-per-linter: 10000 + max-same-issues: 10000 + +linters-settings: + dogsled: + max-blank-identifiers: 3 + maligned: + # print struct with more effective memory layout or not, false by default + suggest-new: true + nolintlint: + allow-unused: false + allow-leading-space: true + require-explanation: false + require-specific: false diff --git a/router/keeper/genesis.go b/router/keeper/genesis.go index 99f3f27..e08848b 100644 --- a/router/keeper/genesis.go +++ b/router/keeper/genesis.go @@ -12,6 +12,8 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) { // Initialize store refund path for forwarded packets in genesis state that have not yet been acked. store := ctx.KVStore(k.storeKey) for key, value := range state.InFlightPackets { + key := key + value := value bz := k.cdc.MustMarshal(&value) store.Set([]byte(key), bz) } diff --git a/router/keeper/keeper.go b/router/keeper/keeper.go index b7037c0..aacadfa 100644 --- a/router/keeper/keeper.go +++ b/router/keeper/keeper.go @@ -363,7 +363,7 @@ func (k Keeper) RetryTimeout( denom := transfertypes.ParseDenomTrace(data.Denom).IBCDenom() - var token = sdk.NewCoin(denom, amount) + token := sdk.NewCoin(denom, amount) // srcPacket and srcPacketSender are empty because inFlightPacket is non-nil. return k.ForwardTransferPacket( diff --git a/router/module_test.go b/router/module_test.go index 1edfc36..646aaa9 100644 --- a/router/module_test.go +++ b/router/module_test.go @@ -6,7 +6,6 @@ import ( "testing" sdk "github.com/cosmos/cosmos-sdk/types" - apptypes "github.com/cosmos/ibc-go/v5/modules/apps/transfer/types" transfertypes "github.com/cosmos/ibc-go/v5/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types" "github.com/golang/mock/gomock" @@ -171,10 +170,10 @@ func TestOnRecvPacket_ForwardNoFee(t *testing.T) { routerModule := setup.RouterModule // Test data - hostAddr := "cosmos1vzxkv3lxccnttr9rs0002s93sgw72h7ghukuhs" - destAddr := "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k" - port := "transfer" - channel := "channel-0" + hostAddr := "cosmos1vzxkv3lxccnttr9rs0002s93sgw72h7ghukuhs" //nolint:goconst + destAddr := "cosmos16plylpsgxechajltx9yeseqexzdzut9g8vla4k" //nolint:goconst + port := "transfer" //nolint:goconst + channel := "channel-0" //nolint:goconst denom := makeIBCDenom(testDestinationPort, testDestinationChannel, testDenom) senderAccAddr := test.AccAddress() testCoin := sdk.NewCoin(denom, sdk.NewInt(100)) @@ -206,7 +205,7 @@ func TestOnRecvPacket_ForwardNoFee(t *testing.T) { keeper.DefaultTransferPacketTimeoutHeight, uint64(ctx.BlockTime().UnixNano())+uint64(keeper.DefaultForwardTransferPacketTimeoutTimestamp.Nanoseconds()), ), - ).Return(&apptypes.MsgTransferResponse{Sequence: 0}, nil), + ).Return(&transfertypes.MsgTransferResponse{Sequence: 0}, nil), setup.Mocks.IBCModuleMock.EXPECT().OnAcknowledgementPacket(ctx, packetFwd, successAck, senderAccAddr). Return(nil), @@ -276,7 +275,7 @@ func TestOnRecvPacket_ForwardWithFee(t *testing.T) { keeper.DefaultTransferPacketTimeoutHeight, uint64(ctx.BlockTime().UnixNano())+uint64(keeper.DefaultForwardTransferPacketTimeoutTimestamp.Nanoseconds()), ), - ).Return(&apptypes.MsgTransferResponse{Sequence: 0}, nil), + ).Return(&transfertypes.MsgTransferResponse{Sequence: 0}, nil), setup.Mocks.IBCModuleMock.EXPECT().OnAcknowledgementPacket(ctx, packetFwd, successAck, senderAccAddr). Return(nil), @@ -369,7 +368,7 @@ func TestOnRecvPacket_ForwardMultihop(t *testing.T) { setup.Mocks.TransferKeeperMock.EXPECT().Transfer( sdk.WrapSDKContext(ctx), msgTransfer1, - ).Return(&apptypes.MsgTransferResponse{Sequence: 0}, nil), + ).Return(&transfertypes.MsgTransferResponse{Sequence: 0}, nil), setup.Mocks.IBCModuleMock.EXPECT().OnRecvPacket(ctx, packet2, senderAccAddr2). Return(acknowledgement), @@ -377,7 +376,7 @@ func TestOnRecvPacket_ForwardMultihop(t *testing.T) { setup.Mocks.TransferKeeperMock.EXPECT().Transfer( sdk.WrapSDKContext(ctx), msgTransfer2, - ).Return(&apptypes.MsgTransferResponse{Sequence: 0}, nil), + ).Return(&transfertypes.MsgTransferResponse{Sequence: 0}, nil), setup.Mocks.IBCModuleMock.EXPECT().OnAcknowledgementPacket(ctx, packetFwd, successAck, senderAccAddr2). Return(nil), diff --git a/router/types/params.go b/router/types/params.go index 894f77e..8884cfe 100644 --- a/router/types/params.go +++ b/router/types/params.go @@ -49,7 +49,6 @@ func validateFeePercentage(i interface{}) error { } if v.IsNegative() { return fmt.Errorf("invalid fee percentage. expected not negative, got %d", v.RoundInt64()) - } if !(v.LTE(sdk.OneDec())) { return fmt.Errorf("invalid fee percentage. expected less than one 1 got %d", v.RoundInt64()) diff --git a/test/setup.go b/test/setup.go index 320bd39..fbb5468 100644 --- a/test/setup.go +++ b/test/setup.go @@ -22,7 +22,7 @@ import ( tmdb "github.com/tendermint/tm-db" ) -func NewTestSetup(t *testing.T, ctl *gomock.Controller) *TestSetup { +func NewTestSetup(t *testing.T, ctl *gomock.Controller) *Setup { initializer := newInitializer() transferKeeperMock := mock.NewMockTransferKeeper(ctl) @@ -39,7 +39,7 @@ func NewTestSetup(t *testing.T, ctl *gomock.Controller) *TestSetup { routerKeeper.SetParams(initializer.Ctx, types.DefaultParams()) - return &TestSetup{ + return &Setup{ Initializer: initializer, Keepers: &testKeepers{ @@ -58,7 +58,7 @@ func NewTestSetup(t *testing.T, ctl *gomock.Controller) *TestSetup { } } -type TestSetup struct { +type Setup struct { Initializer initializer Keepers *testKeepers From 020def79811aa84f466f06a3de8bac73ab2fb24d Mon Sep 17 00:00:00 2001 From: Jacob Gadikian Date: Wed, 8 Feb 2023 21:02:56 +0700 Subject: [PATCH 2/6] still need to fix errors --- router/module_test.go | 4 ++-- router/types/keys.go | 8 +++++--- test/setup.go | 1 - 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/router/module_test.go b/router/module_test.go index 8f993c5..5f505c3 100644 --- a/router/module_test.go +++ b/router/module_test.go @@ -379,7 +379,7 @@ func TestOnRecvPacket_ForwardMultihopStringNext(t *testing.T) { setup.Mocks.TransferKeeperMock.EXPECT().Transfer( sdk.WrapSDKContext(ctx), msgTransfer1, - ).Return(&apptypes.MsgTransferResponse{Sequence: 0}, nil), + ).Return(&transfertypes.MsgTransferResponse{Sequence: 0}, nil), setup.Mocks.IBCModuleMock.EXPECT().OnRecvPacket(ctx, packet2, senderAccAddr2). Return(acknowledgement), @@ -387,7 +387,7 @@ func TestOnRecvPacket_ForwardMultihopStringNext(t *testing.T) { setup.Mocks.TransferKeeperMock.EXPECT().Transfer( sdk.WrapSDKContext(ctx), msgTransfer2, - ).Return(&apptypes.MsgTransferResponse{Sequence: 0}, nil), + ).Return(&transfertypes.MsgTransferResponse{Sequence: 0}, nil), setup.Mocks.IBCModuleMock.EXPECT().OnAcknowledgementPacket(ctx, packetFwd, successAck, senderAccAddr2). Return(nil), diff --git a/router/types/keys.go b/router/types/keys.go index 7c8c1c4..3634eb6 100644 --- a/router/types/keys.go +++ b/router/types/keys.go @@ -16,9 +16,11 @@ const ( QuerierRoute = ModuleName ) -type NonrefundableKey struct{} -type DisableDenomCompositionKey struct{} -type ProcessedKey struct{} +type ( + NonrefundableKey struct{} + DisableDenomCompositionKey struct{} + ProcessedKey struct{} +) func RefundPacketKey(channelID, portID string, sequence uint64) []byte { return []byte(fmt.Sprintf("%s/%s/%d", channelID, portID, sequence)) diff --git a/test/setup.go b/test/setup.go index 0676325..836b451 100644 --- a/test/setup.go +++ b/test/setup.go @@ -35,7 +35,6 @@ func NewTestSetup(t *testing.T, ctl *gomock.Controller) *Setup { paramsKeeper := initializer.paramsKeeper() routerKeeper := initializer.routerKeeper(paramsKeeper, transferKeeperMock, channelKeeperMock, distributionKeeperMock, bankKeeperMock, ics4WrapperMock) - //routerModule := initializer.routerModule(routerKeeper) require.NoError(t, initializer.StateStore.LoadLatestVersion()) From c7951979bf42b97a257ee5c3a5d760a9fac8c9ea Mon Sep 17 00:00:00 2001 From: hoangdv2429 Date: Thu, 9 Feb 2023 00:13:58 +0700 Subject: [PATCH 3/6] remove uses of deprecated --- router/ibc_middleware.go | 3 ++- router/keeper/keeper.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/router/ibc_middleware.go b/router/ibc_middleware.go index 79fc5eb..59d2e65 100644 --- a/router/ibc_middleware.go +++ b/router/ibc_middleware.go @@ -6,6 +6,7 @@ import ( "strings" "time" + errorsmod "cosmossdk.io/errors" "github.com/armon/go-metrics" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -253,7 +254,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( var ack channeltypes.Acknowledgement if err := channeltypes.SubModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil { - return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err) + return errorsmod.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err) } inFlightPacket := im.keeper.GetAndClearInFlightPacket(ctx, packet.SourceChannel, packet.SourcePort, packet.Sequence) diff --git a/router/keeper/keeper.go b/router/keeper/keeper.go index 75fff5d..5a1b59f 100644 --- a/router/keeper/keeper.go +++ b/router/keeper/keeper.go @@ -6,6 +6,7 @@ import ( "strings" "time" + errorsmod "cosmossdk.io/errors" "github.com/armon/go-metrics" "github.com/cosmos/cosmos-sdk/codec" storetypes "github.com/cosmos/cosmos-sdk/store/types" @@ -224,7 +225,7 @@ func (k *Keeper) ForwardTransferPacket( k.Logger(ctx).Error("packetForwardMiddleware error funding community pool", "error", err, ) - return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } } @@ -245,7 +246,7 @@ func (k *Keeper) ForwardTransferPacket( k.Logger(ctx).Error("packetForwardMiddleware error marshaling next as JSON", "error", err, ) - return sdkerrors.Wrapf(sdkerrors.ErrJSONMarshal, err.Error()) + return errorsmod.Wrapf(sdkerrors.ErrJSONMarshal, err.Error()) } msgTransfer.Memo = string(memoBz) } From 10e5fc49b1f10d92746393608b17908480f48459 Mon Sep 17 00:00:00 2001 From: hoangdv2429 Date: Thu, 9 Feb 2023 00:52:17 +0700 Subject: [PATCH 4/6] go.mod --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index bcc747d..119954a 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ go 1.18 module github.com/strangelove-ventures/packet-forward-middleware/v5 require ( + cosmossdk.io/errors v1.0.0-beta.7 github.com/armon/go-metrics v0.4.0 github.com/cosmos/cosmos-sdk v0.46.4 github.com/cosmos/ibc-go/v5 v5.1.0 @@ -21,7 +22,6 @@ require ( ) require ( - cosmossdk.io/errors v1.0.0-beta.7 // indirect cosmossdk.io/math v1.0.0-beta.3 // indirect filippo.io/edwards25519 v1.0.0-rc.1 // indirect github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect From b6a697b09d3ed746998225c98919d62c2f71b404 Mon Sep 17 00:00:00 2001 From: hoangdv2429 Date: Thu, 9 Feb 2023 01:28:21 +0700 Subject: [PATCH 5/6] miss one left --- router/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/keeper/keeper.go b/router/keeper/keeper.go index 5a1b59f..e771b64 100644 --- a/router/keeper/keeper.go +++ b/router/keeper/keeper.go @@ -269,7 +269,7 @@ func (k *Keeper) ForwardTransferPacket( "amount", packetCoin.Amount.String(), "denom", packetCoin.Denom, "error", err, ) - return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } // Store the following information in keeper: From 64e9b4960a03c8a238fb4ea92677f74981df2911 Mon Sep 17 00:00:00 2001 From: hoangdv2429 Date: Thu, 9 Feb 2023 01:31:53 +0700 Subject: [PATCH 6/6] Wrap NOT Wrapf --- router/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/keeper/keeper.go b/router/keeper/keeper.go index e771b64..099d73a 100644 --- a/router/keeper/keeper.go +++ b/router/keeper/keeper.go @@ -102,7 +102,7 @@ func (k *Keeper) WriteAcknowledgementForForwardedPacket( // Lookup module by channel capability _, cap, err := k.channelKeeper.LookupModuleByChannel(ctx, inFlightPacket.RefundPortId, inFlightPacket.RefundChannelId) if err != nil { - return sdkerrors.Wrap(err, "could not retrieve module from port-id") + return errorsmod.Wrap(err, "could not retrieve module from port-id") } // for forwarded packets, the funds were moved into an escrow account if the denom originated on this chain.