From a25ca2fdcab2bb4d58bfbbf122c675f0c144251e Mon Sep 17 00:00:00 2001 From: "KIm, JinSan" Date: Thu, 22 Apr 2021 16:41:33 +0900 Subject: [PATCH] feat: concurrent checkTx (#141) * chore: bump up ostracon, iavl and tm-db * feat: concurrent checkTx (#49) * feat: implement new abci, `BeginRecheckTx()` and `EndRecheckTx()` * test: fix tests * refactor: decompose checkTx & runTx * chore: protect app.checkState w/ RWMutex for simulate * chore: remove unused var * feat: account lock decorator * chore: skip AccountLockDecorator if not checkTx * chore: bump up tendermint * chore: revise accountlock position * chore: accountlock_test * chore: revise accountlock covers `cache.Write()` * chore: revise `sampleBytes` to `2` * fix: test according to `sampleBytes` * chore: revise `getUniqSortedAddressKey()` and add `getAddressKey()` * chore: revise `how to sort` not to use `reflection` * chore: bump up tendermint * test: check `sorted` in `TestGetUniqSortedAddressKey()` * chore: move `accountLock` from `anteTx()` to `checkTx()` # Conflicts: # baseapp/abci.go # baseapp/baseapp.go # baseapp/baseapp_test.go # baseapp/helpers.go # go.mod # go.sum # x/bank/bench_test.go # x/mock/test_utils.go * fix: make it buildable * fix: tests * fix: gasWanted & gasUsed are always `0` (#51) * fix: gasWanted & gasUsed is always `0` * chore: error log for general panic # Conflicts: # baseapp/baseapp.go --- baseapp/abci.go | 53 +++++---- baseapp/accountlock.go | 88 ++++++++++++++ baseapp/accountlock_test.go | 119 ++++++++++++++++++ baseapp/baseapp.go | 192 ++++++++++++++++-------------- baseapp/baseapp_test.go | 25 ++-- baseapp/test_helpers.go | 21 ++-- go.mod | 6 +- go.sum | 8 +- simapp/test_helpers.go | 5 +- x/bank/bench_test.go | 4 +- x/genutil/client/cli/init_test.go | 2 +- x/ibc/testing/chain.go | 15 ++- x/ibc/testing/coordinator.go | 7 +- 13 files changed, 394 insertions(+), 151 deletions(-) create mode 100644 baseapp/accountlock.go create mode 100644 baseapp/accountlock_test.go diff --git a/baseapp/abci.go b/baseapp/abci.go index 1a822def09..c462f23526 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -213,20 +213,16 @@ func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBloc func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx { defer telemetry.MeasureSince(time.Now(), "abci", "check_tx") - var mode runTxMode - - switch { - case req.Type == abci.CheckTxType_New: - mode = runTxModeCheck - - case req.Type == abci.CheckTxType_Recheck: - mode = runTxModeReCheck + tx, err := app.txDecoder(req.Tx) + if err != nil { + return sdkerrors.ResponseCheckTx(err, 0, 0, app.trace) + } - default: + if req.Type != abci.CheckTxType_New && req.Type != abci.CheckTxType_Recheck { panic(fmt.Sprintf("unknown RequestCheckTx type: %s", req.Type)) } - gInfo, result, err := app.runTx(mode, req.Tx) + gInfo, err := app.checkTx(req.Tx, tx, req.Type == abci.CheckTxType_Recheck) if err != nil { return sdkerrors.ResponseCheckTx(err, gInfo.GasWanted, gInfo.GasUsed, app.trace) } @@ -234,12 +230,21 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx { return abci.ResponseCheckTx{ GasWanted: int64(gInfo.GasWanted), // TODO: Should type accept unsigned ints? GasUsed: int64(gInfo.GasUsed), // TODO: Should type accept unsigned ints? - Log: result.Log, - Data: result.Data, - Events: sdk.MarkEventsToIndex(result.Events, app.indexEvents), } } +// BeginRecheckTx implements the ABCI interface and set the check state based on the given header +func (app *BaseApp) BeginRecheckTx(req abci.RequestBeginRecheckTx) abci.ResponseBeginRecheckTx { + // NOTE: This is safe because Ostracon holds a lock on the mempool for Rechecking. + app.setCheckState(req.Header) + return abci.ResponseBeginRecheckTx{Code: abci.CodeTypeOK} +} + +// EndRecheckTx implements the ABCI interface. +func (app *BaseApp) EndRecheckTx(req abci.RequestEndRecheckTx) abci.ResponseEndRecheckTx { + return abci.ResponseEndRecheckTx{Code: abci.CodeTypeOK} +} + // DeliverTx implements the ABCI interface and executes a tx in DeliverTx mode. // State only gets persisted if all messages are valid and get executed successfully. // Otherwise, the ResponseDeliverTx will contain releveant error information. @@ -258,7 +263,12 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx telemetry.SetGauge(float32(gInfo.GasWanted), "tx", "gas", "wanted") }() - gInfo, result, err := app.runTx(runTxModeDeliver, req.Tx) + tx, err := app.txDecoder(req.Tx) + if err != nil { + return sdkerrors.ResponseDeliverTx(err, 0, 0, app.trace) + } + + gInfo, result, err := app.runTx(req.Tx, tx, false) if err != nil { resultStr = "failed" return sdkerrors.ResponseDeliverTx(err, gInfo.GasWanted, gInfo.GasUsed, app.trace) @@ -275,11 +285,10 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx // Commit implements the ABCI interface. It will commit all state that exists in // the deliver state's multi-store and includes the resulting commit ID in the -// returned abci.ResponseCommit. Commit will set the check state based on the -// latest header and reset the deliver state. Also, if a non-zero halt height is -// defined in config, Commit will execute a deferred function call to check -// against that height and gracefully halt if it matches the latest committed -// height. +// returned abci.ResponseCommit. Commit will reset the deliver state. +// Also, if a non-zero halt height is defined in config, Commit will execute +// a deferred function call to check against that height and gracefully halt if +// it matches the latest committed height. func (app *BaseApp) Commit() (res abci.ResponseCommit) { defer telemetry.MeasureSince(time.Now(), "abci", "commit") @@ -293,12 +302,6 @@ func (app *BaseApp) Commit() (res abci.ResponseCommit) { commitID := app.cms.Commit() app.logger.Info("commit synced", "commit", fmt.Sprintf("%X", commitID)) - // Reset the Check state to the latest committed. - // - // NOTE: This is safe because Tendermint holds a lock on the mempool for - // Commit. Use the header from this latest block. - app.setCheckState(header) - // empty/reset the deliver state app.deliverState = nil diff --git a/baseapp/accountlock.go b/baseapp/accountlock.go new file mode 100644 index 0000000000..cd6828446d --- /dev/null +++ b/baseapp/accountlock.go @@ -0,0 +1,88 @@ +package baseapp + +import ( + "encoding/binary" + "sort" + "sync" + + sdk "github.com/line/lbm-sdk/v2/types" +) + +// NOTE should 1 <= sampleBytes <= 4. If modify it, you should revise `getAddressKey()` as well +const sampleBytes = 2 + +type AccountLock struct { + accMtx [1 << (sampleBytes * 8)]sync.Mutex +} + +func (al *AccountLock) Lock(ctx sdk.Context, tx sdk.Tx) []uint32 { + if !ctx.IsCheckTx() || ctx.IsReCheckTx() { + return nil + } + + signers := getSigners(tx) + accKeys := getUniqSortedAddressKey(signers) + + for _, key := range accKeys { + al.accMtx[key].Lock() + } + + return accKeys +} + +func (al *AccountLock) Unlock(accKeys []uint32) { + // NOTE reverse order + for i, length := 0, len(accKeys); i < length; i++ { + key := accKeys[length-1-i] + al.accMtx[key].Unlock() + } +} + +func getSigners(tx sdk.Tx) []sdk.AccAddress { + seen := map[string]bool{} + var signers []sdk.AccAddress + for _, msg := range tx.GetMsgs() { + for _, addr := range msg.GetSigners() { + if !seen[addr.String()] { + signers = append(signers, addr) + seen[addr.String()] = true + } + } + } + return signers +} + +func getUniqSortedAddressKey(addrs []sdk.AccAddress) []uint32 { + accKeys := make([]uint32, 0, len(addrs)) + for _, addr := range addrs { + accKeys = append(accKeys, getAddressKey(addr)) + } + + accKeys = uniq(accKeys) + sort.Sort(uint32Slice(accKeys)) + + return accKeys +} + +func getAddressKey(addr sdk.AccAddress) uint32 { + return uint32(binary.BigEndian.Uint16(addr)) +} + +func uniq(u []uint32) []uint32 { + seen := map[uint32]bool{} + var ret []uint32 + for _, v := range u { + if !seen[v] { + ret = append(ret, v) + seen[v] = true + } + } + return ret +} + +// Uint32Slice attaches the methods of Interface to []uint32, sorting in increasing order. +type uint32Slice []uint32 + +func (p uint32Slice) Len() int { return len(p) } +func (p uint32Slice) Less(i, j int) bool { return p[i] < p[j] } +func (p uint32Slice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } diff --git a/baseapp/accountlock_test.go b/baseapp/accountlock_test.go new file mode 100644 index 0000000000..054fed9b00 --- /dev/null +++ b/baseapp/accountlock_test.go @@ -0,0 +1,119 @@ +package baseapp + +import ( + "reflect" + "sort" + "sync" + "testing" + + "github.com/stretchr/testify/require" + + ostproto "github.com/line/ostracon/proto/ostracon/types" + + "github.com/line/lbm-sdk/v2/crypto/keys/secp256k1" + "github.com/line/lbm-sdk/v2/testutil/testdata" + sdk "github.com/line/lbm-sdk/v2/types" +) + +func TestAccountLock(t *testing.T) { + app := setupBaseApp(t) + ctx := app.NewContext(true, ostproto.Header{}) + + privs := newTestPrivKeys(3) + tx := newTestTx(privs) + + accKeys := app.accountLock.Lock(ctx, tx) + + for _, accKey := range accKeys { + require.True(t, isMutexLock(&app.accountLock.accMtx[accKey])) + } + + app.accountLock.Unlock(accKeys) + + for _, accKey := range accKeys { + require.False(t, isMutexLock(&app.accountLock.accMtx[accKey])) + } +} + +func TestUnlockDoNothingWithNil(t *testing.T) { + app := setupBaseApp(t) + require.NotPanics(t, func() { app.accountLock.Unlock(nil) }) +} + +func TestGetSigner(t *testing.T) { + privs := newTestPrivKeys(3) + tx := newTestTx(privs) + signers := getSigners(tx) + + require.Equal(t, getAddrs(privs), signers) +} + +func TestGetUniqSortedAddressKey(t *testing.T) { + privs := newTestPrivKeys(3) + + addrs := getAddrs(privs) + addrs = append(addrs, addrs[1], addrs[0]) + require.Equal(t, 5, len(addrs)) + + accKeys := getUniqSortedAddressKey(addrs) + + // length should be reduced because `duplicated` is removed + require.Less(t, len(accKeys), len(addrs)) + + // check uniqueness + for i, iv := range accKeys { + for j, jv := range accKeys { + if i != j { + require.True(t, iv != jv) + } + } + } + + // should be sorted + require.True(t, sort.IsSorted(uint32Slice(accKeys))) +} + +type AccountLockTestTx struct { + Msgs []sdk.Msg +} + +var _ sdk.Tx = AccountLockTestTx{} + +func (tx AccountLockTestTx) GetMsgs() []sdk.Msg { + return tx.Msgs +} + +func (tx AccountLockTestTx) ValidateBasic() error { + return nil +} + +func newTestPrivKeys(num int) []*secp256k1.PrivKey { + privs := make([]*secp256k1.PrivKey, 0, num) + for i := 0; i < num; i++ { + privs = append(privs, secp256k1.GenPrivKey()) + } + return privs +} + +func getAddrs(privs []*secp256k1.PrivKey) []sdk.AccAddress { + addrs := make([]sdk.AccAddress, 0, len(privs)) + for _, priv := range privs { + addrs = append(addrs, sdk.AccAddress(priv.PubKey().Address())) + } + return addrs +} + +func newTestTx(privs []*secp256k1.PrivKey) sdk.Tx { + addrs := getAddrs(privs) + msgs := make([]sdk.Msg, len(addrs)) + for i, addr := range addrs { + msgs[i] = testdata.NewTestMsg(addr) + } + return AccountLockTestTx{Msgs: msgs} +} + +// Hack (too slow) +func isMutexLock(mtx *sync.Mutex) bool { + state := reflect.ValueOf(mtx).Elem().FieldByName("state") + return state.Int() == 1 +} diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 1636cd663e..120db3f9da 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "strings" + "sync" "github.com/gogo/protobuf/proto" abci "github.com/line/ostracon/abci/types" @@ -21,21 +22,11 @@ import ( sdkerrors "github.com/line/lbm-sdk/v2/types/errors" ) -const ( - runTxModeCheck runTxMode = iota // Check a transaction - runTxModeReCheck // Recheck a (pending) transaction after a commit - runTxModeSimulate // Simulate a transaction - runTxModeDeliver // Deliver a transaction -) - var ( _ abci.Application = (*BaseApp)(nil) ) type ( - // Enum mode for app.runTx - runTxMode uint8 - // StoreLoader defines a customizable function to control how we load the CommitMultiStore // from disk. This is useful for state migration, when loading a datastore written with // an older version of the software. In particular, if a module changed the substore key name @@ -78,6 +69,9 @@ type BaseApp struct { // nolint: maligned checkState *state // for CheckTx deliverState *state // for DeliverTx + checkStateMtx sync.RWMutex + accountLock AccountLock + // an inter-block write-through cache provided to the context during deliverState interBlockCache sdk.MultiStorePersistentCache @@ -363,6 +357,8 @@ func (app *BaseApp) IsSealed() bool { return app.sealed } // on Commit. func (app *BaseApp) setCheckState(header ostproto.Header) { ms := app.cms.CacheMultiStore() + app.checkStateMtx.Lock() + defer app.checkStateMtx.Unlock() app.checkState = &state{ ms: ms, ctx: sdk.NewContext(ms, header, true, app.logger).WithMinGasPrices(app.minGasPrices), @@ -502,33 +498,31 @@ func validateBasicTxMsgs(msgs []sdk.Msg) error { return nil } -// Returns the applications's deliverState if app is in runTxModeDeliver, -// otherwise it returns the application's checkstate. -func (app *BaseApp) getState(mode runTxMode) *state { - if mode == runTxModeDeliver { - return app.deliverState +func (app *BaseApp) getCheckContextForTx(txBytes []byte, recheck bool) sdk.Context { + app.checkStateMtx.RLock() + defer app.checkStateMtx.RUnlock() + return app.getContextForTx(app.checkState, txBytes).WithIsReCheckTx(recheck) +} + +// retrieve the context for the tx w/ txBytes and other memoized values. +func (app *BaseApp) getRunContextForTx(txBytes []byte, simulate bool) sdk.Context { + if !simulate { + return app.getContextForTx(app.deliverState, txBytes) } - return app.checkState + app.checkStateMtx.RLock() + defer app.checkStateMtx.RUnlock() + ctx := app.getContextForTx(app.checkState, txBytes) + ctx, _ = ctx.CacheContext() + return ctx } -// retrieve the context for the tx w/ txBytes and other memoized values. -func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) sdk.Context { - ctx := app.getState(mode).ctx. +func (app *BaseApp) getContextForTx(s *state, txBytes []byte) sdk.Context { + ctx := s.ctx. WithTxBytes(txBytes). WithVoteInfos(app.voteInfos) - ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx)) - - if mode == runTxModeReCheck { - ctx = ctx.WithIsReCheckTx(true) - } - - if mode == runTxModeSimulate { - ctx, _ = ctx.CacheContext() - } - - return ctx + return ctx.WithConsensusParams(app.GetConsensusParams(ctx)) } // cacheTxContext returns a new context based off of the provided context with @@ -550,6 +544,59 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context return ctx.WithMultiStore(msCache), msCache } +func (app *BaseApp) checkTx(txBytes []byte, tx sdk.Tx, recheck bool) (gInfo sdk.GasInfo, err error) { + ctx := app.getCheckContextForTx(txBytes, recheck) + gasCtx := &ctx + + defer func() { + if r := recover(); r != nil { + recoveryMW := newDefaultRecoveryMiddleware() + err = processRecovery(r, recoveryMW) + } + gInfo = sdk.GasInfo{GasWanted: gasCtx.GasMeter().Limit(), GasUsed: gasCtx.GasMeter().GasConsumed()} + }() + + msgs := tx.GetMsgs() + if err = validateBasicTxMsgs(msgs); err != nil { + return gInfo, err + } + + accKeys := app.accountLock.Lock(ctx, tx) + defer app.accountLock.Unlock(accKeys) + + var anteCtx sdk.Context + anteCtx, err = app.anteTx(ctx, txBytes, tx, false) + if !anteCtx.IsZero() { + gasCtx = &anteCtx + } + + return gInfo, err +} + +func (app *BaseApp) anteTx(ctx sdk.Context, txBytes []byte, tx sdk.Tx, simulate bool) (sdk.Context, error) { + if app.anteHandler == nil { + return ctx, nil + } + + // Branch context before AnteHandler call in case it aborts. + // This is required for both CheckTx and DeliverTx. + // Ref: https://github.com/cosmos/cosmos-sdk/issues/2772 + // + // NOTE: Alternatively, we could require that AnteHandler ensures that + // writes do not happen if aborted/failed. This may have some + // performance benefits, but it'll be more difficult to get right. + anteCtx, msCache := app.cacheTxContext(ctx, txBytes) + anteCtx = anteCtx.WithEventManager(sdk.NewEventManager()) + newCtx, err := app.anteHandler(anteCtx, tx, simulate) + + if err != nil { + return newCtx, err + } + + msCache.Write() + return newCtx, err +} + // runTx processes a transaction within a given execution mode, encoded transaction // bytes, and the decoded transaction itself. All state transitions occur through // a cached Context depending on the mode provided. State only gets persisted @@ -557,33 +604,28 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context // Note, gas execution info is always returned. A reference to a Result is // returned if the tx does not run out of gas and if all the messages are valid // and execute successfully. An error is returned otherwise. -func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, err error) { - // NOTE: GasWanted should be returned by the AnteHandler. GasUsed is - // determined by the GasMeter. We need access to the context to get the gas - // meter so we initialize upfront. - var gasWanted uint64 - - ctx := app.getContextForTx(mode, txBytes) +func (app *BaseApp) runTx(txBytes []byte, tx sdk.Tx, simulate bool) (gInfo sdk.GasInfo, result *sdk.Result, err error) { + ctx := app.getRunContextForTx(txBytes, simulate) ms := ctx.MultiStore() // only run the tx if there is block gas remaining - if mode == runTxModeDeliver && ctx.BlockGasMeter().IsOutOfGas() { + if !simulate && ctx.BlockGasMeter().IsOutOfGas() { gInfo = sdk.GasInfo{GasUsed: ctx.BlockGasMeter().GasConsumed()} return gInfo, nil, sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "no block gas left to run tx") } var startingGas uint64 - if mode == runTxModeDeliver { + if !simulate { startingGas = ctx.BlockGasMeter().GasConsumed() } defer func() { if r := recover(); r != nil { - recoveryMW := newOutOfGasRecoveryMiddleware(gasWanted, ctx, app.runTxRecoveryMiddleware) + recoveryMW := newOutOfGasRecoveryMiddleware(ctx.GasMeter().Limit(), ctx, app.runTxRecoveryMiddleware) err, result = processRecovery(r, recoveryMW), nil } - gInfo = sdk.GasInfo{GasWanted: gasWanted, GasUsed: ctx.GasMeter().GasConsumed()} + gInfo = sdk.GasInfo{GasWanted: ctx.GasMeter().Limit(), GasUsed: ctx.GasMeter().GasConsumed()} }() // If BlockGasMeter() panics it will be caught by the above recover and will @@ -592,7 +634,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // NOTE: This must exist in a separate defer function for the above recovery // to recover from this one. defer func() { - if mode == runTxModeDeliver { + if !simulate { ctx.BlockGasMeter().ConsumeGas( ctx.GasMeter().GasConsumedToLimit(), "block gas meter", ) @@ -603,54 +645,27 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re } }() - tx, err := app.txDecoder(txBytes) - if err != nil { - return sdk.GasInfo{}, nil, err - } - msgs := tx.GetMsgs() - if err := validateBasicTxMsgs(msgs); err != nil { + if err = validateBasicTxMsgs(msgs); err != nil { return sdk.GasInfo{}, nil, err } - var events sdk.Events - if app.anteHandler != nil { - var ( - anteCtx sdk.Context - msCache sdk.CacheMultiStore - ) - - // Branch context before AnteHandler call in case it aborts. - // This is required for both CheckTx and DeliverTx. - // Ref: https://github.com/cosmos/cosmos-sdk/issues/2772 + var newCtx sdk.Context + newCtx, err = app.anteTx(ctx, txBytes, tx, simulate) + if !newCtx.IsZero() { + // At this point, newCtx.MultiStore() is a store branch, or something else + // replaced by the AnteHandler. We want the original multistore. // - // NOTE: Alternatively, we could require that AnteHandler ensures that - // writes do not happen if aborted/failed. This may have some - // performance benefits, but it'll be more difficult to get right. - anteCtx, msCache = app.cacheTxContext(ctx, txBytes) - anteCtx = anteCtx.WithEventManager(sdk.NewEventManager()) - newCtx, err := app.anteHandler(anteCtx, tx, mode == runTxModeSimulate) - - if !newCtx.IsZero() { - // At this point, newCtx.MultiStore() is a store branch, or something else - // replaced by the AnteHandler. We want the original multistore. - // - // Also, in the case of the tx aborting, we need to track gas consumed via - // the instantiated gas meter in the AnteHandler, so we update the context - // prior to returning. - ctx = newCtx.WithMultiStore(ms) - } - - events = ctx.EventManager().Events() - - // GasMeter expected to be set in AnteHandler - gasWanted = ctx.GasMeter().Limit() + // Also, in the case of the tx aborting, we need to track gas consumed via + // the instantiated gas meter in the AnteHandler, so we update the context + // prior to returning. + ctx = newCtx.WithMultiStore(ms) + } - if err != nil { - return gInfo, nil, err - } + events := ctx.EventManager().Events() - msCache.Write() + if err != nil { + return gInfo, nil, err } // Create a new Context based off of the existing Context with a MultiStore branch @@ -661,8 +676,8 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // Attempt to execute all messages and only update state if all messages pass // and we're in DeliverTx. Note, runMsgs will never return a reference to a // Result if any single message fails or does not have a registered Handler. - result, err = app.runMsgs(runMsgCtx, msgs, mode) - if err == nil && mode == runTxModeDeliver { + result, err = app.runMsgs(runMsgCtx, msgs) + if err == nil && !simulate { msCache.Write() if len(events) > 0 { @@ -679,7 +694,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // and DeliverTx. An error is returned if any single message fails or if a // Handler does not exist for a given message route. Otherwise, a reference to a // Result is returned. The caller must not commit state if an error is returned. -func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*sdk.Result, error) { +func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg) (*sdk.Result, error) { msgLogs := make(sdk.ABCIMessageLogs, 0, len(msgs)) events := sdk.EmptyEvents() txMsgData := &sdk.TxMsgData{ @@ -688,11 +703,6 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s // NOTE: GasWanted is determined by the AnteHandler and GasUsed by the GasMeter. for i, msg := range msgs { - // skip actual execution for (Re)CheckTx mode - if mode == runTxModeCheck || mode == runTxModeReCheck { - break - } - var ( msgEvents sdk.Events msgResult *sdk.Result diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 0648ca6f76..49ac80e86b 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -955,6 +955,10 @@ func TestCheckTx(t *testing.T) { app.EndBlock(abci.RequestEndBlock{}) app.Commit() + // Recheck before reviewing `checkStateStore` + app.BeginRecheckTx(abci.RequestBeginRecheckTx{Header: header}) + app.EndRecheckTx(abci.RequestEndRecheckTx{}) + checkStateStore = app.checkState.ctx.KVStore(capKey1) storedBytes := checkStateStore.Get(counterKey) require.Nil(t, storedBytes) @@ -1284,7 +1288,7 @@ func TestTxGasLimits(t *testing.T) { } }() - count := tx.(txTest).Counter + count := tx.(*txTest).Counter newCtx.GasMeter().ConsumeGas(uint64(count), "counter-ante") return newCtx, nil @@ -1294,7 +1298,7 @@ func TestTxGasLimits(t *testing.T) { routerOpt := func(bapp *BaseApp) { r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) { - count := msg.(*msgCounter).Counter + count := msg.(msgCounter).Counter ctx.GasMeter().ConsumeGas(uint64(count), "counter-handler") return &sdk.Result{}, nil }) @@ -1369,7 +1373,7 @@ func TestMaxBlockGasLimits(t *testing.T) { } }() - count := tx.(txTest).Counter + count := tx.(*txTest).Counter newCtx.GasMeter().ConsumeGas(uint64(count), "counter-ante") return @@ -1378,7 +1382,7 @@ func TestMaxBlockGasLimits(t *testing.T) { routerOpt := func(bapp *BaseApp) { r := sdk.NewRoute(routeMsgCounter, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) { - count := msg.(*msgCounter).Counter + count := msg.(msgCounter).Counter ctx.GasMeter().ConsumeGas(uint64(count), "counter-handler") return &sdk.Result{}, nil }) @@ -1424,7 +1428,7 @@ func TestMaxBlockGasLimits(t *testing.T) { for j := 0; j < tc.numDelivers; j++ { _, result, err := app.Deliver(aminoTxEncoder(), tx) - ctx := app.getState(runTxModeDeliver).ctx + ctx := app.deliverState.ctx // check for failed transactions if tc.fail && (j+1) > tc.failAfterDeliver { @@ -1527,7 +1531,7 @@ func TestBaseAppAnteHandler(t *testing.T) { require.Empty(t, res.Events) require.False(t, res.IsOK(), fmt.Sprintf("%v", res)) - ctx := app.getState(runTxModeDeliver).ctx + ctx := app.deliverState.ctx store := ctx.KVStore(capKey1) require.Equal(t, int64(0), getIntFromStore(store, anteKey)) @@ -1543,7 +1547,7 @@ func TestBaseAppAnteHandler(t *testing.T) { require.Empty(t, res.Events) require.False(t, res.IsOK(), fmt.Sprintf("%v", res)) - ctx = app.getState(runTxModeDeliver).ctx + ctx = app.deliverState.ctx store = ctx.KVStore(capKey1) require.Equal(t, int64(1), getIntFromStore(store, anteKey)) require.Equal(t, int64(0), getIntFromStore(store, deliverKey)) @@ -1559,7 +1563,7 @@ func TestBaseAppAnteHandler(t *testing.T) { require.NotEmpty(t, res.Events) require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) - ctx = app.getState(runTxModeDeliver).ctx + ctx = app.deliverState.ctx store = ctx.KVStore(capKey1) require.Equal(t, int64(2), getIntFromStore(store, anteKey)) require.Equal(t, int64(1), getIntFromStore(store, deliverKey)) @@ -1678,9 +1682,8 @@ func TestQuery(t *testing.T) { require.Equal(t, 0, len(res.Value)) // query is still empty after a CheckTx - _, resTx, err := app.Check(aminoTxEncoder(), tx) + _, err := app.Check(aminoTxEncoder(), tx) require.NoError(t, err) - require.NotNil(t, resTx) res = app.Query(query) require.Equal(t, 0, len(res.Value)) @@ -1688,7 +1691,7 @@ func TestQuery(t *testing.T) { header := ostproto.Header{Height: app.LastBlockHeight() + 1} app.BeginBlock(abci.RequestBeginBlock{Header: header}) - _, resTx, err = app.Deliver(aminoTxEncoder(), tx) + _, resTx, err := app.Deliver(aminoTxEncoder(), tx) require.NoError(t, err) require.NotNil(t, resTx) res = app.Query(query) diff --git a/baseapp/test_helpers.go b/baseapp/test_helpers.go index afebbfe927..d8631a76d8 100644 --- a/baseapp/test_helpers.go +++ b/baseapp/test_helpers.go @@ -7,35 +7,40 @@ import ( sdkerrors "github.com/line/lbm-sdk/v2/types/errors" ) -func (app *BaseApp) Check(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo, *sdk.Result, error) { +func (app *BaseApp) Check(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo, error) { // runTx expects tx bytes as argument, so we encode the tx argument into // bytes. Note that runTx will actually decode those bytes again. But since // this helper is only used in tests/simulation, it's fine. - bz, err := txEncoder(tx) + txBytes, err := txEncoder(tx) if err != nil { - return sdk.GasInfo{}, nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err) + return sdk.GasInfo{}, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err) } - return app.runTx(runTxModeCheck, bz) + return app.checkTx(txBytes, tx, false) } func (app *BaseApp) Simulate(txBytes []byte) (sdk.GasInfo, *sdk.Result, error) { - return app.runTx(runTxModeSimulate, txBytes) + tx, err := app.txDecoder(txBytes) + if err != nil { + return sdk.GasInfo{}, nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err) + } + return app.runTx(txBytes, tx, true) } func (app *BaseApp) Deliver(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo, *sdk.Result, error) { // See comment for Check(). - bz, err := txEncoder(tx) + txBytes, err := txEncoder(tx) if err != nil { return sdk.GasInfo{}, nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err) } - return app.runTx(runTxModeDeliver, bz) + return app.runTx(txBytes, tx, false) } // Context with current {check, deliver}State of the app used by tests. func (app *BaseApp) NewContext(isCheckTx bool, header ostproto.Header) sdk.Context { if isCheckTx { - return sdk.NewContext(app.checkState.ms, header, true, app.logger). + ctx := sdk.NewContext(app.checkState.ms, header, true, app.logger). WithMinGasPrices(app.minGasPrices) + return ctx.WithConsensusParams(app.GetConsensusParams(ctx)) } return sdk.NewContext(app.deliverState.ms, header, false, app.logger) diff --git a/go.mod b/go.mod index bcea7f3487..af7ab7203c 100644 --- a/go.mod +++ b/go.mod @@ -25,9 +25,9 @@ require ( github.com/grpc-ecosystem/go-grpc-middleware v1.2.2 github.com/grpc-ecosystem/grpc-gateway v1.16.0 github.com/hashicorp/golang-lru v0.5.4 - github.com/line/iavl/v2 v2.0.0-init.1.0.20210406065347-cfd73e5acce0 - github.com/line/ostracon v0.34.9-0.20210406083837-4183d649b30c - github.com/line/tm-db/v2 v2.0.0-init.1.0.20210406062110-9424ca70955a + github.com/line/iavl/v2 v2.0.0-init.1.0.20210419041411-7de35b5f1306 + github.com/line/ostracon v0.34.9-0.20210419031811-5254cabf172e + github.com/line/tm-db/v2 v2.0.0-init.1.0.20210413083915-5bb60e117524 github.com/magiconair/properties v1.8.4 github.com/mattn/go-isatty v0.0.12 github.com/otiai10/copy v1.4.2 diff --git a/go.sum b/go.sum index 7ca1e6d707..b505056b41 100644 --- a/go.sum +++ b/go.sum @@ -339,17 +339,21 @@ github.com/line/gorocksdb v0.0.0-20210406043732-d4bea34b6d55 h1:cXVtMiJkvQ4kn0px github.com/line/gorocksdb v0.0.0-20210406043732-d4bea34b6d55/go.mod h1:DHRJroSL7NaRkpvocRx3OtRsleXVsYSxBI9SfHFlTQ0= github.com/line/iavl/v2 v2.0.0-init.1.0.20210406065347-cfd73e5acce0 h1:Pcp/mxkvFmasaiPi9DS2ZXCKOalfDGtBKYjdswmF0nI= github.com/line/iavl/v2 v2.0.0-init.1.0.20210406065347-cfd73e5acce0/go.mod h1:Z2UTxsbKefd7bnEywGNxxmVYYGy9Ecd4nd1IZyR0bQM= +github.com/line/iavl/v2 v2.0.0-init.1.0.20210419041411-7de35b5f1306 h1:1wc5BD4kY1v5GZxMq3HfE6dCAPUu6pHqC3PZ1ZEiwVs= +github.com/line/iavl/v2 v2.0.0-init.1.0.20210419041411-7de35b5f1306/go.mod h1:cGVW9rgqmnLU+9ShXC/RWwmfmZu56PyzuwvdfJ04ryg= github.com/line/lbm-sdk v0.39.2-0.2.0 h1:tlQHZcf+AXejSBAhYgO5Tn1NaoCF0XRNf/6c0jeo3HE= github.com/line/lbm-sdk v0.39.2-0.2.0/go.mod h1:UTxdYWx+OeRezEP8P5BxipddlFpq4q92uYydSeYN7B0= github.com/line/linemint v1.0.0 h1:sTrDaGP67/5Klxtsuid8wOkQ2y43Y5QkmD8cDMmDeaA= github.com/line/linemint v1.0.0/go.mod h1:0yUs9eIuuDq07nQql9BmI30FtYGcEC60Tu5JzB5IezM= github.com/line/ostracon v0.34.9-0.20210315041958-2a1f43c788f5/go.mod h1:1THU+kF+6fxLaNYQKcdNyLCO6t9LnqSMaExDMiLozbM= -github.com/line/ostracon v0.34.9-0.20210406083837-4183d649b30c h1:OYIm2Wu22kFoocMgnjU5O/TIphYLHw9q9+e6AsKPkOA= -github.com/line/ostracon v0.34.9-0.20210406083837-4183d649b30c/go.mod h1:/Ov0QcWRleL6TKmHpPMyIx9roxAc/DD1m3N8xST2b0I= +github.com/line/ostracon v0.34.9-0.20210419031811-5254cabf172e h1:+F5duGTfSZiwD9LqMkdtwFgULjKn8pGRz/86U/SyIO0= +github.com/line/ostracon v0.34.9-0.20210419031811-5254cabf172e/go.mod h1:w/itWXCU8Wttz/2Ftp+yJz0UXv9gX6qT1dASn+3kX5M= github.com/line/tm-db v0.5.2 h1:P8kMpcrm9Xyfl6QLyafssNIoIeC01k0fhw2zDvKhtl4= github.com/line/tm-db v0.5.2/go.mod h1:VrPTx04QJhQ9d8TFUTc2GpPBvBf/U9vIdBIzkjBk7Lk= github.com/line/tm-db/v2 v2.0.0-init.1.0.20210406062110-9424ca70955a h1:qSt/WwORC5+nVRnNqx+A0oo5gOCsoVJ0HmHF5Db1YvY= github.com/line/tm-db/v2 v2.0.0-init.1.0.20210406062110-9424ca70955a/go.mod h1:wmkyPabXjtVZ1dvRofmurjaceghywtCSYGqFuFS+TbI= +github.com/line/tm-db/v2 v2.0.0-init.1.0.20210413083915-5bb60e117524 h1:eKXXnUm1SblC0AIXAMNDaSyvIbQ50yXqFbh9+Q8Kjvg= +github.com/line/tm-db/v2 v2.0.0-init.1.0.20210413083915-5bb60e117524/go.mod h1:wmkyPabXjtVZ1dvRofmurjaceghywtCSYGqFuFS+TbI= github.com/line/wasmvm v0.12.0-0.1.0 h1:Xul8w8pLWZDcp0kkz1Y9M6tfZ4WnmMt9g0U/d6lXdE4= github.com/line/wasmvm v0.12.0-0.1.0/go.mod h1:tbXGE9Jz6sYpiJroGr71OQ5TFOufq/P5LWsruA2u6JE= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= diff --git a/simapp/test_helpers.go b/simapp/test_helpers.go index 720f49e08e..6699fff4ce 100644 --- a/simapp/test_helpers.go +++ b/simapp/test_helpers.go @@ -354,7 +354,7 @@ func SignCheckDeliver( require.Nil(t, res) } - // Simulate a sending a transaction and committing a block + // Simulate a sending a transaction and committing a block and recheck app.BeginBlock(abci.RequestBeginBlock{Header: header}) gInfo, res, err := app.Deliver(txCfg.TxEncoder(), tx) @@ -369,6 +369,9 @@ func SignCheckDeliver( app.EndBlock(abci.RequestEndBlock{}) app.Commit() + app.BeginRecheckTx(abci.RequestBeginRecheckTx{Header: header}) + app.EndRecheckTx(abci.RequestEndRecheckTx{}) + return gInfo, res, err } diff --git a/x/bank/bench_test.go b/x/bank/bench_test.go index 409431b946..ef90849bff 100644 --- a/x/bank/bench_test.go +++ b/x/bank/bench_test.go @@ -46,7 +46,7 @@ func BenchmarkOneBankSendTxPerBlock(b *testing.B) { // Committing, and what time comes from Check/Deliver Tx. for i := 0; i < b.N; i++ { benchmarkApp.BeginBlock(abci.RequestBeginBlock{Header: ostproto.Header{Height: height}}) - _, _, err := benchmarkApp.Check(txGen.TxEncoder(), txs[i]) + _, err := benchmarkApp.Check(txGen.TxEncoder(), txs[i]) if err != nil { panic("something is broken in checking transaction") } @@ -88,7 +88,7 @@ func BenchmarkOneBankMultiSendTxPerBlock(b *testing.B) { // Committing, and what time comes from Check/Deliver Tx. for i := 0; i < b.N; i++ { benchmarkApp.BeginBlock(abci.RequestBeginBlock{Header: ostproto.Header{Height: height}}) - _, _, err := benchmarkApp.Check(txGen.TxEncoder(), txs[i]) + _, err := benchmarkApp.Check(txGen.TxEncoder(), txs[i]) if err != nil { panic("something is broken in checking transaction") } diff --git a/x/genutil/client/cli/init_test.go b/x/genutil/client/cli/init_test.go index ad89a0a4d2..d1fe719355 100644 --- a/x/genutil/client/cli/init_test.go +++ b/x/genutil/client/cli/init_test.go @@ -181,7 +181,7 @@ func TestStartStandAlone(t *testing.T) { svrAddr, _, err := server.FreeTCPAddr() require.NoError(t, err) - svr, err := abci_server.NewServer(svrAddr, "socket", app) + svr, err := abci_server.NewServer(svrAddr, "grpc", app) require.NoError(t, err, "error creating listener") svr.SetLogger(logger.With("module", "abci-server")) diff --git a/x/ibc/testing/chain.go b/x/ibc/testing/chain.go index 5f17ca0ff8..b0c7a05f9a 100644 --- a/x/ibc/testing/chain.go +++ b/x/ibc/testing/chain.go @@ -267,7 +267,14 @@ func (chain *TestChain) NextBlock() { } chain.App.BeginBlock(abci.RequestBeginBlock{Header: chain.CurrentHeader}) +} + +func (chain *TestChain) CommitBlock() { + chain.App.EndBlock(abci.RequestEndBlock{Height: chain.CurrentHeader.Height}) + chain.App.Commit() + chain.App.BeginRecheckTx(abci.RequestBeginRecheckTx{Header: chain.CurrentHeader}) + chain.App.EndRecheckTx(abci.RequestEndRecheckTx{Height: chain.CurrentHeader.Height}) } // sendMsgs delivers a transaction through the application without returning the result. @@ -735,7 +742,7 @@ func (chain *TestChain) CreatePortCapability(portID string) { } } - chain.App.Commit() + chain.CommitBlock() chain.NextBlock() } @@ -762,7 +769,7 @@ func (chain *TestChain) CreateChannelCapability(portID, channelID string) { require.NoError(chain.t, err) } - chain.App.Commit() + chain.CommitBlock() chain.NextBlock() } @@ -882,7 +889,7 @@ func (chain *TestChain) SendPacket( } // commit changes - chain.App.Commit() + chain.CommitBlock() chain.NextBlock() return nil @@ -901,7 +908,7 @@ func (chain *TestChain) WriteAcknowledgement( } // commit changes - chain.App.Commit() + chain.CommitBlock() chain.NextBlock() return nil diff --git a/x/ibc/testing/coordinator.go b/x/ibc/testing/coordinator.go index 963594ad22..df69cb70a8 100644 --- a/x/ibc/testing/coordinator.go +++ b/x/ibc/testing/coordinator.go @@ -374,7 +374,7 @@ func GetChainID(index int) string { // CONTRACT: the passed in list of indexes must not contain duplicates func (coord *Coordinator) CommitBlock(chains ...*TestChain) { for _, chain := range chains { - chain.App.Commit() + chain.CommitBlock() chain.NextBlock() } coord.IncrementTime() @@ -382,9 +382,10 @@ func (coord *Coordinator) CommitBlock(chains ...*TestChain) { // CommitNBlocks commits n blocks to state and updates the block height by 1 for each commit. func (coord *Coordinator) CommitNBlocks(chain *TestChain, n uint64) { + chain.App.BeginBlock(abci.RequestBeginBlock{Header: chain.CurrentHeader}) + for i := uint64(0); i < n; i++ { - chain.App.BeginBlock(abci.RequestBeginBlock{Header: chain.CurrentHeader}) - chain.App.Commit() + chain.CommitBlock() chain.NextBlock() coord.IncrementTime() }