From 2496db78df18d66bad11942312b46ee9016659f3 Mon Sep 17 00:00:00 2001 From: jinoosss <112360739+jinoosss@users.noreply.github.com> Date: Wed, 4 Dec 2024 22:32:01 +0900 Subject: [PATCH] fix: Modify `app` path method to simulate for ABCI query (#3207) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Descriptions To simulate transactions, utilize the `.app/simulate` method for ABCI Query. ### Changes 1. change the path of ABCI Query's `.app` to the result data storage location. - You can receive the query result data as `RequestQuery.ResponseData.Data` instead of `RequestQuery.Value`. - Provide it in a common form with other ABCI Queries. 2. remove the gas-consume logic of mocking signature data that is executed when simulating transactions ([/tm2/pkg/sdk/auth/ante.go#L231-L237](https://github.com/gnolang/gno/blob/master/tm2/pkg/sdk/auth/ante.go#L231-L237)) - We will get the correct value when simulating a real transaction. - We want transactions to run without signatures, but we already have checks in place to see if a signature exists. ([tm2/pkg/sdk/auth/ante.go#L104-L106](https://github.com/gnolang/gno/blob/master/tm2/pkg/sdk/auth/ante.go#L104-L106)) ### Example #### [Request Simulate] ```curl curl --location 'http://localhost:26657' \ --header 'Content-Type: application/json' \ --data '{ "id": 1, "jsonrpc": "2.0", "method": "abci_query", "params": [ ".app/simulate", "CnMKDS9iYW5rLk1zZ1NlbmQSYgooZzFqZzhtdHV0dTlraGhmd2M0bnhtdWhjcGZ0ZjBwYWpkaGZ2c3FmNRIoZzFmZnp4aGE1N2RoMHFndjltYTV2MzkzdXIwemV4ZnZwNmxzanBhZRoMNTAwMDAwMHVnbm90Eg4IgIl6EggzMDB1Z25vdBp+CjoKEy90bS5QdWJLZXlTZWNwMjU2azESIwohA+FhNtsXHjLfSJk1lB8FbiL4mGPjc50Kt81J7EKDnJ2yEkCrIOTBt7YcDGcQ6Ohfv1r3nftAPaTATAtPfYD5zLQf7WDf1KPvWARe//CANtLLtIzcPVl7P/HnHxmfCYEwfGogIgUxMjMxMw==", "0", false ] }' ``` #### [Response] ```curl { "jsonrpc": "2.0", "id": 1, "result": { "response": { "ResponseBase": { "Error": null, "Data": "eyJFcnJvciI6bnVsbCwiRGF0YSI6IiIsIkV2ZW50cyI6W10sIkxvZyI6Im1zZzowLHN1Y2Nlc3M6dHJ1ZSxsb2c6LGV2ZW50czpbXSIsIkluZm8iOiIiLCJHYXNXYW50ZWQiOjEwMDAwMDAsIkdhc1VzZWQiOjQ0NjI5fQ==", "Events": null, "Log": "", "Info": "" }, "Key": null, "Value": null, "Proof": null, "Height": "0" } } } ``` ### Related Issue - https://github.com/gnolang/gno/issues/1826
Contributors' checklist... - [x] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests
--------- Co-authored-by: n3wbie Co-authored-by: Miloš Živković --- .../cmd/gnoland/testdata/simulate_gas.txtar | 28 ++++++++++++ tm2/pkg/sdk/auth/ante.go | 45 +------------------ tm2/pkg/sdk/auth/ante_test.go | 5 ++- tm2/pkg/sdk/baseapp.go | 10 ++++- tm2/pkg/sdk/baseapp_test.go | 34 +++++++++++++- 5 files changed, 75 insertions(+), 47 deletions(-) create mode 100644 gno.land/cmd/gnoland/testdata/simulate_gas.txtar diff --git a/gno.land/cmd/gnoland/testdata/simulate_gas.txtar b/gno.land/cmd/gnoland/testdata/simulate_gas.txtar new file mode 100644 index 00000000000..cd58b4ccc8f --- /dev/null +++ b/gno.land/cmd/gnoland/testdata/simulate_gas.txtar @@ -0,0 +1,28 @@ +# load the package +loadpkg gno.land/r/simulate $WORK/simulate + +# start a new node +gnoland start + +# simulate only +gnokey maketx call -pkgpath gno.land/r/simulate -func Hello -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -simulate only test1 +stdout 'GAS USED: 50299' + +# simulate skip +gnokey maketx call -pkgpath gno.land/r/simulate -func Hello -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -simulate skip test1 +stdout 'GAS USED: 50299' # same as simulate only + + +-- package/package.gno -- +package call_package + +func Render() string { + return "notok" +} + +-- simulate/simulate.gno -- +package simulate + +func Hello() string { + return "Hello" +} diff --git a/tm2/pkg/sdk/auth/ante.go b/tm2/pkg/sdk/auth/ante.go index 49662b47a55..d36b376aa8d 100644 --- a/tm2/pkg/sdk/auth/ante.go +++ b/tm2/pkg/sdk/auth/ante.go @@ -15,11 +15,8 @@ import ( "github.com/gnolang/gno/tm2/pkg/store" ) -var ( - // simulation signature values used to estimate gas consumption - simSecp256k1Pubkey secp256k1.PubKeySecp256k1 - simSecp256k1Sig [64]byte -) +// simulation signature values used to estimate gas consumption +var simSecp256k1Pubkey secp256k1.PubKeySecp256k1 func init() { // This decodes a valid hex string into a sepc256k1Pubkey for use in transaction simulation @@ -228,14 +225,6 @@ func processSig( return nil, abciResult(std.ErrInternal("setting PubKey on signer's account")) } - if simulate { - // Simulated txs should not contain a signature and are not required to - // contain a pubkey, so we must account for tx size of including a - // std.Signature (Amino encoding) and simulate gas consumption - // (assuming a SECP256k1 simulation key). - consumeSimSigGas(ctx.GasMeter(), pubKey, sig, params) - } - if res := sigGasConsumer(ctx.GasMeter(), sig.Signature, pubKey, params); !res.IsOK() { return nil, res } @@ -251,42 +240,12 @@ func processSig( return acc, res } -func consumeSimSigGas(gasmeter store.GasMeter, pubkey crypto.PubKey, sig std.Signature, params Params) { - simSig := std.Signature{PubKey: pubkey} - if len(sig.Signature) == 0 { - simSig.Signature = simSecp256k1Sig[:] - } - - sigBz := amino.MustMarshalSized(simSig) - cost := store.Gas(len(sigBz) + 6) - - // If the pubkey is a multi-signature pubkey, then we estimate for the maximum - // number of signers. - if _, ok := pubkey.(multisig.PubKeyMultisigThreshold); ok { - cost *= params.TxSigLimit - } - - gasmeter.ConsumeGas(params.TxSizeCostPerByte*cost, "txSize") -} - // ProcessPubKey verifies that the given account address matches that of the // std.Signature. In addition, it will set the public key of the account if it // has not been set. func ProcessPubKey(acc std.Account, sig std.Signature, simulate bool) (crypto.PubKey, sdk.Result) { // If pubkey is not known for account, set it from the std.Signature. pubKey := acc.GetPubKey() - if simulate { - // In simulate mode the transaction comes with no signatures, thus if the - // account's pubkey is nil, both signature verification and gasKVStore.Set() - // shall consume the largest amount, i.e. it takes more gas to verify - // secp256k1 keys than ed25519 ones. - if pubKey == nil { - return simSecp256k1Pubkey, sdk.Result{} - } - - return pubKey, sdk.Result{} - } - if pubKey == nil { pubKey = sig.PubKey if pubKey == nil { diff --git a/tm2/pkg/sdk/auth/ante_test.go b/tm2/pkg/sdk/auth/ante_test.go index be4167a6238..86e34391770 100644 --- a/tm2/pkg/sdk/auth/ante_test.go +++ b/tm2/pkg/sdk/auth/ante_test.go @@ -611,10 +611,11 @@ func TestProcessPubKey(t *testing.T) { wantErr bool }{ {"no sigs, simulate off", args{acc1, std.Signature{}, false}, true}, - {"no sigs, simulate on", args{acc1, std.Signature{}, true}, false}, + {"no sigs, simulate on", args{acc1, std.Signature{}, true}, true}, + {"no sigs, account with pub, simulate off", args{acc2, std.Signature{}, false}, false}, {"no sigs, account with pub, simulate on", args{acc2, std.Signature{}, true}, false}, {"pubkey doesn't match addr, simulate off", args{acc1, std.Signature{PubKey: priv2.PubKey()}, false}, true}, - {"pubkey doesn't match addr, simulate on", args{acc1, std.Signature{PubKey: priv2.PubKey()}, true}, false}, + {"pubkey doesn't match addr, simulate on", args{acc1, std.Signature{PubKey: priv2.PubKey()}, true}, true}, } for _, tt := range tests { tt := tt diff --git a/tm2/pkg/sdk/baseapp.go b/tm2/pkg/sdk/baseapp.go index 415309eab9a..1802a21f453 100644 --- a/tm2/pkg/sdk/baseapp.go +++ b/tm2/pkg/sdk/baseapp.go @@ -409,8 +409,16 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc } else { result = app.Simulate(txBytes, tx) } + res.Height = req.Height - res.Value = amino.MustMarshal(result) + + bytes, err := amino.Marshal(result) + if err != nil { + res.Error = ABCIError(std.ErrInternal(fmt.Sprintf("cannot encode to JSON: %s", err))) + } else { + res.Value = bytes + } + return res case "version": res.Height = req.Height diff --git a/tm2/pkg/sdk/baseapp_test.go b/tm2/pkg/sdk/baseapp_test.go index 08e8191170a..cf944c44f06 100644 --- a/tm2/pkg/sdk/baseapp_test.go +++ b/tm2/pkg/sdk/baseapp_test.go @@ -634,6 +634,38 @@ func TestDeliverTx(t *testing.T) { } } +// Test that the gas used between Simulate and DeliverTx is the same. +func TestGasUsedBetweenSimulateAndDeliver(t *testing.T) { + t.Parallel() + + anteKey := []byte("ante-key") + anteOpt := func(bapp *BaseApp) { bapp.SetAnteHandler(anteHandlerTxTest(t, mainKey, anteKey)) } + + deliverKey := []byte("deliver-key") + routerOpt := func(bapp *BaseApp) { + bapp.Router().AddRoute(routeMsgCounter, newMsgCounterHandler(t, mainKey, deliverKey)) + } + + app := setupBaseApp(t, anteOpt, routerOpt) + app.InitChain(abci.RequestInitChain{ChainID: "test-chain"}) + + header := &bft.Header{ChainID: "test-chain", Height: 1} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + + tx := newTxCounter(0, 0) + txBytes, err := amino.Marshal(tx) + require.Nil(t, err) + + simulateRes := app.Simulate(txBytes, tx) + require.True(t, simulateRes.IsOK(), fmt.Sprintf("%v", simulateRes)) + require.Greater(t, simulateRes.GasUsed, int64(0)) // gas used should be greater than 0 + + deliverRes := app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes}) + require.True(t, deliverRes.IsOK(), fmt.Sprintf("%v", deliverRes)) + + require.Equal(t, simulateRes.GasUsed, deliverRes.GasUsed) // gas used should be the same from simulate and deliver +} + // One call to DeliverTx should process all the messages, in order. func TestMultiMsgDeliverTx(t *testing.T) { t.Parallel() @@ -753,7 +785,7 @@ func TestSimulateTx(t *testing.T) { require.True(t, queryResult.IsOK(), queryResult.Log) var res Result - amino.MustUnmarshal(queryResult.Value, &res) + require.NoError(t, amino.Unmarshal(queryResult.Value, &res)) require.Nil(t, err, "Result unmarshalling failed") require.True(t, res.IsOK(), res.Log) require.Equal(t, gasConsumed, res.GasUsed, res.Log)