From 1bbf4bb86bd4eb462b0b129351dace550af0e517 Mon Sep 17 00:00:00 2001 From: codchen Date: Mon, 27 Nov 2023 13:40:27 +0800 Subject: [PATCH] make BuildDependencyGraph use typed tx --- baseapp/baseapp.go | 4 + baseapp/deliver_tx_test.go | 3 +- x/accesscontrol/keeper/keeper.go | 8 +- x/accesscontrol/keeper/keeper_test.go | 139 ++++++++++---------------- 4 files changed, 62 insertions(+), 92 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 8460038ee..9f8aee071 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -914,6 +914,10 @@ func (app *BaseApp) runTx(ctx sdk.Context, mode runTxMode, tx sdk.Tx, checksum [ defer consumeBlockGas() } + if tx == nil { + return sdk.GasInfo{}, nil, nil, 0, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "tx decode error") + } + msgs := tx.GetMsgs() if err := validateBasicTxMsgs(msgs); err != nil { diff --git a/baseapp/deliver_tx_test.go b/baseapp/deliver_tx_test.go index 21643a2b4..9307a964f 100644 --- a/baseapp/deliver_tx_test.go +++ b/baseapp/deliver_tx_test.go @@ -1526,7 +1526,8 @@ func TestDeliverTx(t *testing.T) { txBytes, err := codec.Marshal(tx) require.NoError(t, err) - res := app.DeliverTx(app.deliverState.ctx, abci.RequestDeliverTx{Tx: txBytes}, tx, sha256.Sum256(txBytes)) + decoded, _ := app.txDecoder(txBytes) + res := app.DeliverTx(app.deliverState.ctx, abci.RequestDeliverTx{Tx: txBytes}, decoded, sha256.Sum256(txBytes)) require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) events := res.GetEvents() require.Len(t, events, 3, "should contain ante handler, message type and counter events respectively") diff --git a/x/accesscontrol/keeper/keeper.go b/x/accesscontrol/keeper/keeper.go index 137d7ba3e..2c9cbc461 100644 --- a/x/accesscontrol/keeper/keeper.go +++ b/x/accesscontrol/keeper/keeper.go @@ -493,15 +493,11 @@ func (k Keeper) IterateWasmDependencies(ctx sdk.Context, handler func(wasmDepend } } -func (k Keeper) BuildDependencyDag(ctx sdk.Context, txDecoder sdk.TxDecoder, anteDepGen sdk.AnteDepGenerator, txs [][]byte) (*types.Dag, error) { +func (k Keeper) BuildDependencyDag(ctx sdk.Context, anteDepGen sdk.AnteDepGenerator, txs []sdk.Tx) (*types.Dag, error) { defer MeasureBuildDagDuration(time.Now(), "BuildDependencyDag") // contains the latest msg index for a specific Access Operation dependencyDag := types.NewDag() - for txIndex, txBytes := range txs { - tx, err := txDecoder(txBytes) // TODO: results in repetitive decoding for txs with runtx decode (potential optimization) - if err != nil { - return nil, err - } + for txIndex, tx := range txs { // get the ante dependencies and add them to the dag anteDeps, err := anteDepGen([]acltypes.AccessOperation{}, tx, txIndex) if err != nil { diff --git a/x/accesscontrol/keeper/keeper_test.go b/x/accesscontrol/keeper/keeper_test.go index f08cd1ade..ae30a7930 100644 --- a/x/accesscontrol/keeper/keeper_test.go +++ b/x/accesscontrol/keeper/keeper_test.go @@ -1998,13 +1998,11 @@ func TestBuildDependencyDag(t *testing.T) { txBuilder := simapp.MakeTestEncodingConfig().TxConfig.NewTxBuilder() err := txBuilder.SetMsgs(msgs...) require.NoError(t, err) - bz, err := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) - require.NoError(t, err) - txs := [][]byte{ - bz, + txs := []sdk.Tx{ + txBuilder.GetTx(), } // ensure no errors creating dag - _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), app.GetAnteDepGenerator(), txs) + _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, app.GetAnteDepGenerator(), txs) require.NoError(t, err) } @@ -2021,13 +2019,11 @@ func TestBuildDependencyDagWithGovMessage(t *testing.T) { txBuilder := simapp.MakeTestEncodingConfig().TxConfig.NewTxBuilder() err := txBuilder.SetMsgs(msgs...) require.NoError(t, err) - bz, err := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) - require.NoError(t, err) - txs := [][]byte{ - bz, + txs := []sdk.Tx{ + txBuilder.GetTx(), } // ensure no errors creating dag - _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), app.GetAnteDepGenerator(), txs) + _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, app.GetAnteDepGenerator(), txs) require.ErrorIs(t, err, types.ErrGovMsgInBlock) } @@ -2047,13 +2043,11 @@ func TestBuildDependencyDag_GovPropMessage(t *testing.T) { txBuilder := simapp.MakeTestEncodingConfig().TxConfig.NewTxBuilder() err := txBuilder.SetMsgs(msgs...) require.NoError(t, err) - bz, err := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) - require.NoError(t, err) - txs := [][]byte{ - bz, + txs := []sdk.Tx{ + txBuilder.GetTx(), } // expect ErrGovMsgInBlock - _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), app.GetAnteDepGenerator(), txs) + _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, app.GetAnteDepGenerator(), txs) require.EqualError(t, err, types.ErrGovMsgInBlock.Error()) } @@ -2071,13 +2065,11 @@ func TestBuildDependencyDag_GovDepositMessage(t *testing.T) { txBuilder := simapp.MakeTestEncodingConfig().TxConfig.NewTxBuilder() err := txBuilder.SetMsgs(msgs...) require.NoError(t, err) - bz, err := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) - require.NoError(t, err) - txs := [][]byte{ - bz, + txs := []sdk.Tx{ + txBuilder.GetTx(), } // expect ErrGovMsgInBlock - _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), app.GetAnteDepGenerator(), txs) + _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, app.GetAnteDepGenerator(), txs) require.EqualError(t, err, types.ErrGovMsgInBlock.Error()) } @@ -2098,49 +2090,28 @@ func TestBuildDependencyDag_MultipleTransactions(t *testing.T) { txBuilder := simapp.MakeTestEncodingConfig().TxConfig.NewTxBuilder() err := txBuilder.SetMsgs(msgs1...) require.NoError(t, err) - bz1, err := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) - require.NoError(t, err) + tx1 := txBuilder.GetTx() err = txBuilder.SetMsgs(msgs2...) require.NoError(t, err) - bz2, err := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) - require.NoError(t, err) + tx2 := txBuilder.GetTx() - txs := [][]byte{ - bz1, - bz2, + txs := []sdk.Tx{ + tx1, + tx2, } - _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), app.GetAnteDepGenerator(), txs) + _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, app.GetAnteDepGenerator(), txs) require.NoError(t, err) mockAnteDepGenerator := func(_ []acltypes.AccessOperation, _ sdk.Tx, _ int) ([]acltypes.AccessOperation, error) { return nil, errors.New("Mocked error") } - _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), mockAnteDepGenerator, txs) + _, err = app.AccessControlKeeper.BuildDependencyDag(ctx, mockAnteDepGenerator, txs) require.ErrorContains(t, err, "Mocked error") } -func TestBuildDependencyDag_DecoderError(t *testing.T) { - // Set up a mocked app with a failing decoder - app := simapp.Setup(false) - ctx := app.BaseApp.NewContext(false, tmproto.Header{}) - - // Encode an invalid transaction - txs := [][]byte{ - []byte("invalid tx"), - } - - _, err := app.AccessControlKeeper.BuildDependencyDag( - ctx, - simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), - app.GetAnteDepGenerator(), - txs, - ) - require.Error(t, err) -} - -func BencharkAccessOpsBuildDependencyDag(b *testing.B) { +func BenchmarkAccessOpsBuildDependencyDag(b *testing.B) { app := simapp.Setup(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) @@ -2156,30 +2127,30 @@ func BencharkAccessOpsBuildDependencyDag(b *testing.B) { txBuilder := simapp.MakeTestEncodingConfig().TxConfig.NewTxBuilder() _ = txBuilder.SetMsgs(msgs1...) - bz1, _ := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) + tx1 := txBuilder.GetTx() _ = txBuilder.SetMsgs(msgs2...) - bz2, _ := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) - - txs := [][]byte{ - bz1, - bz1, - bz1, - bz1, - bz1, - bz1, - bz2, - bz2, - bz2, - bz2, - bz2, - bz2, - bz2, - bz2, - bz2, - bz2, - bz2, - bz2, + tx2 := txBuilder.GetTx() + + txs := []sdk.Tx{ + tx1, + tx1, + tx1, + tx1, + tx1, + tx1, + tx2, + tx2, + tx2, + tx2, + tx2, + tx2, + tx2, + tx2, + tx2, + tx2, + tx2, + tx2, } mockAnteDepGenerator := func(_ []acltypes.AccessOperation, _ sdk.Tx, _ int) ([]acltypes.AccessOperation, error) { @@ -2235,7 +2206,7 @@ func BencharkAccessOpsBuildDependencyDag(b *testing.B) { for i := 0; i < b.N; i++ { _, _ = app.AccessControlKeeper.BuildDependencyDag( - ctx, simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), mockAnteDepGenerator, txs) + ctx, mockAnteDepGenerator, txs) } } @@ -2256,21 +2227,19 @@ func TestInvalidAccessOpsBuildDependencyDag(t *testing.T) { txBuilder := simapp.MakeTestEncodingConfig().TxConfig.NewTxBuilder() err := txBuilder.SetMsgs(msgs1...) require.NoError(t, err) - bz1, err := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) - require.NoError(t, err) + tx1 := txBuilder.GetTx() err = txBuilder.SetMsgs(msgs2...) require.NoError(t, err) - bz2, err := simapp.MakeTestEncodingConfig().TxConfig.TxEncoder()(txBuilder.GetTx()) - require.NoError(t, err) + tx2 := txBuilder.GetTx() - txs := [][]byte{ - bz1, - bz2, - bz2, - bz2, - bz2, - bz2, + txs := []sdk.Tx{ + tx1, + tx2, + tx2, + tx2, + tx2, + tx2, } mockAnteDepGenerator := func(_ []acltypes.AccessOperation, _ sdk.Tx, _ int) ([]acltypes.AccessOperation, error) { @@ -2285,7 +2254,7 @@ func TestInvalidAccessOpsBuildDependencyDag(t *testing.T) { // ensure no errors creating dag _, err = app.AccessControlKeeper.BuildDependencyDag( - ctx, simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), mockAnteDepGenerator, txs) + ctx, mockAnteDepGenerator, txs) require.Error(t, err) mockAnteDepGenerator = func(_ []acltypes.AccessOperation, _ sdk.Tx, _ int) ([]acltypes.AccessOperation, error) { @@ -2301,7 +2270,7 @@ func TestInvalidAccessOpsBuildDependencyDag(t *testing.T) { // ensure no errors creating dag _, err = app.AccessControlKeeper.BuildDependencyDag( - ctx, simapp.MakeTestEncodingConfig().TxConfig.TxDecoder(), mockAnteDepGenerator, txs) + ctx, mockAnteDepGenerator, txs) require.NoError(t, err) }