Skip to content

Commit

Permalink
fix: Modify app path method to simulate for ABCI query (gnolang#3207)
Browse files Browse the repository at this point in the history
<!-- please provide a detailed description of the changes made in this
pull request. -->
## 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
- gnolang#1826

<details><summary>Contributors' checklist...</summary>

- [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
</details>

---------

Co-authored-by: n3wbie <[email protected]>
Co-authored-by: Miloš Živković <[email protected]>
  • Loading branch information
3 people committed Dec 10, 2024
1 parent 9156613 commit 936ade0
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 47 deletions.
28 changes: 28 additions & 0 deletions gno.land/cmd/gnoland/testdata/simulate_gas.txtar
Original file line number Diff line number Diff line change
@@ -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"
}
45 changes: 2 additions & 43 deletions tm2/pkg/sdk/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions tm2/pkg/sdk/auth/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion tm2/pkg/sdk/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 33 additions & 1 deletion tm2/pkg/sdk/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 936ade0

Please sign in to comment.