diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index df74a2d0d2..579a3f9e36 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -27,7 +27,7 @@ jobs: **/**.go go.mod go.sum - - uses: golangci/golangci-lint-action@v3 + - uses: golangci/golangci-lint-action@v4 with: # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. version: v1.55 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 99d5adac12..5adbc8afa6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -187,7 +187,7 @@ jobs: sed -i.bak "/$(echo $filename | sed 's/\//\\\//g')/d" coverage.txt done if: env.GIT_DIFF - - uses: codecov/codecov-action@v3.1.5 + - uses: codecov/codecov-action@v4.1.0 with: token: ${{ secrets.CODECOV_TOKEN }} file: ./coverage.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f26479fd6..709b043bdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,10 +49,14 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/crisis) [#1167](https://github.com/Finschia/finschia-sdk/pull/1167) Use `CacheContext()` in `AssertInvariants()` * (chore) [\#1168](https://github.com/Finschia/finschia-sdk/pull/1168) Replace `ExactArgs(0)` with `NoArgs()` in `x/upgrade` module * (server) [\#1175](https://github.com/Finschia/finschia-sdk/pull/1175) Use go embed for swagger +* (x/collection) [\#1287](https://github.com/Finschia/finschia-sdk/pull/1287) add nft id validation to MsgSendNFT ### Bug Fixes * chore(deps) [\#1141](https://github.com/Finschia/finschia-sdk/pull/1141) Bump github.com/cosmos/ledger-cosmos-go from 0.12.2 to 0.13.2 to fix ledger signing issue * (x/auth, x/slashing) [\#1179](https://github.com/Finschia/finschia-sdk/pull/1179) modify missing changes of converting to tendermint +* (x/auth) [#1274](https://github.com/Finschia/finschia-sdk/pull/1274) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. +* (x/collection) [\#1276](https://github.com/Finschia/finschia-sdk/pull/1276) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis +* (x/foundation) [\#1277](https://github.com/Finschia/finschia-sdk/pull/1277) add init logic of foundation module accounts to InitGenesis in order to eliminate potential panic * (x/collection) [\#1268](https://github.com/Finschia/finschia-sdk/pull/1268) export x/collection params into genesis ### Removed diff --git a/baseapp/block_gas_test.go b/baseapp/block_gas_test.go index c831fb21ba..933dde385a 100644 --- a/baseapp/block_gas_test.go +++ b/baseapp/block_gas_test.go @@ -100,7 +100,7 @@ func TestBaseApp_BlockGas(t *testing.T) { txBuilder.SetFeeAmount(feeAmount) txBuilder.SetGasLimit(txtypes.MaxGasWanted) // tx validation checks that gasLimit can't be bigger than this - privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{6}, []uint64{0} + privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{8}, []uint64{0} _, txBytes, err := createTestTx(encCfg.TxConfig, txBuilder, privs, accNums, accSeqs, ctx.ChainID()) require.NoError(t, err) diff --git a/x/auth/types/account.go b/x/auth/types/account.go index 52813e9999..d770f0dfaa 100644 --- a/x/auth/types/account.go +++ b/x/auth/types/account.go @@ -225,6 +225,10 @@ func (ma ModuleAccount) Validate() error { return errors.New("module account name cannot be blank") } + if ma.BaseAccount == nil { + return errors.New("uninitialized ModuleAccount: BaseAccount is nil") + } + if ma.Address != sdk.AccAddress(crypto.AddressHash([]byte(ma.Name))).String() { return fmt.Errorf("address %s cannot be derived from the module name '%s'", ma.Address, ma.Name) } diff --git a/x/auth/types/account_test.go b/x/auth/types/account_test.go index 0aed16c51c..a90a633051 100644 --- a/x/auth/types/account_test.go +++ b/x/auth/types/account_test.go @@ -7,7 +7,7 @@ import ( "testing" "github.com/stretchr/testify/require" - yaml "gopkg.in/yaml.v2" + "gopkg.in/yaml.v2" "github.com/Finschia/finschia-sdk/crypto/keys/secp256k1" "github.com/Finschia/finschia-sdk/testutil/testdata" @@ -207,3 +207,8 @@ func TestGenesisAccountsContains(t *testing.T) { genAccounts = append(genAccounts, acc) require.True(t, genAccounts.Contains(acc.GetAddress())) } + +func TestModuleAccountValidateNilBaseAccount(t *testing.T) { + ma := &types.ModuleAccount{Name: "foo"} + _ = ma.Validate() +} diff --git a/x/collection/genesis.go b/x/collection/genesis.go index ac08c34fe0..028101b3f7 100644 --- a/x/collection/genesis.go +++ b/x/collection/genesis.go @@ -64,8 +64,11 @@ func ValidateGenesis(data GenesisState) error { if len(contractNextTokenIDs.TokenIds) == 0 { return sdkerrors.ErrInvalidRequest.Wrap("next token ids cannot be empty") } - for _, nextTokenIDs := range contractNextTokenIDs.TokenIds { - if err := ValidateClassID(nextTokenIDs.ClassId); err != nil { + for _, nextTokenID := range contractNextTokenIDs.TokenIds { + if nextTokenID.Id.IsZero() { + return sdkerrors.ErrInvalidRequest.Wrap("nextTokenID.Id is not supposed to be zero") + } + if err := ValidateClassID(nextTokenID.ClassId); err != nil { return err } } diff --git a/x/collection/genesis_test.go b/x/collection/genesis_test.go index d4c7d0a7c1..4fcc543541 100644 --- a/x/collection/genesis_test.go +++ b/x/collection/genesis_test.go @@ -445,6 +445,17 @@ func TestValidateGenesis(t *testing.T) { }, false, }, + "should throw error when next token id is zero in genesis": { + &collection.GenesisState{ + Params: collection.Params{}, + NextTokenIds: []collection.ContractNextTokenIDs{ + {ContractId: "deadbeef", TokenIds: []collection.NextTokenID{ + {ClassId: "deadbeef", Id: sdk.NewUint(0)}, + }}, + }, + }, + false, + }, } for name, tc := range testCases { diff --git a/x/collection/msgs.go b/x/collection/msgs.go index 71f3c2ec9e..4c416335f0 100644 --- a/x/collection/msgs.go +++ b/x/collection/msgs.go @@ -340,7 +340,7 @@ func (m MsgSendNFT) ValidateBasic() error { return ErrEmptyField.Wrap("token ids cannot be empty") } for _, id := range m.TokenIds { - if err := ValidateTokenID(id); err != nil { + if err := ValidateNFTID(id); err != nil { return err } } diff --git a/x/collection/msgs_test.go b/x/collection/msgs_test.go index 9f8ebba60d..236629d1ab 100644 --- a/x/collection/msgs_test.go +++ b/x/collection/msgs_test.go @@ -259,6 +259,13 @@ func TestMsgSendNFT(t *testing.T) { ids: []string{""}, err: collection.ErrInvalidTokenID, }, + "FT ids": { + contractID: "deadbeef", + from: addrs[0], + to: addrs[1], + ids: []string{collection.NewFTID("deadbeef")}, + err: sdkerrors.ErrInvalidRequest.Wrapf("invalid id: %s", collection.NewFTID("deadbeef")), + }, } for name, tc := range testCases { diff --git a/x/foundation/keeper/internal/genesis.go b/x/foundation/keeper/internal/genesis.go index a88d6fa6a6..b5320033f6 100644 --- a/x/foundation/keeper/internal/genesis.go +++ b/x/foundation/keeper/internal/genesis.go @@ -1,6 +1,8 @@ package internal import ( + "fmt" + sdk "github.com/Finschia/finschia-sdk/types" "github.com/Finschia/finschia-sdk/x/foundation" ) @@ -48,6 +50,13 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data *foundation.GenesisState) erro k.SetPool(ctx, data.Pool) + // init module accounts just in case + if acc := k.authKeeper.GetModuleAccount(ctx, foundation.ModuleName); acc == nil { + panic(fmt.Sprintf("failed to create module account=%s", foundation.ModuleName)) + } + if acc := k.authKeeper.GetModuleAccount(ctx, foundation.TreasuryName); acc == nil { + panic(fmt.Sprintf("failed to create module account=%s", foundation.TreasuryName)) + } return nil } diff --git a/x/foundation/keeper/internal/genesis_test.go b/x/foundation/keeper/internal/genesis_test.go index 81d34636f3..fb7149c93f 100644 --- a/x/foundation/keeper/internal/genesis_test.go +++ b/x/foundation/keeper/internal/genesis_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" @@ -11,7 +12,9 @@ import ( "github.com/Finschia/finschia-sdk/simapp" "github.com/Finschia/finschia-sdk/testutil/testdata" sdk "github.com/Finschia/finschia-sdk/types" + authtypes "github.com/Finschia/finschia-sdk/x/auth/types" "github.com/Finschia/finschia-sdk/x/foundation" + "github.com/Finschia/finschia-sdk/x/foundation/keeper/internal" ) func workingPolicy() foundation.DecisionPolicy { @@ -285,3 +288,58 @@ func TestImportExportGenesis(t *testing.T) { require.Equal(t, tc.export, actual, name) } } + +func TestShouldPanicWhenFailToGenerateFoundationModuleAccountInInitGenesis(t *testing.T) { + checkTx := false + app := simapp.Setup(checkTx) + testdata.RegisterInterfaces(app.InterfaceRegistry()) + testdata.RegisterMsgServer(app.MsgServiceRouter(), testdata.MsgServerImpl{}) + gs := &foundation.GenesisState{ + Params: foundation.DefaultParams(), + Foundation: foundation.DefaultFoundation(), + } + ctx := app.BaseApp.NewContext(checkTx, tmproto.Header{}) + + testCases := map[string]struct { + mockAccKeeper *stubAccKeeper + }{ + "failed to generate module account=" + foundation.ModuleName: { + mockAccKeeper: &stubAccKeeper{nameToFail: foundation.ModuleName}, + }, + "failed to generate module account=" + foundation.TreasuryName: { + mockAccKeeper: &stubAccKeeper{nameToFail: foundation.TreasuryName}, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + assert.Panics(t, func() { + k := internal.NewKeeper( + app.AppCodec(), + app.GetKey(foundation.ModuleName), + app.MsgServiceRouter(), + tc.mockAccKeeper, + app.BankKeeper, + authtypes.FeeCollectorName, + foundation.DefaultConfig(), + foundation.DefaultAuthority().String(), + app.GetSubspace(foundation.ModuleName), + ) + + _ = k.InitGenesis(ctx, gs) + assert.FailNow(t, "not supposed to reach here, should panic before") + }) + }) + } +} + +type stubAccKeeper struct { + nameToFail string +} + +func (s *stubAccKeeper) GetModuleAccount(_ sdk.Context, name string) authtypes.ModuleAccountI { + if s.nameToFail == name { + return nil + } + return authtypes.NewEmptyModuleAccount("dontcare") +}