Skip to content

Commit

Permalink
fix : change deduct gas fee timing before state update
Browse files Browse the repository at this point in the history
- as issue that is state already updated even deduct gas fee is failed
- change deduct gas fee timing to before state update
  • Loading branch information
onlyhyde committed Nov 28, 2024
1 parent 9b34a60 commit 0dbc313
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 12 deletions.
45 changes: 44 additions & 1 deletion gno.land/cmd/gnoland/testdata/deduct_gasfee.txtar
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# load the package
loadpkg gno.land/r/demo/foo20

# another test user
adduser test2

# start a new node
gnoland start

Expand All @@ -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)
}
29 changes: 18 additions & 11 deletions tm2/pkg/sdk/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}

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

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/sdk/baseapp.go#L865-L870

Added lines #L865 - L870 were not covered by tests
}

// only update state if all messages pass
if result.IsOK() {
msCache.MultiWrite()
Expand Down

0 comments on commit 0dbc313

Please sign in to comment.