diff --git a/docs/learn/advanced/00-baseapp.md b/docs/learn/advanced/00-baseapp.md index 15a9feb31cbb..f19c132994ae 100644 --- a/docs/learn/advanced/00-baseapp.md +++ b/docs/learn/advanced/00-baseapp.md @@ -354,7 +354,7 @@ The response contains: https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/x/auth/ante/basic.go#L102 ``` -* `Events ([]cmn.KVPair)`: Key-Value tags for filtering and indexing transactions (eg. by account). See [`event`s](./08-events.md) for more. +* `Events ([]cmn.KVPair)`: Key-Value tags for filtering and indexing transactions (eg. by account). See [`events`](./08-events.md) for more. * `Codespace (string)`: Namespace for the Code. #### RecheckTx @@ -495,7 +495,7 @@ Each transaction returns a response to the underlying consensus engine of type [ * `Info (string):` Additional information. May be non-deterministic. * `GasWanted (int64)`: Amount of gas requested for transaction. It is provided by users when they generate the transaction. * `GasUsed (int64)`: Amount of gas consumed by transaction. During transaction execution, this value is computed by multiplying the standard cost of a transaction byte by the size of the raw transaction, and by adding gas each time a read/write to the store occurs. -* `Events ([]cmn.KVPair)`: Key-Value tags for filtering and indexing transactions (eg. by account). See [`event`s](./08-events.md) for more. +* `Events ([]cmn.KVPair)`: Key-Value tags for filtering and indexing transactions (eg. by account). See [`events`](./08-events.md) for more. * `Codespace (string)`: Namespace for the Code. #### EndBlock diff --git a/docs/learn/advanced/08-events.md b/docs/learn/advanced/08-events.md index 2b8d754ac6a8..0c5772e325a6 100644 --- a/docs/learn/advanced/08-events.md +++ b/docs/learn/advanced/08-events.md @@ -4,7 +4,7 @@ sidebar_position: 1 # Events :::note Synopsis -`Event`s are objects that contain information about the execution of the application. They are mainly used by service providers like block explorers and wallet to track the execution of various messages and index transactions. +`Events` are objects that contain information about the execution of the application. They are mainly used by service providers like block explorers and wallet to track the execution of various messages and index transactions. ::: :::note Pre-requisite Readings diff --git a/store/CHANGELOG.md b/store/CHANGELOG.md index cdd7bb1c59fa..84b2a0d35eb3 100644 --- a/store/CHANGELOG.md +++ b/store/CHANGELOG.md @@ -23,6 +23,12 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog +## [Unreleased] + +### Bug Fixes + +* (store) [#20425](https://github.com/cosmos/cosmos-sdk/pull/20425) Fix nil pointer panic when query historical state where a new store don't exist. + ## v1.1.0 (March 20, 2024) ### Improvements diff --git a/store/cache/cache.go b/store/cache/cache.go index 98d17d0341ac..748eae8c422b 100644 --- a/store/cache/cache.go +++ b/store/cache/cache.go @@ -43,7 +43,7 @@ type ( func NewCommitKVStoreCache(store types.CommitKVStore, size uint) *CommitKVStoreCache { cache, err := lru.NewARC(int(size)) if err != nil { - panic(fmt.Errorf("failed to create KVStore cache: %s", err)) + panic(fmt.Errorf("failed to create KVStore cache: %w", err)) } return &CommitKVStoreCache{ diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index cfcc5f47b8b2..33589123bcd0 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -5,7 +5,7 @@ import ( "errors" "fmt" "io" - "math" + "math" "sort" "strings" "sync" @@ -607,6 +607,10 @@ func (rs *Store) CacheMultiStoreWithVersion(version int64) (types.CacheMultiStor if storeInfos[key.Name()] { return nil, err } + + // If the store donesn't exist at this version, create a dummy one to prevent + // nil pointer panic in newer query APIs. + cacheStore = dbadapter.Store{DB: dbm.NewMemDB()} } default: @@ -830,7 +834,7 @@ func (rs *Store) Snapshot(height uint64, protoWriter protoio.Writer) error { keys := keysFromStoreKeyMap(rs.stores) for _, key := range keys { switch store := rs.GetCommitKVStore(key).(type) { - case *iavl.Store: + case *iavl.Store: stores = append(stores, namedStore{name: key.Name(), Store: store}) case *transient.Store, *mem.Store: // Non-persisted stores shouldn't be snapshotted @@ -850,7 +854,7 @@ func (rs *Store) Snapshot(height uint64, protoWriter protoio.Writer) error { // are demarcated by new SnapshotStore items. for _, store := range stores { rs.logger.Debug("starting snapshot", "store", store.name, "height", height) - exporter, err := store.Export(int64(height)) + exporter, err := store.Export(int64(height)) if err != nil { rs.logger.Error("snapshot failed; exporter error", "store", store.name, "err", err) return err diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index 2702f3e08623..ebdd657de41f 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -100,16 +100,19 @@ func TestCacheMultiStoreWithVersion(t *testing.T) { require.Equal(t, kvStore.Get(k), v) // add new module stores (store4 and store5) to multi stores and commit - ms.MountStoreWithDB(types.NewKVStoreKey("store4"), types.StoreTypeIAVL, nil) - ms.MountStoreWithDB(types.NewKVStoreKey("store5"), types.StoreTypeIAVL, nil) + key4, key5 := types.NewKVStoreKey("store4"), types.NewKVStoreKey("store5") + ms.MountStoreWithDB(key4, types.StoreTypeIAVL, nil) + ms.MountStoreWithDB(key5, types.StoreTypeIAVL, nil) err = ms.LoadLatestVersionAndUpgrade(&types.StoreUpgrades{Added: []string{"store4", "store5"}}) require.NoError(t, err) ms.Commit() // cache multistore of version before adding store4 should works - _, err = ms.CacheMultiStoreWithVersion(1) + cms2, err := ms.CacheMultiStoreWithVersion(1) require.NoError(t, err) + require.Empty(t, cms2.GetKVStore(key4).Get([]byte("key"))) + // require we cannot commit (write) to a cache-versioned multi-store require.Panics(t, func() { kvStore.Set(k, []byte("newValue")) diff --git a/tests/e2e/accounts/base_account_test.go b/tests/e2e/accounts/base_account_test.go index 60292a3c52dd..64a6dff9a50a 100644 --- a/tests/e2e/accounts/base_account_test.go +++ b/tests/e2e/accounts/base_account_test.go @@ -80,5 +80,4 @@ func bechify(t *testing.T, app *simapp.SimApp, addr []byte) string { func fundAccount(t *testing.T, app *simapp.SimApp, ctx sdk.Context, addr sdk.AccAddress, amt string) { require.NoError(t, testutil.FundAccount(ctx, app.BankKeeper, addr, coins(t, amt))) - } diff --git a/testutil/testdata/grpc_query.go b/testutil/testdata/grpc_query.go index 1e5ae1830de9..1078f60b8b81 100644 --- a/testutil/testdata/grpc_query.go +++ b/testutil/testdata/grpc_query.go @@ -3,9 +3,10 @@ package testdata import ( "context" "fmt" - "github.com/cosmos/gogoproto/types/any/test" "testing" + "github.com/cosmos/gogoproto/types/any/test" + "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc" "gotest.tools/v3/assert" diff --git a/x/accounts/defaults/multisig/account.go b/x/accounts/defaults/multisig/account.go index 6ab25a8440d9..d6fadbfabff9 100644 --- a/x/accounts/defaults/multisig/account.go +++ b/x/accounts/defaults/multisig/account.go @@ -91,7 +91,10 @@ func (a *Account) Init(ctx context.Context, msg *v1.MsgInit) (*v1.MsgInitRespons return nil, err } - totalWeight += msg.Members[i].Weight + totalWeight, err = safeAdd(totalWeight, msg.Members[i].Weight) + if err != nil { + return nil, err + } } if err := validateConfig(*msg.Config, totalWeight); err != nil { @@ -279,6 +282,7 @@ func (a Account) ExecuteProposal(ctx context.Context, msg *v1.MsgExecuteProposal } totalWeight := yesVotes + noVotes + abstainVotes + var ( rejectErr error execErr error @@ -387,3 +391,14 @@ func (a *Account) RegisterQueryHandlers(builder *accountstd.QueryBuilder) { accountstd.RegisterQueryHandler(builder, a.QueryProposal) accountstd.RegisterQueryHandler(builder, a.QueryConfig) } + +func safeAdd(nums ...uint64) (uint64, error) { + var sum uint64 + for _, num := range nums { + if sum+num < sum { + return 0, errors.New("overflow") + } + sum += num + } + return sum, nil +} diff --git a/x/accounts/defaults/multisig/account_test.go b/x/accounts/defaults/multisig/account_test.go index ab95b5be4803..58d3e11a4457 100644 --- a/x/accounts/defaults/multisig/account_test.go +++ b/x/accounts/defaults/multisig/account_test.go @@ -2,6 +2,7 @@ package multisig import ( "context" + "math" "testing" "time" @@ -312,6 +313,29 @@ func TestUpdateConfig(t *testing.T) { }, }, }, + { + "change members, invalid weights", + &v1.MsgUpdateConfig{ + UpdateMembers: []*v1.Member{ + { + Address: "addr1", + Weight: math.MaxUint64, + }, + { + Address: "addr2", + Weight: 1, + }, + }, + Config: &v1.Config{ + Threshold: 666, + Quorum: 400, + VotingPeriod: 60, + }, + }, + "overflow", + nil, + nil, + }, } for _, tc := range testcases { @@ -658,3 +682,33 @@ func TestProposalPassing(t *testing.T) { require.Equal(t, expectedMembers, cfg.Members) } + +func TestWeightOverflow(t *testing.T) { + ctx, ss := newMockContext(t) + acc := setup(t, ctx, ss, nil) + + startAcc := &v1.MsgInit{ + Config: &v1.Config{ + Threshold: 2640, + Quorum: 2000, + VotingPeriod: 60, + }, + Members: []*v1.Member{ + { + Address: "addr1", + Weight: math.MaxUint64, + }, + }, + } + + _, err := acc.Init(ctx, startAcc) + require.NoError(t, err) + + // add another member with weight 1 to trigger an overflow + startAcc.Members = append(startAcc.Members, &v1.Member{ + Address: "addr2", + Weight: 1, + }) + _, err = acc.Init(ctx, startAcc) + require.ErrorContains(t, err, "overflow") +} diff --git a/x/accounts/defaults/multisig/update_config.go b/x/accounts/defaults/multisig/update_config.go index a1966c787b0b..d352d49926fa 100644 --- a/x/accounts/defaults/multisig/update_config.go +++ b/x/accounts/defaults/multisig/update_config.go @@ -44,7 +44,11 @@ func (a Account) UpdateConfig(ctx context.Context, msg *v1.MsgUpdateConfig) (*v1 // get the weight from the stored members totalWeight := uint64(0) err := a.Members.Walk(ctx, nil, func(_ []byte, value uint64) (stop bool, err error) { - totalWeight += value + var adderr error + totalWeight, adderr = safeAdd(totalWeight, value) + if adderr != nil { + return true, adderr + } return false, nil }) if err != nil { diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index c057d5a7fa73..21b5300fd53c 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -12,14 +12,14 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -// TxFeeChecker check if the provided fee is enough and returns the effective fee and tx priority, -// the effective fee should be deducted later, and the priority should be returned in abci response. +// TxFeeChecker checks if the provided fee is enough and returns the effective fee and tx priority. +// The effective fee should be deducted later, and the priority should be returned in the ABCI response. type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) // DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. -// Call next AnteHandler if fees successfully deducted. -// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator +// Call next AnteHandler if fees are successfully deducted. +// CONTRACT: The Tx must implement the FeeTx interface to use DeductFeeDecorator. type DeductFeeDecorator struct { accountKeeper AccountKeeper bankKeeper types.BankKeeper @@ -43,7 +43,7 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKee func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { - return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") } txService := dfd.accountKeeper.GetEnvironment().TransactionService @@ -76,10 +76,11 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, nex func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error { feeTx, ok := sdkTx.(sdk.FeeTx) if !ok { - return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") } - if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { + addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName) + if len(addr) == 0 { return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) } @@ -87,8 +88,8 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee feeGranter := feeTx.FeeGranter() deductFeesFrom := feePayer - // if feegranter set deduct fee from feegranter account. - // this works with only when feegrant enabled. + // if feegranter set, deduct fee from feegranter account. + // this works only when feegrant is enabled. if feeGranter != nil { feeGranterAddr := sdk.AccAddress(feeGranter) @@ -132,7 +133,7 @@ func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc []byte, fees s err := bankKeeper.SendCoinsFromAccountToModule(ctx, sdk.AccAddress(acc), types.FeeCollectorName, fees) if err != nil { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + return fmt.Errorf("failed to deduct fees: %w", err) } return nil diff --git a/x/bank/CHANGELOG.md b/x/bank/CHANGELOG.md index 6f298085ee62..9d5a91718557 100644 --- a/x/bank/CHANGELOG.md +++ b/x/bank/CHANGELOG.md @@ -49,4 +49,3 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Consensus Breaking Changes * [#19188](https://github.com/cosmos/cosmos-sdk/pull/19188) Remove creation of `BaseAccount` when sending a message to an account that does not exist -* [#20343](https://github.com/cosmos/cosmos-sdk/pull/20343) Add a check in send moduleaccount to account to prevent module accounts from sending disabled tokens to accounts diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index 3c6ade1c235d..c8f9e5ba7bca 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -269,13 +269,6 @@ func (k BaseKeeper) SendCoinsFromModuleToAccount( return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", senderModule) } - for _, coin := range amt { - sendEnabled, found := k.getSendEnabled(ctx, coin.Denom) - if found && !sendEnabled { - return fmt.Errorf("denom: %s, is prohibited from being sent at this time", coin.Denom) - } - } - if k.BlockedAddr(recipientAddr) { return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", recipientAddr) } diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 45fdb7031dc8..12db16b24c2b 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -389,24 +389,6 @@ func (suite *KeeperTestSuite) TestSendCoinsFromModuleToAccount_Blocklist() { )) } -func (suite *KeeperTestSuite) TestSendCoinsFromModuleToAccount_CoinSendDisabled() { - ctx := suite.ctx - require := suite.Require() - keeper := suite.bankKeeper - - suite.mockMintCoins(mintAcc) - require.NoError(keeper.MintCoins(ctx, banktypes.MintModuleName, initCoins)) - - keeper.SetSendEnabled(ctx, sdk.DefaultBondDenom, false) - - suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) - err := keeper.SendCoinsFromModuleToAccount( - ctx, banktypes.MintModuleName, accAddrs[2], initCoins, - ) - require.Contains(err.Error(), "stake, is prohibited from being sent at this time") - keeper.SetSendEnabled(ctx, sdk.DefaultBondDenom, true) -} - func (suite *KeeperTestSuite) TestSupply_DelegateUndelegateCoins() { ctx := suite.ctx require := suite.Require()