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

Problem: require gas in ica precompile is higher than consumed #1237

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
### State Machine Breaking

- [#1232](https://github.com/crypto-org-chain/cronos/pull/1232) Adjust require gas in relayer precompile to be closed with actual consumed.
- [#1237](https://github.com/crypto-org-chain/cronos/pull/1237) Adjust require gas in ica precompile to be closed with actual consumed.


*October 17, 2023*
Expand Down
3 changes: 1 addition & 2 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,6 @@
allKeys[k] = v
}

gasConfig := storetypes.TransientGasConfig()
app.EvmKeeper = evmkeeper.NewKeeper(
appCodec, keys[evmtypes.StoreKey], tkeys[evmtypes.TransientKey], authtypes.NewModuleAddress(govtypes.ModuleName),
app.AccountKeeper, app.BankKeeper, app.StakingKeeper,
Expand All @@ -537,7 +536,7 @@
evmS,
[]vm.PrecompiledContract{
cronosprecompiles.NewRelayerContract(app.IBCKeeper, appCodec, app.Logger()),
cronosprecompiles.NewIcaContract(&app.ICAAuthKeeper, &app.CronosKeeper, appCodec, gasConfig),
cronosprecompiles.NewIcaContract(&app.ICAAuthKeeper, &app.CronosKeeper, appCodec, app.Logger()),

Check failure on line 539 in app/app.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

not enough arguments in call to cronosprecompiles.NewIcaContract

Check failure on line 539 in app/app.go

View workflow job for this annotation

GitHub Actions / unittest

not enough arguments in call to cronosprecompiles.NewIcaContract

Check failure on line 539 in app/app.go

View workflow job for this annotation

GitHub Actions / unittest

not enough arguments in call to cronosprecompiles.NewIcaContract

Check failure on line 539 in app/app.go

View workflow job for this annotation

GitHub Actions / unittest

not enough arguments in call to cronosprecompiles.NewIcaContract

Check failure on line 539 in app/app.go

View workflow job for this annotation

GitHub Actions / unittest

not enough arguments in call to cronosprecompiles.NewIcaContract
},
allKeys,
)
Expand Down
46 changes: 36 additions & 10 deletions x/cronos/keeper/precompiles/ica.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"math/big"
"time"

"github.com/cometbft/cometbft/libs/log"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -20,6 +22,7 @@ import (
icaauthtypes "github.com/crypto-org-chain/cronos/v2/x/icaauth/types"
"github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/vm"
)

Expand All @@ -33,6 +36,7 @@ var (
icaABI abi.ABI
icaCallbackABI abi.ABI
icaContractAddress = common.BytesToAddress([]byte{102})
icaMethodNamedByMethod = map[[4]byte]string{}
icaGasRequiredByMethod = map[[4]byte]uint64{}
)

Expand All @@ -49,14 +53,15 @@ func init() {
copy(methodID[:], icaABI.Methods[methodName].ID[:4])
switch methodName {
case RegisterAccountMethodName:
icaGasRequiredByMethod[methodID] = 300000
icaGasRequiredByMethod[methodID] = 231455
case QueryAccountMethodName:
icaGasRequiredByMethod[methodID] = 100000
icaGasRequiredByMethod[methodID] = 1000 + 1000 // HasCost + ReadFlat
case SubmitMsgsMethodName:
icaGasRequiredByMethod[methodID] = 300000
icaGasRequiredByMethod[methodID] = 83086
default:
icaGasRequiredByMethod[methodID] = 0
}
icaMethodNamedByMethod[methodID] = methodName
}
}

Expand All @@ -71,15 +76,17 @@ type IcaContract struct {
icaauthKeeper types.Icaauthkeeper
cronosKeeper types.CronosKeeper
kvGasConfig storetypes.GasConfig
logger log.Logger
}

func NewIcaContract(icaauthKeeper types.Icaauthkeeper, cronosKeeper types.CronosKeeper, cdc codec.Codec, kvGasConfig storetypes.GasConfig) vm.PrecompiledContract {
func NewIcaContract(icaauthKeeper types.Icaauthkeeper, cronosKeeper types.CronosKeeper, cdc codec.Codec, kvGasConfig storetypes.GasConfig, logger log.Logger) vm.PrecompiledContract {
return &IcaContract{
BaseContract: NewBaseContract(icaContractAddress),
cdc: cdc,
icaauthKeeper: icaauthKeeper,
cronosKeeper: cronosKeeper,
kvGasConfig: kvGasConfig,
logger: logger.With("precompiles", "ica"),
}
}

Expand All @@ -88,16 +95,35 @@ func (ic *IcaContract) Address() common.Address {
}

// RequiredGas calculates the contract gas use
func (ic *IcaContract) RequiredGas(input []byte) uint64 {
// base cost to prevent large input size
baseCost := uint64(len(input)) * ic.kvGasConfig.WriteCostPerByte
// write `max(0, len(input) * DefaultTxSizeCostPerByte + requiredGasTable[methodPrefix] - intrinsicGas)`
// query `len(input) * ReadCostPerByte + HasCost + ReadFlat`
func (ic *IcaContract) RequiredGas(input []byte) (gas uint64) {
var methodID [4]byte
copy(methodID[:], input[:4])
requiredGas, ok := icaGasRequiredByMethod[methodID]
if ok {
return requiredGas + baseCost
if !ok {
requiredGas = 0
}
// base cost to prevent large input size
baseCost := uint64(0)
method := icaMethodNamedByMethod[methodID]
intrinsicGas, err := core.IntrinsicGas(input, nil, false, true, true)
if err != nil {
return 0
}
if method == QueryAccountMethodName {
baseCost = uint64(len(input)) * ic.kvGasConfig.ReadCostPerByte
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yihuang we only can calculate Has cost based on input, not sure how to get the Get cost since it's based on the result.

} else {
baseCost = uint64(len(input)) * authtypes.DefaultTxSizeCostPerByte
}
defer func() {
ic.logger.Debug("required", "gas", gas, "method", method, "len", len(input), "intrinsic", intrinsicGas)
}()
total := requiredGas + baseCost
if total < intrinsicGas {
return 0
}
return baseCost
return total - intrinsicGas
}

func (ic *IcaContract) IsStateful() bool {
Expand Down
Loading