diff --git a/aclmapping/bank/mappings.go b/aclmapping/bank/mappings.go index 3d65fa90ba..47989a1823 100644 --- a/aclmapping/bank/mappings.go +++ b/aclmapping/bank/mappings.go @@ -7,6 +7,7 @@ import ( sdkacltypes "github.com/cosmos/cosmos-sdk/types/accesscontrol" aclkeeper "github.com/cosmos/cosmos-sdk/x/accesscontrol/keeper" acltypes "github.com/cosmos/cosmos-sdk/x/accesscontrol/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" utils "github.com/sei-protocol/sei-chain/aclmapping/utils" ) @@ -23,12 +24,13 @@ func GetBankDepedencyGenerator() aclkeeper.DependencyGeneratorMap { return dependencyGeneratorMap } -// TODO:: we can make resource types more granular (e.g KV_PARAM or KV_BANK_BALANCE) func MsgSendDependencyGenerator(keeper aclkeeper.Keeper, ctx sdk.Context, msg sdk.Msg) ([]sdkacltypes.AccessOperation, error) { msgSend, ok := msg.(*banktypes.MsgSend) if !ok { return []sdkacltypes.AccessOperation{}, ErrorInvalidMsgType } + fromAddrIdentifier := string(banktypes.CreateAccountBalancesPrefixFromBech32(msgSend.FromAddress)) + toAddrIdentifier := string(banktypes.CreateAccountBalancesPrefixFromBech32(msgSend.ToAddress)) accessOperations := []sdkacltypes.AccessOperation{ // MsgSend also checks if the coin denom is enabled, but the information is from the params. @@ -37,46 +39,53 @@ func MsgSendDependencyGenerator(keeper aclkeeper.Keeper, ctx sdk.Context, msg sd // Checks balance of sender { AccessType: sdkacltypes.AccessType_READ, - ResourceType: sdkacltypes.ResourceType_KV, - IdentifierTemplate: utils.GetIdentifierTemplatePerModule(utils.BANK, msgSend.FromAddress), + ResourceType: sdkacltypes.ResourceType_KV_BANK_BALANCES, + IdentifierTemplate: fromAddrIdentifier, }, // Reduce the amount from the sender's balance { AccessType: sdkacltypes.AccessType_WRITE, - ResourceType: sdkacltypes.ResourceType_KV, - IdentifierTemplate: utils.GetIdentifierTemplatePerModule(utils.BANK, msgSend.FromAddress), + ResourceType: sdkacltypes.ResourceType_KV_BANK_BALANCES, + IdentifierTemplate: fromAddrIdentifier, }, // Checks balance for receiver { AccessType: sdkacltypes.AccessType_READ, - ResourceType: sdkacltypes.ResourceType_KV, - IdentifierTemplate: utils.GetIdentifierTemplatePerModule(utils.BANK, msgSend.ToAddress), + ResourceType: sdkacltypes.ResourceType_KV_BANK_BALANCES, + IdentifierTemplate: toAddrIdentifier, }, { AccessType: sdkacltypes.AccessType_WRITE, - ResourceType: sdkacltypes.ResourceType_KV, - IdentifierTemplate: utils.GetIdentifierTemplatePerModule(utils.BANK, msgSend.ToAddress), + ResourceType: sdkacltypes.ResourceType_KV_BANK_BALANCES, + IdentifierTemplate: toAddrIdentifier, }, // Tries to create the reciever's account if it doesn't exist { AccessType: sdkacltypes.AccessType_READ, - ResourceType: sdkacltypes.ResourceType_KV, - IdentifierTemplate: utils.GetIdentifierTemplatePerModule(utils.AUTH, msgSend.ToAddress), + ResourceType: sdkacltypes.ResourceType_KV_AUTH_ADDRESS_STORE, + IdentifierTemplate: string(authtypes.CreateAddressStoreKeyFromBech32(msgSend.ToAddress)), }, { AccessType: sdkacltypes.AccessType_WRITE, - ResourceType: sdkacltypes.ResourceType_KV, - IdentifierTemplate: utils.GetIdentifierTemplatePerModule(utils.AUTH, msgSend.ToAddress), + ResourceType: sdkacltypes.ResourceType_KV_AUTH_ADDRESS_STORE, + IdentifierTemplate: string(authtypes.CreateAddressStoreKeyFromBech32(msgSend.ToAddress)), + }, + + // Gets Account Info for the sender + { + AccessType: sdkacltypes.AccessType_READ, + ResourceType: sdkacltypes.ResourceType_KV_AUTH_ADDRESS_STORE, + IdentifierTemplate: string(authtypes.CreateAddressStoreKeyFromBech32(msgSend.FromAddress)), }, - // Last Operation should always be a commit { ResourceType: sdkacltypes.ResourceType_ANY, AccessType: sdkacltypes.AccessType_COMMIT, IdentifierTemplate: utils.DefaultIDTemplate, }, } + return accessOperations, nil } diff --git a/aclmapping/bank/mappings_test.go b/aclmapping/bank/mappings_test.go new file mode 100644 index 0000000000..9800592f0b --- /dev/null +++ b/aclmapping/bank/mappings_test.go @@ -0,0 +1,158 @@ +package aclbankmapping + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + "github.com/cosmos/cosmos-sdk/simapp" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkacltypes "github.com/cosmos/cosmos-sdk/types/accesscontrol" + acltypes "github.com/cosmos/cosmos-sdk/x/accesscontrol/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/cosmos/cosmos-sdk/x/bank" + "github.com/cosmos/cosmos-sdk/x/bank/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + aclutils "github.com/sei-protocol/sei-chain/aclmapping/utils" + oracletypes "github.com/sei-protocol/sei-chain/x/oracle/types" + "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" +) + + +func cacheTxContext(ctx sdk.Context) (sdk.Context, sdk.CacheMultiStore) { + ms := ctx.MultiStore() + msCache := ms.CacheMultiStore() + return ctx.WithMultiStore(msCache), msCache +} + +func TestMsgBankSendAclOps(t *testing.T) { + priv1 := secp256k1.GenPrivKey() + addr1 := sdk.AccAddress(priv1.PubKey().Address()) + priv2 := secp256k1.GenPrivKey() + addr2 := sdk.AccAddress(priv2.PubKey().Address()) + coins := sdk.Coins{sdk.NewInt64Coin("foocoin", 10)} + + tests := []struct { + name string + expectedError error + msg *types.MsgSend + dynamicDep bool + }{ + { + name: "default send", + msg: types.NewMsgSend(addr1, addr2, coins), + expectedError: nil, + dynamicDep: true, + }, + { + name: "dont check synchronous", + msg: types.NewMsgSend(addr1, addr2, coins), + expectedError: nil, + dynamicDep: false, + }, + } + + acc1 := &authtypes.BaseAccount{ + Address: addr1.String(), + } + acc2 := &authtypes.BaseAccount{ + Address: addr2.String(), + } + accs := authtypes.GenesisAccounts{acc1, acc2} + balances := []types.Balance{ + { + Address: addr1.String(), + Coins: coins, + }, + { + Address: addr2.String(), + Coins: coins, + }, + } + + app := simapp.SetupWithGenesisAccounts(accs, balances...) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + handler := bank.NewHandler(app.BankKeeper) + msgValidator := sdkacltypes.NewMsgValidator(aclutils.StoreKeyToResourceTypePrefixMap) + ctx = ctx.WithMsgValidator(msgValidator) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + handlerCtx, cms := cacheTxContext(ctx) + + _, err := handler(handlerCtx, tc.msg) + + depdenencies , _ := MsgSendDependencyGenerator(app.AccessControlKeeper, handlerCtx, tc.msg) + + if !tc.dynamicDep { + depdenencies = sdkacltypes.SynchronousAccessOps() + } + + if tc.expectedError != nil { + require.EqualError(t, err, tc.expectedError.Error()) + } else { + require.NoError(t, err) + } + missing := handlerCtx.MsgValidator().ValidateAccessOperations(depdenencies, cms.GetEvents()) + require.Empty(t, missing) + }) + } +} + +func TestGeneratorInvalidMessageTypes(t *testing.T) { + accs := authtypes.GenesisAccounts{} + balances := []types.Balance{} + + app := simapp.SetupWithGenesisAccounts(accs, balances...) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + oracleVote := oracletypes.MsgAggregateExchangeRateVote{ + ExchangeRates: "1usei", + Feeder: "test", + Validator: "validator", + } + + _, err := MsgSendDependencyGenerator(app.AccessControlKeeper, ctx, &oracleVote) + require.Error(t, err) +} + +func TestMsgBeginBankSendGenerator(t *testing.T) { + priv1 := secp256k1.GenPrivKey() + addr1 := sdk.AccAddress(priv1.PubKey().Address()) + priv2 := secp256k1.GenPrivKey() + addr2 := sdk.AccAddress(priv2.PubKey().Address()) + coins := sdk.Coins{sdk.NewInt64Coin("foocoin", 10)} + + acc1 := &authtypes.BaseAccount{ + Address: addr1.String(), + } + acc2 := &authtypes.BaseAccount{ + Address: addr2.String(), + } + accs := authtypes.GenesisAccounts{acc1, acc2} + balances := []types.Balance{ + { + Address: addr1.String(), + Coins: coins, + }, + { + Address: addr2.String(), + Coins: coins, + }, + } + + app := simapp.SetupWithGenesisAccounts(accs, balances...) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + sendMsg := banktypes.MsgSend{ + FromAddress: addr1.String(), + ToAddress: addr2.String(), + Amount: coins, + } + + accessOps, err := MsgSendDependencyGenerator(app.AccessControlKeeper, ctx, &sendMsg) + require.NoError(t, err) + err = acltypes.ValidateAccessOps(accessOps) + require.NoError(t, err) +} diff --git a/aclmapping/utils/identifier_templates.go b/aclmapping/utils/identifier_templates.go index 2eeea12ea3..ba79cecc2a 100644 --- a/aclmapping/utils/identifier_templates.go +++ b/aclmapping/utils/identifier_templates.go @@ -1,4 +1,4 @@ -package util +package utils import ( "fmt" diff --git a/aclmapping/utils/resource_type.go b/aclmapping/utils/resource_type.go new file mode 100644 index 0000000000..bbd421582f --- /dev/null +++ b/aclmapping/utils/resource_type.go @@ -0,0 +1,51 @@ +package utils + +import ( + aclsdktypes "github.com/cosmos/cosmos-sdk/types/accesscontrol" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + dextypes "github.com/sei-protocol/sei-chain/x/dex/types" + epochtypes "github.com/sei-protocol/sei-chain/x/epoch/types" + oracletypes "github.com/sei-protocol/sei-chain/x/oracle/types" + tokenfactorytypes "github.com/sei-protocol/sei-chain/x/tokenfactory/types" +) + +var StoreKeyToResourceTypePrefixMap = aclsdktypes.StoreKeyToResourceTypePrefixMap{ + aclsdktypes.ParentNodeKey: { + aclsdktypes.ResourceType_ANY: aclsdktypes.EmptyPrefix, + aclsdktypes.ResourceType_KV: aclsdktypes.EmptyPrefix, + aclsdktypes.ResourceType_Mem: aclsdktypes.EmptyPrefix, + }, + dextypes.StoreKey: { + aclsdktypes.ResourceType_KV_DEX: aclsdktypes.EmptyPrefix, + aclsdktypes.ResourceType_DexMem: aclsdktypes.EmptyPrefix, + }, + banktypes.StoreKey: { + aclsdktypes.ResourceType_KV_BANK: aclsdktypes.EmptyPrefix, + aclsdktypes.ResourceType_KV_BANK_BALANCES: banktypes.BalancesPrefix, + aclsdktypes.ResourceType_KV_BANK_SUPPLY: banktypes.SupplyKey, + aclsdktypes.ResourceType_KV_BANK_DENOM: banktypes.DenomMetadataPrefix, + }, + authtypes.StoreKey: { + aclsdktypes.ResourceType_KV_AUTH: aclsdktypes.EmptyPrefix, + aclsdktypes.ResourceType_KV_AUTH_ADDRESS_STORE: authtypes.AddressStoreKeyPrefix, + }, + oracletypes.StoreKey: { + aclsdktypes.ResourceType_KV_ORACLE: aclsdktypes.EmptyPrefix, + aclsdktypes.ResourceType_KV_ORACLE_VOTE_TARGETS: oracletypes.VoteTargetKey, + aclsdktypes.ResourceType_KV_ORACLE_AGGREGATE_VOTES: oracletypes.AggregateExchangeRateVoteKey, + aclsdktypes.ResourceType_KV_ORACLE_FEEDERS: oracletypes.FeederDelegationKey, + }, + stakingtypes.StoreKey: { + aclsdktypes.ResourceType_KV_STAKING: aclsdktypes.EmptyPrefix, + aclsdktypes.ResourceType_KV_STAKING_DELEGATION: stakingtypes.DelegationKey, + aclsdktypes.ResourceType_KV_STAKING_VALIDATOR: stakingtypes.ValidatorsKey, + }, + tokenfactorytypes.StoreKey: { + aclsdktypes.ResourceType_KV_TOKENFACTORY: aclsdktypes.EmptyPrefix, + }, + epochtypes.StoreKey: { + aclsdktypes.ResourceType_KV_EPOCH: aclsdktypes.EmptyPrefix, + }, +} diff --git a/app/app.go b/app/app.go index 5b1a7bca10..05caa1c0c5 100644 --- a/app/app.go +++ b/app/app.go @@ -37,11 +37,11 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/auth/ante" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" aclmodule "github.com/cosmos/cosmos-sdk/x/accesscontrol" aclclient "github.com/cosmos/cosmos-sdk/x/accesscontrol/client" aclkeeper "github.com/cosmos/cosmos-sdk/x/accesscontrol/keeper" acltypes "github.com/cosmos/cosmos-sdk/x/accesscontrol/types" - authrest "github.com/cosmos/cosmos-sdk/x/auth/client/rest" authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" authsims "github.com/cosmos/cosmos-sdk/x/auth/simulation" @@ -996,11 +996,13 @@ func (app *App) ProcessTxConcurrent( resultChan chan<- ChannelResult, txCompletionSignalingMap acltypes.MessageCompletionSignalMapping, txBlockingSignalsMap acltypes.MessageCompletionSignalMapping, + txMsgAccessOpMapping acltypes.MsgIndexToAccessOpMapping, ) { defer wg.Done() // Store the Channels in the Context Object for each transaction ctx = ctx.WithTxBlockingChannels(getChannelsFromSignalMapping(txBlockingSignalsMap)) ctx = ctx.WithTxCompletionChannels(getChannelsFromSignalMapping(txCompletionSignalingMap)) + ctx = ctx.WithTxMsgAccessOps(txMsgAccessOpMapping) // Deliver the transaction and store the result in the channel @@ -1013,18 +1015,18 @@ func (app *App) ProcessBlockConcurrent( txs [][]byte, completionSignalingMap map[int]acltypes.MessageCompletionSignalMapping, blockingSignalsMap map[int]acltypes.MessageCompletionSignalMapping, -) []*abci.ExecTxResult { + txMsgAccessOpMapping map[int]acltypes.MsgIndexToAccessOpMapping, +) ([]*abci.ExecTxResult, bool) { defer metrics.BlockProcessLatency(time.Now(), metrics.CONCURRENT) - var waitGroup sync.WaitGroup - resultChan := make(chan ChannelResult) txResults := []*abci.ExecTxResult{} - // If there's no transactions then return empty results if len(txs) == 0 { - return txResults + return txResults, true } + var waitGroup sync.WaitGroup + resultChan := make(chan ChannelResult) // For each transaction, start goroutine and deliver TX for txIndex, txBytes := range txs { waitGroup.Add(1) @@ -1036,6 +1038,7 @@ func (app *App) ProcessBlockConcurrent( resultChan, completionSignalingMap[txIndex], blockingSignalsMap[txIndex], + txMsgAccessOpMapping[txIndex], ) } @@ -1059,7 +1062,16 @@ func (app *App) ProcessBlockConcurrent( txResults = append(txResults, txResultsMap[txIndex]) } - return txResults + ok := true + for i, result := range txResults { + if result.GetCode() == sdkerrors.ErrInvalidConcurrencyExecution.ABCICode() { + ctx.Logger().Error(fmt.Sprintf("Invalid concurrent execution of deliverTx index=%d", i)) + metrics.IncrFailedConcurrentDeliverTxCounter() + ok = false + } + } + + return txResults, ok } func (app *App) ProcessBlock(ctx sdk.Context, txs [][]byte, req BlockProcessRequest, lastCommit abci.CommitInfo) ([]abci.Event, []*abci.ExecTxResult, abci.ResponseEndBlock, error) { @@ -1115,7 +1127,24 @@ func (app *App) ProcessBlock(ctx sdk.Context, txs [][]byte, req BlockProcessRequ // CacheMultiStore where it writes the data to the parent store (DeliverState) in sorted Key order to maintain // deterministic ordering between validators in the case of concurrent deliverTXs processBlockCtx, processBlockCache := app.CacheContext(ctx) - txResults = app.ProcessBlockConcurrent(processBlockCtx, txs, dependencyDag.CompletionSignalingMap, dependencyDag.BlockingSignalsMap) + concurrentResults, ok := app.ProcessBlockConcurrent( + processBlockCtx, + txs, + dependencyDag.CompletionSignalingMap, + dependencyDag.BlockingSignalsMap, + dependencyDag.TxMsgAccessOpMapping, + ) + if ok { + ctx.Logger().Info("Concurrent Execution succeeded, proceeding to commit block") + txResults = concurrentResults + // Write the results back to the concurrent contexts - if concurrent execution fails, + // this should not be called and the state is rolled back and retried with synchronous execution + processBlockCache.Write() + } else { + ctx.Logger().Error("Concurrent Execution failed, retrying with Synchronous") + txResults = app.ProcessBlockSynchronous(ctx, txs) + } + // Write the results back to the concurrent contexts processBlockCache.Write() case acltypes.ErrGovMsgInBlock: diff --git a/go.mod b/go.mod index af5c2bddbf..8fb6dfbdbd 100644 --- a/go.mod +++ b/go.mod @@ -131,7 +131,7 @@ require ( ) replace ( - github.com/cosmos/cosmos-sdk => github.com/sei-protocol/sei-cosmos v0.1.200 + github.com/cosmos/cosmos-sdk => github.com/sei-protocol/sei-cosmos v0.1.239 github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 github.com/keybase/go-keychain => github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 github.com/tendermint/tendermint => github.com/sei-protocol/sei-tendermint v0.1.59 diff --git a/go.sum b/go.sum index 321f744059..e54feded88 100644 --- a/go.sum +++ b/go.sum @@ -1098,8 +1098,8 @@ github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg github.com/seccomp/libseccomp-golang v0.9.1/go.mod h1:GbW5+tmTXfcxTToHLXlScSlAvWlF4P2Ca7zGrPiEpWo= github.com/securego/gosec/v2 v2.11.0/go.mod h1:SX8bptShuG8reGC0XS09+a4H2BoWSJi+fscA+Pulbpo= github.com/segmentio/fasthash v1.0.3/go.mod h1:waKX8l2N8yckOgmSsXJi7x1ZfdKZ4x7KRMzBtS3oedY= -github.com/sei-protocol/sei-cosmos v0.1.200 h1:EbHDzt0aAASO7fy8Dac04lLzm0xhF1O1ILiA+UD3w98= -github.com/sei-protocol/sei-cosmos v0.1.200/go.mod h1:8ccWQxpBkWbpvBos/T4QO9K9gQxFs0duTqKRnagKo+0= +github.com/sei-protocol/sei-cosmos v0.1.239 h1:IuWt2PVB6kDVA7NHBRp5BGwgjw1Lg4dJT91yb4VmEZI= +github.com/sei-protocol/sei-cosmos v0.1.239/go.mod h1:c2dhT0+W9l83qjFJrQ6CcjhtHwHOoha5K1rtt4LeDBU= github.com/sei-protocol/sei-tendermint v0.1.59 h1:POGL60PumMQHF4EzAHzvkGfDnodQJLHpl65LuiwSO/Y= github.com/sei-protocol/sei-tendermint v0.1.59/go.mod h1:Olwbjyagrpoxj5DAUhHxMTWDVEfQ3FYdpypaJ3+6Hs8= github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= diff --git a/store/testutils.go b/store/testutils.go index 7a710bc745..cf73ff0d76 100644 --- a/store/testutils.go +++ b/store/testutils.go @@ -11,7 +11,7 @@ import ( func NewTestKVStore() types.KVStore { mem := dbadapter.Store{DB: dbm.NewMemDB()} - return cachekv.NewStore(mem) + return cachekv.NewStore(mem, nil) } func NewTestCacheMultiStore(stores map[types.StoreKey]types.CacheWrapper) types.CacheMultiStore { diff --git a/utils/metrics/metrics_util.go b/utils/metrics/metrics_util.go index 005c50e873..9bce5f919b 100644 --- a/utils/metrics/metrics_util.go +++ b/utils/metrics/metrics_util.go @@ -45,6 +45,18 @@ func IncrDagBuildErrorCounter(reason string) { ) } +// Counts the number of concurrent transactions that failed +// Metric Names: +// +// sei_tx_concurrent_delivertx_error +func IncrFailedConcurrentDeliverTxCounter() { + metrics.IncrCounterWithLabels( + []string{"sei", "tx", "concurrent", "delievertx", "error"}, + 1, + []metrics.Label{}, + ) +} + // Measures the time taken to execute a sudo msg // Metric Names: // diff --git a/x/oracle/simulation/operations.go b/x/oracle/simulation/operations.go index 2028e73625..3988c4b164 100644 --- a/x/oracle/simulation/operations.go +++ b/x/oracle/simulation/operations.go @@ -65,7 +65,7 @@ func WeightedOperations( } // SimulateMsgAggregateExchangeRateVote generates a MsgAggregateExchangeRateVote with random values. -// nolint: funlen +//nolint: funlen func SimulateMsgAggregateExchangeRateVote(ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper) simtypes.Operation { return func( r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string, @@ -122,7 +122,7 @@ func SimulateMsgAggregateExchangeRateVote(ak types.AccountKeeper, bk types.BankK } // SimulateMsgDelegateFeedConsent generates a MsgDelegateFeedConsent with random values. -// nolint: funlen +//nolint: funlen func SimulateMsgDelegateFeedConsent(ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper) simtypes.Operation { return func( r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string,