Skip to content

Commit

Permalink
Cherry-pick: Problem: fee collection not compatible with parallel exe…
Browse files Browse the repository at this point in the history
…cution (crypto-org-chain#237)

Reason for Cherry-pick:
- Migrate Block STM (Parallel Transaction Execution)
- Avoid common conflict problem (fee collection happens for every tx)

Notes for Cherry-pick:
- Use transient store instead of object store introduced by Cronos

Original Commit Message:

* Problem: no efficient way to collect fee

Solution:
- support an idea of virtual account in bank module, where the incoming
  coins are accumulated in a per-tx object store first, then accumulate
  and credit to the real account at end blocker.

  it's nesserary to support parallel tx execution, where we try not to
  access shared states.

more efficient sum

support SendCoinsFromModuleToAccountVirtual

fix test

fix test

* fix lint

* fix test

* fix test

* fix test

* fix test

* fix test

* fix mock keeper

* try fix lint

* try fix lint

* reuse code

* try fix linter

* Update x/bank/keeper/send.go

Signed-off-by: yihuang <[email protected]>

* algin panic call

* fix error handling

* try fix lint

* nolintlint generate falst postiive

---------

Signed-off-by: yihuang <[email protected]>
Cherry-picked-by: zsystm <[email protected]>
  • Loading branch information
yihuang authored and zsystm committed May 28, 2024
1 parent 650cf91 commit eee3c28
Show file tree
Hide file tree
Showing 19 changed files with 333 additions and 21 deletions.
1 change: 0 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ linters:
- ineffassign
- misspell
- nakedret
- nolintlint
- staticcheck
- revive
- stylecheck
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Features

* (baseapp) [#205](https://github.com/crypto-org-chain/cosmos-sdk/pull/205) Add `TxExecutor` baseapp option, add `TxIndex`/`TxCount`/`MsgIndex`/`BlockGasUsed` fields to `Context, to support tx parallel execution.

* (bank) [#237](https://github.com/crypto-org-chain/cosmos-sdk/pull/237) Support virtual accounts in sending coins.

## [v0.50.6](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.6) - 2024-04-22

### Features
Expand Down
4 changes: 3 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func NewSimApp(
panic(err)
}

tkeys := storetypes.NewTransientStoreKeys(paramstypes.TStoreKey)
tkeys := storetypes.NewTransientStoreKeys(banktypes.TStoreKey, paramstypes.TStoreKey)
app := &SimApp{
BaseApp: bApp,
legacyAmino: legacyAmino,
Expand All @@ -292,6 +292,7 @@ func NewSimApp(
BlockedAddresses(),
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
logger,
tkeys[banktypes.TStoreKey],
)

// optional: enable sign mode textual by overwriting the default tx config (after setting the bank keeper)
Expand Down Expand Up @@ -456,6 +457,7 @@ func NewSimApp(
authz.ModuleName,
)
app.ModuleManager.SetOrderEndBlockers(
banktypes.ModuleName,
crisistypes.ModuleName,
govtypes.ModuleName,
stakingtypes.ModuleName,
Expand Down
1 change: 1 addition & 0 deletions simapp/app_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ var (
authz.ModuleName,
},
EndBlockers: []string{
banktypes.ModuleName,
crisistypes.ModuleName,
govtypes.ModuleName,
stakingtypes.ModuleName,
Expand Down
6 changes: 5 additions & 1 deletion tests/integration/distribution/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,13 @@ func initFixture(t testing.TB) *fixture {
keys := storetypes.NewKVStoreKeys(
authtypes.StoreKey, banktypes.StoreKey, distrtypes.StoreKey, stakingtypes.StoreKey,
)
okeys := storetypes.NewObjectStoreKeys(
banktypes.ObjectStoreKey,
)
cdc := moduletestutil.MakeTestEncodingConfig(auth.AppModuleBasic{}, distribution.AppModuleBasic{}).Codec

logger := log.NewTestLogger(t)
cms := integration.CreateMultiStore(keys, logger)
cms := integration.CreateMultiStore(keys, okeys, logger)

newCtx := sdk.NewContext(cms, types.Header{}, true, logger)

Expand Down Expand Up @@ -92,6 +95,7 @@ func initFixture(t testing.TB) *fixture {
bankKeeper := bankkeeper.NewBaseKeeper(
cdc,
runtime.NewKVStoreService(keys[banktypes.StoreKey]),
okeys[banktypes.ObjectStoreKey],
accountKeeper,
blockedAddresses,
authority.String(),
Expand Down
6 changes: 5 additions & 1 deletion tests/integration/evidence/keeper/infraction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,13 @@ func initFixture(t testing.TB) *fixture {
keys := storetypes.NewKVStoreKeys(
authtypes.StoreKey, banktypes.StoreKey, paramtypes.StoreKey, consensusparamtypes.StoreKey, evidencetypes.StoreKey, stakingtypes.StoreKey, slashingtypes.StoreKey,
)
okeys := storetypes.NewObjectStoreKeys(
banktypes.ObjectStoreKey,
)
cdc := moduletestutil.MakeTestEncodingConfig(auth.AppModuleBasic{}, evidence.AppModuleBasic{}).Codec

logger := log.NewTestLogger(t)
cms := integration.CreateMultiStore(keys, logger)
cms := integration.CreateMultiStore(keys, okeys, logger)

newCtx := sdk.NewContext(cms, cmtproto.Header{}, true, logger)

Expand Down Expand Up @@ -114,6 +117,7 @@ func initFixture(t testing.TB) *fixture {
bankKeeper := bankkeeper.NewBaseKeeper(
cdc,
runtime.NewKVStoreService(keys[banktypes.StoreKey]),
okeys[banktypes.ObjectStoreKey],
accountKeeper,
blockedAddresses,
authority.String(),
Expand Down
6 changes: 5 additions & 1 deletion tests/integration/gov/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ func initFixture(t testing.TB) *fixture {
keys := storetypes.NewKVStoreKeys(
authtypes.StoreKey, banktypes.StoreKey, distrtypes.StoreKey, stakingtypes.StoreKey, types.StoreKey,
)
okeys := storetypes.NewObjectStoreKeys(
banktypes.ObjectStoreKey,
)
cdc := moduletestutil.MakeTestEncodingConfig(auth.AppModuleBasic{}, bank.AppModuleBasic{}, gov.AppModuleBasic{}).Codec

logger := log.NewTestLogger(t)
cms := integration.CreateMultiStore(keys, logger)
cms := integration.CreateMultiStore(keys, okeys, logger)

newCtx := sdk.NewContext(cms, cmtproto.Header{}, true, logger)

Expand Down Expand Up @@ -85,6 +88,7 @@ func initFixture(t testing.TB) *fixture {
bankKeeper := bankkeeper.NewBaseKeeper(
cdc,
runtime.NewKVStoreService(keys[banktypes.StoreKey]),
okeys[banktypes.ObjectStoreKey],
accountKeeper,
blockedAddresses,
authority.String(),
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/slashing/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ func initFixture(t testing.TB) *fixture {
keys := storetypes.NewKVStoreKeys(
authtypes.StoreKey, banktypes.StoreKey, slashingtypes.StoreKey, stakingtypes.StoreKey,
)
okeys := storetypes.NewObjectStoreKeys(banktypes.ObjectStoreKey)
cdc := moduletestutil.MakeTestEncodingConfig(auth.AppModuleBasic{}).Codec

logger := log.NewTestLogger(t)
cms := integration.CreateMultiStore(keys, logger)
cms := integration.CreateMultiStore(keys, okeys, logger)

newCtx := sdk.NewContext(cms, cmtproto.Header{}, true, logger)

Expand Down Expand Up @@ -85,6 +86,7 @@ func initFixture(t testing.TB) *fixture {
bankKeeper := bankkeeper.NewBaseKeeper(
cdc,
runtime.NewKVStoreService(keys[banktypes.StoreKey]),
okeys[banktypes.ObjectStoreKey],
accountKeeper,
blockedAddresses,
authority.String(),
Expand Down
6 changes: 5 additions & 1 deletion tests/integration/staking/keeper/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,13 @@ func initFixture(t testing.TB) *fixture {
keys := storetypes.NewKVStoreKeys(
authtypes.StoreKey, banktypes.StoreKey, types.StoreKey,
)
okeys := storetypes.NewObjectStoreKeys(
banktypes.ObjectStoreKey,
)
cdc := moduletestutil.MakeTestEncodingConfig(auth.AppModuleBasic{}, staking.AppModuleBasic{}).Codec

logger := log.NewTestLogger(t)
cms := integration.CreateMultiStore(keys, logger)
cms := integration.CreateMultiStore(keys, okeys, logger)

newCtx := sdk.NewContext(cms, cmtprototypes.Header{}, true, logger)

Expand Down Expand Up @@ -127,6 +130,7 @@ func initFixture(t testing.TB) *fixture {
bankKeeper := bankkeeper.NewBaseKeeper(
cdc,
runtime.NewKVStoreService(keys[banktypes.StoreKey]),
okeys[banktypes.ObjectStoreKey],
accountKeeper,
blockedAddresses,
authority.String(),
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/staking/keeper/determinstic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,11 @@ func initDeterministicFixture(t *testing.T) *deterministicFixture {
keys := storetypes.NewKVStoreKeys(
authtypes.StoreKey, banktypes.StoreKey, stakingtypes.StoreKey,
)
okeys := storetypes.NewObjectStoreKeys(banktypes.ObjectStoreKey)
cdc := moduletestutil.MakeTestEncodingConfig(auth.AppModuleBasic{}, distribution.AppModuleBasic{}).Codec

logger := log.NewTestLogger(t)
cms := integration.CreateMultiStore(keys, logger)
cms := integration.CreateMultiStore(keys, okeys, logger)

newCtx := sdk.NewContext(cms, cmtproto.Header{}, true, logger)

Expand Down Expand Up @@ -100,6 +101,7 @@ func initDeterministicFixture(t *testing.T) *deterministicFixture {
bankKeeper := bankkeeper.NewBaseKeeper(
cdc,
runtime.NewKVStoreService(keys[banktypes.StoreKey]),
okeys[banktypes.ObjectStoreKey],
accountKeeper,
blockedAddresses,
authority.String(),
Expand Down
4 changes: 2 additions & 2 deletions testutil/integration/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func Example() {
// replace the logger by testing values in a real test case (e.g. log.NewTestLogger(t))
logger := log.NewNopLogger()

cms := integration.CreateMultiStore(keys, logger)
cms := integration.CreateMultiStore(keys, nil, logger)
newCtx := sdk.NewContext(cms, cmtproto.Header{}, true, logger)

accountKeeper := authkeeper.NewAccountKeeper(
Expand Down Expand Up @@ -126,7 +126,7 @@ func Example_oneModule() {
// replace the logger by testing values in a real test case (e.g. log.NewTestLogger(t))
logger := log.NewLogger(io.Discard)

cms := integration.CreateMultiStore(keys, logger)
cms := integration.CreateMultiStore(keys, nil, logger)
newCtx := sdk.NewContext(cms, cmtproto.Header{}, true, logger)

accountKeeper := authkeeper.NewAccountKeeper(
Expand Down
2 changes: 2 additions & 0 deletions x/bank/keeper/collections_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

func TestBankStateCompatibility(t *testing.T) {
key := storetypes.NewKVStoreKey(banktypes.StoreKey)
tkey := storetypes.NewTransientStoreKey(banktypes.TStoreKey)
testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test"))
ctx := testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()})
encCfg := moduletestutil.MakeTestEncodingConfig()
Expand All @@ -44,6 +45,7 @@ func TestBankStateCompatibility(t *testing.T) {
map[string]bool{accAddrs[4].String(): true},
authtypes.NewModuleAddress("gov").String(),
log.NewNopLogger(),
tkey,
)

// test we can decode balances without problems
Expand Down
10 changes: 8 additions & 2 deletions x/bank/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/log"
"cosmossdk.io/math"

storetypes "cosmossdk.io/store/types"
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -46,9 +46,14 @@ type Keeper interface {
MintCoins(ctx context.Context, moduleName string, amt sdk.Coins) error
BurnCoins(ctx context.Context, moduleName string, amt sdk.Coins) error

SendCoinsFromAccountToModuleVirtual(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
SendCoinsFromModuleToAccountVirtual(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error

DelegateCoins(ctx context.Context, delegatorAddr, moduleAccAddr sdk.AccAddress, amt sdk.Coins) error
UndelegateCoins(ctx context.Context, moduleAccAddr, delegatorAddr sdk.AccAddress, amt sdk.Coins) error

CreditVirtualAccounts(ctx context.Context) error

types.QueryServer
}

Expand Down Expand Up @@ -88,6 +93,7 @@ func NewBaseKeeper(
blockedAddrs map[string]bool,
authority string,
logger log.Logger,
tkey storetypes.StoreKey,
) BaseKeeper {
if _, err := ak.AddressCodec().StringToBytes(authority); err != nil {
panic(fmt.Errorf("invalid bank authority address: %w", err))
Expand All @@ -97,7 +103,7 @@ func NewBaseKeeper(
logger = logger.With(log.ModuleKey, "x/"+types.ModuleName)

return BaseKeeper{
BaseSendKeeper: NewBaseSendKeeper(cdc, storeService, ak, blockedAddrs, authority, logger),
BaseSendKeeper: NewBaseSendKeeper(cdc, storeService, ak, blockedAddrs, authority, logger, tkey),
ak: ak,
cdc: cdc,
storeService: storeService,
Expand Down
54 changes: 50 additions & 4 deletions x/bank/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,13 @@ func TestKeeperTestSuite(t *testing.T) {
}

func (suite *KeeperTestSuite) SetupTest() {
key := storetypes.NewKVStoreKey(banktypes.StoreKey)
testCtx := testutil.DefaultContextWithDB(suite.T(), key, storetypes.NewTransientStoreKey("transient_test"))
ctx := testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()})
keys := storetypes.NewKVStoreKeys(banktypes.StoreKey)
tkeys := storetypes.NewTransientStoreKeys(banktypes.TStoreKey)
testCtx := testutil.DefaultContextWithKeys(keys, tkeys, nil)
ctx := testCtx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()})
encCfg := moduletestutil.MakeTestEncodingConfig()

storeService := runtime.NewKVStoreService(key)
storeService := runtime.NewKVStoreService(keys[banktypes.StoreKey])

// gomock initializations
ctrl := gomock.NewController(suite.T())
Expand All @@ -149,6 +150,7 @@ func (suite *KeeperTestSuite) SetupTest() {
map[string]bool{accAddrs[4].String(): true},
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
log.NewNopLogger(),
tkeys[banktypes.TStoreKey],
)

banktypes.RegisterInterfaces(encCfg.InterfaceRegistry)
Expand Down Expand Up @@ -196,6 +198,16 @@ func (suite *KeeperTestSuite) mockSendCoinsFromAccountToModule(acc *authtypes.Ba
suite.authKeeper.EXPECT().HasAccount(suite.ctx, moduleAcc.GetAddress()).Return(true)
}

func (suite *KeeperTestSuite) mockSendCoinsFromAccountToModuleVirtual(acc *authtypes.BaseAccount, moduleAcc *authtypes.ModuleAccount) {
suite.authKeeper.EXPECT().GetModuleAccount(suite.ctx, moduleAcc.Name).Return(moduleAcc)
suite.authKeeper.EXPECT().GetAccount(suite.ctx, acc.GetAddress()).Return(acc)
}

func (suite *KeeperTestSuite) mockSendCoinsFromModuleToAccountVirtual(moduleAcc *authtypes.ModuleAccount, accAddr sdk.AccAddress) {
suite.authKeeper.EXPECT().GetModuleAddress(moduleAcc.Name).Return(moduleAcc.GetAddress())
suite.authKeeper.EXPECT().HasAccount(suite.ctx, accAddr).Return(true)
}

func (suite *KeeperTestSuite) mockSendCoins(ctx context.Context, sender sdk.AccountI, receiver sdk.AccAddress) {
suite.authKeeper.EXPECT().GetAccount(ctx, sender.GetAddress()).Return(sender)
suite.authKeeper.EXPECT().HasAccount(ctx, receiver).Return(true)
Expand Down Expand Up @@ -312,6 +324,7 @@ func (suite *KeeperTestSuite) TestPrependSendRestriction() {

func (suite *KeeperTestSuite) TestGetAuthority() {
storeService := runtime.NewKVStoreService(storetypes.NewKVStoreKey(banktypes.StoreKey))
tkey := storetypes.NewTransientStoreKey(banktypes.TStoreKey)
NewKeeperWithAuthority := func(authority string) keeper.BaseKeeper {
return keeper.NewBaseKeeper(
moduletestutil.MakeTestEncodingConfig().Codec,
Expand All @@ -320,6 +333,7 @@ func (suite *KeeperTestSuite) TestGetAuthority() {
nil,
authority,
log.NewNopLogger(),
tkey,
)
}

Expand Down Expand Up @@ -632,6 +646,38 @@ func (suite *KeeperTestSuite) TestSendCoinsNewAccount() {
require.Equal(acc1Balances, updatedAcc1Bal)
}

func (suite *KeeperTestSuite) TestSendCoinsVirtual() {
ctx := suite.ctx
require := suite.Require()
keeper := suite.bankKeeper
sdkCtx := sdk.UnwrapSDKContext(ctx)
acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0])
feeDenom1 := "fee1"
feeDenom2 := "fee2"

balances := sdk.NewCoins(sdk.NewInt64Coin(feeDenom1, 100), sdk.NewInt64Coin(feeDenom2, 100))
suite.mockFundAccount(accAddrs[0])
require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], balances))

sendAmt := sdk.NewCoins(sdk.NewInt64Coin(feeDenom1, 50), sdk.NewInt64Coin(feeDenom2, 50))
suite.mockSendCoinsFromAccountToModuleVirtual(acc0, burnerAcc)
require.NoError(
keeper.SendCoinsFromAccountToModuleVirtual(sdkCtx, accAddrs[0], authtypes.Burner, sendAmt),
)

refundAmt := sdk.NewCoins(sdk.NewInt64Coin(feeDenom1, 25), sdk.NewInt64Coin(feeDenom2, 25))
suite.mockSendCoinsFromModuleToAccountVirtual(burnerAcc, accAddrs[0])
require.NoError(
keeper.SendCoinsFromModuleToAccountVirtual(sdkCtx, authtypes.Burner, accAddrs[0], refundAmt),
)

suite.authKeeper.EXPECT().HasAccount(suite.ctx, burnerAcc.GetAddress()).Return(true)
require.NoError(keeper.CreditVirtualAccounts(ctx))

require.Equal(math.NewInt(25), keeper.GetBalance(suite.ctx, burnerAcc.GetAddress(), feeDenom1).Amount)
require.Equal(math.NewInt(25), keeper.GetBalance(suite.ctx, burnerAcc.GetAddress(), feeDenom2).Amount)
}

func (suite *KeeperTestSuite) TestInputOutputNewAccount() {
ctx := suite.ctx
require := suite.Require()
Expand Down
Loading

0 comments on commit eee3c28

Please sign in to comment.