From 72a94d0443989e28ac65acc3d7e88680aff7d862 Mon Sep 17 00:00:00 2001 From: 0xTopaz Date: Tue, 26 Nov 2024 23:30:39 +0900 Subject: [PATCH 1/8] fix: deduct gas fee after runTx --- .../cmd/gnoland/testdata/deduct_gasfee.txtar | 22 ++++++++++ gno.land/pkg/gnoland/app.go | 8 ++++ tm2/pkg/sdk/auth/ante.go | 40 ++++++++++++----- tm2/pkg/sdk/auth/ante_test.go | 43 ------------------- tm2/pkg/sdk/baseapp.go | 10 +++++ tm2/pkg/sdk/options.go | 7 +++ tm2/pkg/sdk/types.go | 3 ++ 7 files changed, 79 insertions(+), 54 deletions(-) create mode 100644 gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar diff --git a/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar b/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar new file mode 100644 index 00000000000..d1e366f03cf --- /dev/null +++ b/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar @@ -0,0 +1,22 @@ +# load the package +loadpkg gno.land/r/demo/foo20 + +# start a new node +gnoland start + +# 1. check previous balance +gnokey query bank/balances/g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5 +stdout 'height: 0' +stdout 'data: "10000000000000ugnot"' + +# 2. execute the Faucet function +gnokey maketx call -pkgpath gno.land/r/demo/foo20 -func Faucet -broadcast=true -chainid=tendermint_test -gas-fee 1ugnot -gas-wanted 100000000 -memo "" test1 + +## check output +stdout OK! +stdout 'GAS USED: 748508' + +# 3. check the balance after the Faucet function +gnokey query bank/balances/g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5 +stdout 'height: 0' +stdout 'data: "9999999251492ugnot"' diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index e0c93f6194f..e4d77656413 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -120,6 +120,14 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { return }, ) + // Set GasFeeCollector + gasFeeCollector := auth.NewGasFeeCollector(acctKpr, bankKpr) + baseApp.SetGasFeeCollector(func(ctx sdk.Context, tx std.Tx, gasUsed int64) ( + res sdk.Result, + ) { + res = gasFeeCollector(ctx, tx, gasUsed) + return + }) // Set begin and end transaction hooks. // These are used to create gno transaction stores and commit them when finishing diff --git a/tm2/pkg/sdk/auth/ante.go b/tm2/pkg/sdk/auth/ante.go index 49662b47a55..fd76c292f1e 100644 --- a/tm2/pkg/sdk/auth/ante.go +++ b/tm2/pkg/sdk/auth/ante.go @@ -123,17 +123,6 @@ func NewAnteHandler(ak AccountKeeper, bank BankKeeperI, sigGasConsumer Signature return newCtx, res, true } - // deduct the fees - if !tx.Fee.GasFee.IsZero() { - res = DeductFees(bank, newCtx, signerAccs[0], std.Coins{tx.Fee.GasFee}) - if !res.IsOK() { - return newCtx, res, true - } - - // reload the account as fees have been deducted - signerAccs[0] = ak.GetAccount(newCtx, signerAccs[0].GetAddress()) - } - // stdSigs contains the sequence number, account number, and signatures. // When simulating, this would just be a 0-length slice. stdSigs := tx.GetSignatures() @@ -344,6 +333,35 @@ func consumeMultisignatureVerificationGas(meter store.GasMeter, } } +// NewGasFeeCollector returns a new GasFeeCollector that will be used to +// deduct fees from the first signer. +func NewGasFeeCollector(ak AccountKeeper, bank BankKeeperI) sdk.GasFeeCollector { + return func( + ctx sdk.Context, + tx std.Tx, + gasUsed int64, + ) (res sdk.Result) { + signerAddrs := tx.GetSigners() + signerAccs := make([]std.Account, len(signerAddrs)) + signerAccs[0], res = GetSignerAcc(ctx, ak, signerAddrs[0]) + if !res.IsOK() { + return res + } + + if !tx.Fee.GasFee.IsZero() { + collectGasFee := std.NewCoin( + tx.Fee.GasFee.Denom, + tx.Fee.GasFee.Amount*gasUsed, + ) + res = DeductFees(bank, ctx, signerAccs[0], std.Coins{collectGasFee}) + if !res.IsOK() { + return res + } + } + return sdk.Result{GasWanted: tx.Fee.GasWanted} + } +} + // DeductFees deducts fees from the given account. // // NOTE: We could use the CoinKeeper (in addition to the AccountKeeper, because diff --git a/tm2/pkg/sdk/auth/ante_test.go b/tm2/pkg/sdk/auth/ante_test.go index be4167a6238..47dd74f8484 100644 --- a/tm2/pkg/sdk/auth/ante_test.go +++ b/tm2/pkg/sdk/auth/ante_test.go @@ -306,49 +306,6 @@ func TestAnteHandlerSequences(t *testing.T) { checkValidTx(t, anteHandler, ctx, tx, false) } -// Test logic around fee deduction. -func TestAnteHandlerFees(t *testing.T) { - t.Parallel() - - // setup - env := setupTestEnv() - ctx := env.ctx - anteHandler := NewAnteHandler(env.acck, env.bank, DefaultSigVerificationGasConsumer, defaultAnteOptions()) - - // keys and addresses - priv1, _, addr1 := tu.KeyTestPubAddr() - - // set the accounts - acc1 := env.acck.NewAccountWithAddress(ctx, addr1) - env.acck.SetAccount(ctx, acc1) - - // msg and signatures - var tx std.Tx - msg := tu.NewTestMsg(addr1) - privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} - fee := tu.NewTestFee() - msgs := []std.Msg{msg} - - // signer does not have enough funds to pay the fee - tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, false, std.InsufficientFundsError{}) - - acc1.SetCoins(std.NewCoins(std.NewCoin("atom", 149))) - env.acck.SetAccount(ctx, acc1) - checkInvalidTx(t, anteHandler, ctx, tx, false, std.InsufficientFundsError{}) - - collector := env.bank.(DummyBankKeeper).acck.GetAccount(ctx, FeeCollectorAddress()) - require.Nil(t, collector) - require.Equal(t, env.acck.GetAccount(ctx, addr1).GetCoins().AmountOf("atom"), int64(149)) - - acc1.SetCoins(std.NewCoins(std.NewCoin("atom", 150))) - env.acck.SetAccount(ctx, acc1) - checkValidTx(t, anteHandler, ctx, tx, false) - - require.Equal(t, env.bank.(DummyBankKeeper).acck.GetAccount(ctx, FeeCollectorAddress()).GetCoins().AmountOf("atom"), int64(150)) - require.Equal(t, env.acck.GetAccount(ctx, addr1).GetCoins().AmountOf("atom"), int64(0)) -} - // Test logic around memo gas consumption. func TestAnteHandlerMemoGas(t *testing.T) { t.Parallel() diff --git a/tm2/pkg/sdk/baseapp.go b/tm2/pkg/sdk/baseapp.go index c11f81d852a..4795aa62983 100644 --- a/tm2/pkg/sdk/baseapp.go +++ b/tm2/pkg/sdk/baseapp.go @@ -45,6 +45,8 @@ type BaseApp struct { beginTxHook BeginTxHook // BaseApp-specific hook run before running transaction messages. endTxHook EndTxHook // BaseApp-specific hook run after running transaction messages. + gasFeeCollector GasFeeCollector // gas fee collector + // -------------------- // Volatile state // checkState is set on initialization and reset on Commit. @@ -582,6 +584,14 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliv res.ResponseBase = result.ResponseBase res.GasWanted = result.GasWanted res.GasUsed = result.GasUsed + + // NOTE: After runTx, gas fee is deducted from the first signer. + // This is done in the BaseApp to ensure that the gas fee is deducted + // res.GasUsed is the finalized amount of gas used to run tx. + // Therefore, gas fee is deducted here. + if app.gasFeeCollector != nil { + app.gasFeeCollector(ctx, tx, res.GasUsed) + } return } } diff --git a/tm2/pkg/sdk/options.go b/tm2/pkg/sdk/options.go index b9840a7510b..4804b1a7674 100644 --- a/tm2/pkg/sdk/options.go +++ b/tm2/pkg/sdk/options.go @@ -99,3 +99,10 @@ func (app *BaseApp) SetEndTxHook(endTx EndTxHook) { } app.endTxHook = endTx } + +func (app *BaseApp) SetGasFeeCollector(gfc GasFeeCollector) { + if app.sealed { + panic("SetGasFeeCollector() on sealed BaseApp") + } + app.gasFeeCollector = gfc +} diff --git a/tm2/pkg/sdk/types.go b/tm2/pkg/sdk/types.go index 47395362f1a..a2ae7d8a86b 100644 --- a/tm2/pkg/sdk/types.go +++ b/tm2/pkg/sdk/types.go @@ -58,3 +58,6 @@ const ( // Deliver a transaction RunTxModeDeliver RunTxMode = iota ) + +// GasFeeCollector handles gas consumption for each transaction. +type GasFeeCollector func(ctx Context, tx Tx, gasUsed int64) (res Result) From 3d94e89a3efce12b368f24e13a491807767d742b Mon Sep 17 00:00:00 2001 From: 0xTopaz Date: Wed, 27 Nov 2024 00:15:14 +0900 Subject: [PATCH 2/8] test: fix txtar testing --- gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar b/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar index d1e366f03cf..19a4574aa2d 100644 --- a/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar +++ b/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar @@ -11,12 +11,9 @@ stdout 'data: "10000000000000ugnot"' # 2. execute the Faucet function gnokey maketx call -pkgpath gno.land/r/demo/foo20 -func Faucet -broadcast=true -chainid=tendermint_test -gas-fee 1ugnot -gas-wanted 100000000 -memo "" test1 - -## check output -stdout OK! -stdout 'GAS USED: 748508' +stdout 'GAS USED: 747265' # 3. check the balance after the Faucet function gnokey query bank/balances/g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5 stdout 'height: 0' -stdout 'data: "9999999251492ugnot"' +stdout 'data: "9999999252735ugnot"' From 7bb41170dcc026aeaabd9bca121b2e56e7437f66 Mon Sep 17 00:00:00 2001 From: 0xTopaz Date: Wed, 27 Nov 2024 16:06:49 +0900 Subject: [PATCH 3/8] fix: process error case - when gasFeeCollector return error, both Error and Log of result set to res - If gas-fee amount is zero, return error with ErrInsufficientFee --- tm2/pkg/sdk/auth/ante.go | 2 ++ tm2/pkg/sdk/baseapp.go | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tm2/pkg/sdk/auth/ante.go b/tm2/pkg/sdk/auth/ante.go index fd76c292f1e..bce0185c3a1 100644 --- a/tm2/pkg/sdk/auth/ante.go +++ b/tm2/pkg/sdk/auth/ante.go @@ -357,6 +357,8 @@ func NewGasFeeCollector(ak AccountKeeper, bank BankKeeperI) sdk.GasFeeCollector if !res.IsOK() { return res } + } else { + return abciResult(std.ErrInsufficientFee(fmt.Sprintf("invalid fee amount: %s", tx.Fee.GasFee))) } return sdk.Result{GasWanted: tx.Fee.GasWanted} } diff --git a/tm2/pkg/sdk/baseapp.go b/tm2/pkg/sdk/baseapp.go index 4795aa62983..311ec4cd32c 100644 --- a/tm2/pkg/sdk/baseapp.go +++ b/tm2/pkg/sdk/baseapp.go @@ -590,7 +590,11 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliv // res.GasUsed is the finalized amount of gas used to run tx. // Therefore, gas fee is deducted here. if app.gasFeeCollector != nil { - app.gasFeeCollector(ctx, tx, res.GasUsed) + feeCollectResult := app.gasFeeCollector(ctx, tx, res.GasUsed) + if !feeCollectResult.IsOK() { + res.ResponseBase.Error = feeCollectResult.ResponseBase.Error + res.ResponseBase.Log = feeCollectResult.ResponseBase.Log + } } return } From 1a36856b52ca079febf8915a4bdbcdced4204539 Mon Sep 17 00:00:00 2001 From: 0xTopaz Date: Wed, 27 Nov 2024 16:14:39 +0900 Subject: [PATCH 4/8] test: add test case code Testcase includes error case and success case - Error Case 1. If UnknownAddress, return error with UnknownAddressError msg 2. If Invalid gas fee, return error with InsufficientFeeError msg 3. If not enough funds, return error with InsufficientFundsError msg 4. If fee denom are invalid, panic - Success Case 1. If success, gasFeeCollector return error with nil and result IsOK, and FeeCollectorAddress get (used gas * fee.Amount) from account balance --- tm2/pkg/sdk/auth/ante_test.go | 89 +++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/tm2/pkg/sdk/auth/ante_test.go b/tm2/pkg/sdk/auth/ante_test.go index 47dd74f8484..755469d9f91 100644 --- a/tm2/pkg/sdk/auth/ante_test.go +++ b/tm2/pkg/sdk/auth/ante_test.go @@ -57,6 +57,25 @@ func defaultAnteOptions() AnteOptions { } } +// run the tx through the gasFeeCollector and ensure its valid +func checkGasFeeValidTx(t *testing.T, gasFeeCollector sdk.GasFeeCollector, ctx sdk.Context, tx std.Tx, gasUsed int64) { + t.Helper() + + result := gasFeeCollector(ctx, tx, gasUsed) + require.Nil(t, result.Error) + require.Equal(t, tx.Fee.GasWanted, result.GasWanted) + require.True(t, result.IsOK()) +} + +// run the tx through the anteHandler and ensure it fails with the given code +func checkGasFeeInvalidTx(t *testing.T, gasFeeCollector sdk.GasFeeCollector, ctx sdk.Context, tx std.Tx, gasUsed int64, err abci.Error) { + t.Helper() + + result := gasFeeCollector(ctx, tx, gasUsed) + require.True(t, result.IsErr()) + require.Equal(t, reflect.TypeOf(err), reflect.TypeOf(sdk.ABCIError(result.Error)), fmt.Sprintf("Expected %v, got %v", err, result)) +} + // Test various error cases in the AnteHandler control flow. func TestAnteHandlerSigErrors(t *testing.T) { t.Parallel() @@ -823,3 +842,73 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) } + +// TestNewGasFeeCollector +func TestNewGasFeeCollector(t *testing.T) { + t.Parallel() + + // setup + env := setupTestEnv() + ctx := env.ctx + gasFeeCollector := NewGasFeeCollector(env.acck, env.bank) + + // keys and addresses + priv1, _, addr1 := tu.KeyTestPubAddr() + + // msg and signatures + var tx std.Tx + msg := tu.NewTestMsg(addr1) + privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} + fee := tu.NewTestFee() + msgs := []std.Msg{msg} + + // test signer use new account (UnknownAddress) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) + checkGasFeeInvalidTx(t, gasFeeCollector, ctx, tx, int64(1000), std.UnknownAddressError{}) + + // set the accounts + acc1 := env.acck.NewAccountWithAddress(ctx, addr1) + env.acck.SetAccount(ctx, acc1) + + // test signer does not set gas-fee + zeroGasFee := std.NewFee(50000, std.NewCoin("atom", 0)) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, zeroGasFee) + checkGasFeeInvalidTx(t, gasFeeCollector, ctx, tx, int64(1000), std.InsufficientFeeError{}) + + // test signer does not have enough funds to pay the fee + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) + checkGasFeeInvalidTx(t, gasFeeCollector, ctx, tx, int64(1000), std.InsufficientFundsError{}) + + // test inValid fee + t.Run("invalid denom test", func(t *testing.T) { + invalidDenomGasFee := std.NewFee(50000, std.Coin{Denom: "atom@", Amount: 100}) + invalidDenomGasFee2 := std.NewFee(50000, std.Coin{Denom: "atom denom", Amount: 100}) + invalidDenomGasFee3 := std.NewFee(50000, std.Coin{Denom: "a", Amount: 100}) + + txInvalidDenom := tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, invalidDenomGasFee) + txInvalidDenom2 := tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, invalidDenomGasFee2) + txInvalidDenom3 := tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, invalidDenomGasFee3) + + require.Panics(t, func() { gasFeeCollector(ctx, txInvalidDenom, int64(1000)) }) + require.Panics(t, func() { gasFeeCollector(ctx, txInvalidDenom2, int64(1000)) }) + require.Panics(t, func() { gasFeeCollector(ctx, txInvalidDenom3, int64(1000)) }) + }) + + // test signer does not have enough funds to pay the fee + notEnoughFee := std.NewFee(50000, std.NewCoin("atom", 100)) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, notEnoughFee) + checkGasFeeInvalidTx(t, gasFeeCollector, ctx, tx, int64(1000), std.InsufficientFundsError{}) + + // test good tx from one signer + acc1.SetCoins(tu.NewTestCoins()) + env.acck.SetAccount(ctx, acc1) + require.Equal(t, env.acck.GetAccount(ctx, addr1).GetCoins().AmountOf("atom"), int64(10000000)) + + collector := env.bank.(DummyBankKeeper).acck.GetAccount(ctx, FeeCollectorAddress()) + require.Nil(t, collector) + tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) + checkGasFeeValidTx(t, gasFeeCollector, ctx, tx, int64(1000)) + + require.Equal(t, env.bank.(DummyBankKeeper).acck.GetAccount(ctx, FeeCollectorAddress()).GetCoins().AmountOf("atom"), int64(150000)) + require.Equal(t, env.acck.GetAccount(ctx, addr1).GetCoins().AmountOf("atom"), int64(9850000)) +} From d5635411e7ae09826146893e80361a53190bd99f Mon Sep 17 00:00:00 2001 From: 0xTopaz Date: Wed, 27 Nov 2024 17:32:58 +0900 Subject: [PATCH 5/8] fix: lint error and test code error in git action ci --- gno.land/cmd/gnoland/testdata/addpkg_namespace.txtar | 2 +- .../gnoland/testdata/realm_banker_issued_coin_denom.txtar | 6 +++--- gno.land/cmd/gnoland/testdata/restart_missing_type.txtar | 6 +++--- gno.land/pkg/gnoland/app_test.go | 2 +- gno.land/pkg/integration/testdata/adduser.txtar | 4 ++-- tm2/pkg/sdk/auth/ante_test.go | 2 ++ 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/gno.land/cmd/gnoland/testdata/addpkg_namespace.txtar b/gno.land/cmd/gnoland/testdata/addpkg_namespace.txtar index d207289e0ff..92fca0ac4a5 100644 --- a/gno.land/cmd/gnoland/testdata/addpkg_namespace.txtar +++ b/gno.land/cmd/gnoland/testdata/addpkg_namespace.txtar @@ -37,7 +37,7 @@ stdout 'OK!' # Check that `sys/users` has been enabled # gui call -> sys/users.IsEnable -gnokey maketx call -pkgpath gno.land/r/sys/users -func IsEnabled -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test gui +gnokey maketx call -pkgpath gno.land/r/sys/users -func IsEnabled -gas-fee 10ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test gui stdout 'OK!' stdout 'true' diff --git a/gno.land/cmd/gnoland/testdata/realm_banker_issued_coin_denom.txtar b/gno.land/cmd/gnoland/testdata/realm_banker_issued_coin_denom.txtar index 71ef6400471..5aa9b6d7cb8 100644 --- a/gno.land/cmd/gnoland/testdata/realm_banker_issued_coin_denom.txtar +++ b/gno.land/cmd/gnoland/testdata/realm_banker_issued_coin_denom.txtar @@ -13,7 +13,7 @@ gnokey maketx addpkg -pkgdir $WORK/short -pkgpath gno.land/r/test/realm_banker - gnokey maketx addpkg -pkgdir $WORK/long -pkgpath gno.land/r/test/package89_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789_1234567890 -gas-fee 1000000ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test test1 ## test2 spend all balance -gnokey maketx send -send "9999999ugnot" -to g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5 -gas-fee 1ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test test2 +gnokey maketx send -send "9970973ugnot" -to g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5 -gas-fee 1ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test test2 ## check test2 balance gnokey query bank/balances/${USER_ADDR_test2} @@ -33,8 +33,8 @@ gnokey maketx call -pkgpath gno.land/r/test/realm_banker -func Burn -args ${USER gnokey query bank/balances/${USER_ADDR_test2} stdout '"31330/gno.land/r/test/realm_banker:ugnot"' -## transfer 1ugnot to test2 for gas-fee of below tx -gnokey maketx send -send "1ugnot" -to ${USER_ADDR_test2} -gas-fee 1ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test test1 +## transfer 33387ugnot to test2 for gas-fee of below tx +gnokey maketx send -send "33387ugnot" -to ${USER_ADDR_test2} -gas-fee 1ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test test1 ## transfer coin gnokey maketx send -send "1330/gno.land/r/test/realm_banker:ugnot" -to g1yr0dpfgthph7y6mepdx8afuec4q3ga2lg8tjt0 -gas-fee 1ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test test2 diff --git a/gno.land/cmd/gnoland/testdata/restart_missing_type.txtar b/gno.land/cmd/gnoland/testdata/restart_missing_type.txtar index c492f1c6646..7c3a30dce20 100644 --- a/gno.land/cmd/gnoland/testdata/restart_missing_type.txtar +++ b/gno.land/cmd/gnoland/testdata/restart_missing_type.txtar @@ -88,7 +88,7 @@ gnoland restart ], "fee": { "gas_wanted": "1000000", - "gas_fee": "1000000ugnot" + "gas_fee": "10ugnot" }, "signatures": [], "memo": "" @@ -163,7 +163,7 @@ gnoland restart ], "fee": { "gas_wanted": "20000000", - "gas_fee": "1000000ugnot" + "gas_fee": "10ugnot" }, "signatures": [], "memo": "" @@ -194,7 +194,7 @@ gnoland restart ], "fee": { "gas_wanted": "16000000", - "gas_fee": "1000000ugnot" + "gas_fee": "10ugnot" }, "signatures": [], "memo": "" diff --git a/gno.land/pkg/gnoland/app_test.go b/gno.land/pkg/gnoland/app_test.go index 999e04b2c4b..215820320cb 100644 --- a/gno.land/pkg/gnoland/app_test.go +++ b/gno.land/pkg/gnoland/app_test.go @@ -261,7 +261,7 @@ func createAndSignTx( tx := std.Tx{ Msgs: msgs, Fee: std.Fee{ - GasFee: std.NewCoin("ugnot", 2000000), + GasFee: std.NewCoin("ugnot", 20), GasWanted: 10000000, }, } diff --git a/gno.land/pkg/integration/testdata/adduser.txtar b/gno.land/pkg/integration/testdata/adduser.txtar index 8c5706a68c4..c4818f5adba 100644 --- a/gno.land/pkg/integration/testdata/adduser.txtar +++ b/gno.land/pkg/integration/testdata/adduser.txtar @@ -4,10 +4,10 @@ adduser test8 gnoland start ## add bar.gno package located in $WORK directory as gno.land/r/foobar/bar -gnokey maketx addpkg -pkgdir $WORK/bar -pkgpath gno.land/r/foobar/bar -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test8 +gnokey maketx addpkg -pkgdir $WORK/bar -pkgpath gno.land/r/foobar/bar -gas-fee 10ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test8 ## execute Render -gnokey maketx run -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test8 $WORK/script/script.gno +gnokey maketx run -gas-fee 10ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test8 $WORK/script/script.gno ## compare render stdout 'main: --- hello from foo ---' diff --git a/tm2/pkg/sdk/auth/ante_test.go b/tm2/pkg/sdk/auth/ante_test.go index 755469d9f91..a1aa1c9e491 100644 --- a/tm2/pkg/sdk/auth/ante_test.go +++ b/tm2/pkg/sdk/auth/ante_test.go @@ -881,6 +881,8 @@ func TestNewGasFeeCollector(t *testing.T) { // test inValid fee t.Run("invalid denom test", func(t *testing.T) { + t.Parallel() + invalidDenomGasFee := std.NewFee(50000, std.Coin{Denom: "atom@", Amount: 100}) invalidDenomGasFee2 := std.NewFee(50000, std.Coin{Denom: "atom denom", Amount: 100}) invalidDenomGasFee3 := std.NewFee(50000, std.Coin{Denom: "a", Amount: 100}) From 15936e3185d22b0d721b82d4400ac40906af3598 Mon Sep 17 00:00:00 2001 From: 0xTopaz Date: Wed, 27 Nov 2024 18:08:29 +0900 Subject: [PATCH 6/8] fix: test code error in git action ci --- .../gnoland/testdata/addpkg_namespace.txtar | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/gno.land/cmd/gnoland/testdata/addpkg_namespace.txtar b/gno.land/cmd/gnoland/testdata/addpkg_namespace.txtar index 92fca0ac4a5..c4639be79e3 100644 --- a/gno.land/cmd/gnoland/testdata/addpkg_namespace.txtar +++ b/gno.land/cmd/gnoland/testdata/addpkg_namespace.txtar @@ -14,70 +14,70 @@ gnoland start # Check if sys/users is disabled # gui call -> sys/users.IsEnable -gnokey maketx call -pkgpath gno.land/r/sys/users -func IsEnabled -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test gui +gnokey maketx call -pkgpath gno.land/r/sys/users -func IsEnabled -gas-fee 5ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test gui stdout 'OK!' stdout 'false' # Gui should be able to addpkg on test1 addr # gui addpkg -> gno.land/r//mysuperpkg -gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/$USER_ADDR_test1/mysuperpkg -gas-fee 1000000ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui +gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/$USER_ADDR_test1/mysuperpkg -gas-fee 10ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui stdout 'OK!' # Gui should be able to addpkg on random name # gui addpkg -> gno.land/r/randomname/mysuperpkg -gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/randomname/mysuperpkg -gas-fee 1000000ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui +gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/randomname/mysuperpkg -gas-fee 5ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui stdout 'OK!' ## When `sys/users` is enabled # Enable `sys/users` # admin call -> sys/users.AdminEnable -gnokey maketx call -pkgpath gno.land/r/sys/users -func AdminEnable -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test admin +gnokey maketx call -pkgpath gno.land/r/sys/users -func AdminEnable -gas-fee 10ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test admin stdout 'OK!' # Check that `sys/users` has been enabled # gui call -> sys/users.IsEnable -gnokey maketx call -pkgpath gno.land/r/sys/users -func IsEnabled -gas-fee 10ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test gui +gnokey maketx call -pkgpath gno.land/r/sys/users -func IsEnabled -gas-fee 5ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test gui stdout 'OK!' stdout 'true' # Try to add a pkg an with unregistered user # gui addpkg -> gno.land/r//one -! gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/$USER_ADDR_test1/one -gas-fee 1000000ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui +! gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/$USER_ADDR_test1/one -gas-fee 5ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui stderr 'unauthorized user' # Try to add a pkg with an unregistered user, on their own address as namespace # gui addpkg -> gno.land/r//one -gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/$USER_ADDR_gui/one -gas-fee 1000000ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui +gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/$USER_ADDR_gui/one -gas-fee 5ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui stdout 'OK!' ## Test unregistered namespace # Call addpkg with admin user on gui namespace # admin addpkg -> gno.land/r/guiland/one -! gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/guiland/one -gas-fee 1000000ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test admin +! gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/guiland/one -gas-fee 10ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test admin stderr 'unauthorized user' ## Test registered namespace # Test admin invites gui # admin call -> demo/users.Invite -gnokey maketx call -pkgpath gno.land/r/demo/users -func Invite -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -args $USER_ADDR_gui admin +gnokey maketx call -pkgpath gno.land/r/demo/users -func Invite -gas-fee 10ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -args $USER_ADDR_gui admin stdout 'OK!' # test gui register namespace # gui call -> demo/users.Register -gnokey maketx call -pkgpath gno.land/r/demo/users -func Register -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -args $USER_ADDR_admin -args 'guiland' -args 'im gui' gui +gnokey maketx call -pkgpath gno.land/r/demo/users -func Register -gas-fee 2ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -args $USER_ADDR_admin -args 'guiland' -args 'im gui' gui stdout 'OK!' # Test gui publishing on guiland/one # gui addpkg -> gno.land/r/guiland/one -gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/guiland/one -gas-fee 1000000ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui +gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/guiland/one -gas-fee 5ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test gui stdout 'OK!' # Test admin publishing on guiland/two # admin addpkg -> gno.land/r/guiland/two -! gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/guiland/two -gas-fee 1000000ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test admin +! gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/guiland/two -gas-fee 10ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test admin stderr 'unauthorized user' -- one.gno -- From 9b34a6083161bc30fcc6b6bed754d19ef646533b Mon Sep 17 00:00:00 2001 From: 0xTopaz Date: Wed, 27 Nov 2024 20:19:14 +0900 Subject: [PATCH 7/8] test: add test case --- tm2/pkg/sdk/baseapp_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tm2/pkg/sdk/baseapp_test.go b/tm2/pkg/sdk/baseapp_test.go index 08e8191170a..d4c59c0258d 100644 --- a/tm2/pkg/sdk/baseapp_test.go +++ b/tm2/pkg/sdk/baseapp_test.go @@ -319,6 +319,9 @@ func TestBaseAppOptionSeal(t *testing.T) { require.Panics(t, func() { app.SetEndTxHook(nil) }) + require.Panics(t, func() { + app.SetGasFeeCollector(nil) + }) } func TestSetMinGasPrices(t *testing.T) { @@ -329,6 +332,11 @@ func TestSetMinGasPrices(t *testing.T) { db := memdb.NewMemDB() app := newBaseApp(t.Name(), db, SetMinGasPrices("5000stake/10gas")) require.Equal(t, minGasPrices, app.minGasPrices) + + // invalid input + _, err = ParseGasPrices("5000stake") + require.NotNil(t, err) + require.Panics(t, func() { SetMinGasPrices("5000stake") }) } func TestInitChainer(t *testing.T) { From 0dbc31329d344d4c4ceedea21d1564bb30d04536 Mon Sep 17 00:00:00 2001 From: 0xTopaz Date: Thu, 28 Nov 2024 12:29:14 +0900 Subject: [PATCH 8/8] fix : change deduct gas fee timing before state update - as issue that is state already updated even deduct gas fee is failed - change deduct gas fee timing to before state update --- .../cmd/gnoland/testdata/deduct_gasfee.txtar | 45 ++++++++++++++++++- tm2/pkg/sdk/baseapp.go | 29 +++++++----- 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar b/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar index 19a4574aa2d..6e1e400bcf6 100644 --- a/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar +++ b/gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar @@ -1,6 +1,9 @@ # load the package loadpkg gno.land/r/demo/foo20 +# another test user +adduser test2 + # start a new node gnoland start @@ -11,9 +14,49 @@ stdout 'data: "10000000000000ugnot"' # 2. execute the Faucet function gnokey maketx call -pkgpath gno.land/r/demo/foo20 -func Faucet -broadcast=true -chainid=tendermint_test -gas-fee 1ugnot -gas-wanted 100000000 -memo "" test1 -stdout 'GAS USED: 747265' +stdout 'GAS USED: 767128' # 3. check the balance after the Faucet function gnokey query bank/balances/g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5 stdout 'height: 0' stdout 'data: "9999999252735ugnot"' + +# 4. check the balance of test2 +gnokey query bank/balances/${USER_ADDR_test2} +stdout 'height: 0' +stdout 'data: "10000000ugnot"' + +# 5. add realm_banker +gnokey maketx addpkg -pkgdir $WORK/short -pkgpath gno.land/r/test/realm_banker -gas-fee 10ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test test1 + +# 6. mint coin from banker +gnokey maketx call -pkgpath gno.land/r/test/realm_banker -func Mint -args ${USER_ADDR_test2} -args "ugnot" -args "10000" -gas-fee 1000000ugnot -gas-wanted 10000000 -broadcast -chainid=tendermint_test test1 + +# 7. check balance after minting, without patching banker will return '10000ugnot' +gnokey query bank/balances/${USER_ADDR_test2} +stdout '"10000/gno.land/r/test/realm_banker:ugnot,10000000ugnot"' + +# 8. (fail case by insufficient funds)send 5000000ugnot to test1 from test2, by using 100foo20 as gas-fee +! gnokey maketx send -send "5000000ugnot" -to ${USER_ADDR_test1} -gas-fee 10foo20 -gas-wanted 10000000 -broadcast -chainid=tendermint_test test2 + +# 9. Now, as #8 is failed, test2 balance should be the same as before("10000000ugnot") +gnokey query bank/balances/${USER_ADDR_test2} +stdout 'data: "10000/gno.land/r/test/realm_banker:ugnot,10000000ugnot"' + + +-- short/realm_banker.gno -- +package realm_banker + +import ( + "std" +) + +func Mint(addr std.Address, denom string, amount int64) { + banker := std.GetBanker(std.BankerTypeRealmIssue) + banker.IssueCoin(addr, denom, amount) +} + +func Burn(addr std.Address, denom string, amount int64) { + banker := std.GetBanker(std.BankerTypeRealmIssue) + banker.RemoveCoin(addr, denom, amount) +} diff --git a/tm2/pkg/sdk/baseapp.go b/tm2/pkg/sdk/baseapp.go index 311ec4cd32c..c0f3ae2b671 100644 --- a/tm2/pkg/sdk/baseapp.go +++ b/tm2/pkg/sdk/baseapp.go @@ -585,17 +585,6 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliv res.GasWanted = result.GasWanted res.GasUsed = result.GasUsed - // NOTE: After runTx, gas fee is deducted from the first signer. - // This is done in the BaseApp to ensure that the gas fee is deducted - // res.GasUsed is the finalized amount of gas used to run tx. - // Therefore, gas fee is deducted here. - if app.gasFeeCollector != nil { - feeCollectResult := app.gasFeeCollector(ctx, tx, res.GasUsed) - if !feeCollectResult.IsOK() { - res.ResponseBase.Error = feeCollectResult.ResponseBase.Error - res.ResponseBase.Log = feeCollectResult.ResponseBase.Log - } - } return } } @@ -863,6 +852,24 @@ func (app *BaseApp) runTx(ctx Context, tx Tx) (result Result) { app.endTxHook(runMsgCtx, result) } + // NOTE: Before update state, gas fee is deducted from the first signer. + // This is done in the BaseApp to ensure that the gas fee is deducted + // If the process of collecting a gas fee results in an error with insufficient funds, + // then the status should not be updated. + // result.GasUsed is the finalized amount of gas used to run Messages. + // This is because gasMeter does not accumulate gas used after this point, + // and gasUsed in the result accumulates the amount of gas used so far. + // Therefore, gas fee is deducted here. + isGenesis := ctx.BlockHeight() == 0 + if !isGenesis && app.gasFeeCollector != nil { + feeCollectResult := app.gasFeeCollector(runMsgCtx, tx, result.GasUsed) + if !feeCollectResult.IsOK() { + result.ResponseBase.Error = feeCollectResult.ResponseBase.Error + result.ResponseBase.Log = feeCollectResult.ResponseBase.Log + return result + } + } + // only update state if all messages pass if result.IsOK() { msCache.MultiWrite()