Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Modify app path method to simulate for ABCI query #3207

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
} 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)))

Check warning on line 417 in tm2/pkg/sdk/baseapp.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/sdk/baseapp.go#L417

Added line #L417 was not covered by tests
} 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
Loading