Skip to content

Commit

Permalink
Problem: precompiles required-gas need adjustments (#1178)
Browse files Browse the repository at this point in the history 
* adjust precompile required gas

update integration tests config

add switch

fix lint

rename const

revert gas cost

estimate cost

convert to map

fix alert

fix golangci

update gas value based on estimation

add basecost to the precompile

increase gas in integration test

* adjust config
  • Loading branch information
@thomas-nguy
thomas-nguy authored Sep 30, 2023
1 parent 071bc21 commit 79c911c
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 40 deletions.
8 changes: 5 additions & 3 deletions app/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,16 +544,18 @@ func New(
for k, v := range memKeys {
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,
app.FeeMarketKeeper,
tracer,
evmS,
[]vm.PrecompiledContract{
cronosprecompiles.NewBankContract(app.BankKeeper, appCodec),
cronosprecompiles.NewRelayerContract(app.IBCKeeper, appCodec),
cronosprecompiles.NewIcaContract(&app.ICAAuthKeeper, &app.CronosKeeper, appCodec),
cronosprecompiles.NewBankContract(app.BankKeeper, appCodec, gasConfig),
cronosprecompiles.NewRelayerContract(app.IBCKeeper, appCodec, gasConfig),
cronosprecompiles.NewIcaContract(&app.ICAAuthKeeper, &app.CronosKeeper, appCodec, gasConfig),
},
allKeys,
)
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/configs/ibc.jsonnet
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ config {
{
id: 'cronos_777-1',
max_gas: 500000,
gas_multiplier: 2,
gas_multiplier: 4,
address_type: {
derivation: 'ethermint',
proto_type: {
Expand Down
6 changes: 3 additions & 3 deletions integration_tests/test_ica_precompile.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def test_call(ibc):
name = "signer2"
addr = ADDRS[name]
contract = w3.eth.contract(address=CONTRACT, abi=contract_info)
data = {"from": ADDRS[name], "gas": 200000}
data = {"from": ADDRS[name], "gas": 400000}
ica_address = register_acc(
cli_controller,
w3,
Expand Down Expand Up @@ -173,7 +173,7 @@ def test_sc_call(ibc):
name = "signer2"
signer = ADDRS[name]
keys = KEYS[name]
data = {"from": signer, "gas": 200000}
data = {"from": signer, "gas": 400000}
ica_address = register_acc(
cli_controller,
w3,
Expand All @@ -189,7 +189,7 @@ def test_sc_call(ibc):

# register from another user should fail
name = "signer1"
data = {"from": ADDRS[name], "gas": 200000}
data = {"from": ADDRS[name], "gas": 400000}
version = ""
tx = tcontract.functions.callRegister(connid, version).build_transaction(data)
res = send_transaction(w3, tx, KEYS[name])
Expand Down
56 changes: 42 additions & 14 deletions x/cronos/keeper/precompiles/bank.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"errors"
"math/big"

storetypes "github.com/cosmos/cosmos-sdk/store/types"

sdkmath "cosmossdk.io/math"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/ethereum/go-ethereum/accounts/abi"
Expand All @@ -18,43 +20,69 @@ import (
)

const (
EVMDenomPrefix = "evm/"
BankContractRequiredGas = 10000
EVMDenomPrefix = "evm/"
MintMethodName = "mint"
BurnMethodName = "burn"
BalanceOfMethodName = "balanceOf"
TransferMethodName = "transfer"
)

var (
bankABI abi.ABI
BankContractAddress = common.BytesToAddress([]byte{100})
bankABI abi.ABI
bankContractAddress = common.BytesToAddress([]byte{100})
bankGasRequiredByMethod = map[[4]byte]uint64{}
)

func init() {
if err := bankABI.UnmarshalJSON([]byte(bank.BankModuleMetaData.ABI)); err != nil {
panic(err)
}
for methodName := range bankABI.Methods {
var methodID [4]byte
copy(methodID[:], bankABI.Methods[methodName].ID[:4])
switch methodName {
case MintMethodName, BurnMethodName:
bankGasRequiredByMethod[methodID] = 200000
case BalanceOfMethodName:
bankGasRequiredByMethod[methodID] = 10000
case TransferMethodName:
bankGasRequiredByMethod[methodID] = 150000
default:
bankGasRequiredByMethod[methodID] = 0
}
}
}

func EVMDenom(token common.Address) string {
return EVMDenomPrefix + token.Hex()
}

type BankContract struct {
bankKeeper types.BankKeeper
cdc codec.Codec
bankKeeper types.BankKeeper
cdc codec.Codec
kvGasConfig storetypes.GasConfig
}

// NewBankContract creates the precompiled contract to manage native tokens
func NewBankContract(bankKeeper types.BankKeeper, cdc codec.Codec) vm.PrecompiledContract {
return &BankContract{bankKeeper, cdc}
func NewBankContract(bankKeeper types.BankKeeper, cdc codec.Codec, kvGasConfig storetypes.GasConfig) vm.PrecompiledContract {
return &BankContract{bankKeeper, cdc, kvGasConfig}
}

func (bc *BankContract) Address() common.Address {
return BankContractAddress
return bankContractAddress
}

// RequiredGas calculates the contract gas use
func (bc *BankContract) RequiredGas(input []byte) uint64 {
// TODO estimate required gas
return BankContractRequiredGas
// base cost to prevent large input size
baseCost := uint64(len(input)) * bc.kvGasConfig.WriteCostPerByte
var methodID [4]byte
copy(methodID[:], input[:4])
requiredGas, ok := bankGasRequiredByMethod[methodID]
if ok {
return requiredGas + baseCost
}
return baseCost
}

func (bc *BankContract) IsStateful() bool {
Expand Down Expand Up @@ -82,7 +110,7 @@ func (bc *BankContract) Run(evm *vm.EVM, contract *vm.Contract, readonly bool) (
stateDB := evm.StateDB.(ExtStateDB)
precompileAddr := bc.Address()
switch method.Name {
case "mint", "burn":
case MintMethodName, BurnMethodName:
if readonly {
return nil, errors.New("the method is not readonly")
}
Expand Down Expand Up @@ -126,7 +154,7 @@ func (bc *BankContract) Run(evm *vm.EVM, contract *vm.Contract, readonly bool) (
return nil, err
}
return method.Outputs.Pack(true)
case "balanceOf":
case BalanceOfMethodName:
args, err := method.Inputs.Unpack(contract.Input[4:])
if err != nil {
return nil, errors.New("fail to unpack input arguments")
Expand All @@ -136,7 +164,7 @@ func (bc *BankContract) Run(evm *vm.EVM, contract *vm.Contract, readonly bool) (
// query from storage
balance := bc.bankKeeper.GetBalance(stateDB.CacheContext(), sdk.AccAddress(addr.Bytes()), EVMDenom(token)).Amount.BigInt()
return method.Outputs.Pack(balance)
case "transfer":
case TransferMethodName:
if readonly {
return nil, errors.New("the method is not readonly")
}
Expand Down
51 changes: 41 additions & 10 deletions x/cronos/keeper/precompiles/ica.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"math/big"
"time"

storetypes "github.com/cosmos/cosmos-sdk/store/types"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
Expand All @@ -21,13 +23,17 @@ import (
"github.com/ethereum/go-ethereum/core/vm"
)

// TODO: Replace this const with adjusted gas cost corresponding to input when executing precompile contract.
const ICAContractRequiredGas = 10000
const (
RegisterAccountMethodName = "registerAccount"
QueryAccountMethodName = "queryAccount"
SubmitMsgsMethodName = "submitMsgs"
)

var (
icaABI abi.ABI
icaCallbackABI abi.ABI
icaContractAddress = common.BytesToAddress([]byte{102})
icaABI abi.ABI
icaCallbackABI abi.ABI
icaContractAddress = common.BytesToAddress([]byte{102})
icaGasRequiredByMethod = map[[4]byte]uint64{}
)

func init() {
Expand All @@ -37,6 +43,21 @@ func init() {
if err := icaCallbackABI.UnmarshalJSON([]byte(icacallback.ICACallbackMetaData.ABI)); err != nil {
panic(err)
}

for methodName := range icaABI.Methods {
var methodID [4]byte
copy(methodID[:], icaABI.Methods[methodName].ID[:4])
switch methodName {
case RegisterAccountMethodName:
icaGasRequiredByMethod[methodID] = 300000
case QueryAccountMethodName:
icaGasRequiredByMethod[methodID] = 100000
case SubmitMsgsMethodName:
icaGasRequiredByMethod[methodID] = 300000
default:
icaGasRequiredByMethod[methodID] = 0
}
}
}

func OnPacketResultCallback(args ...interface{}) ([]byte, error) {
Expand All @@ -49,14 +70,16 @@ type IcaContract struct {
cdc codec.Codec
icaauthKeeper types.Icaauthkeeper
cronosKeeper types.CronosKeeper
kvGasConfig storetypes.GasConfig
}

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

Expand All @@ -66,7 +89,15 @@ func (ic *IcaContract) Address() common.Address {

// RequiredGas calculates the contract gas use
func (ic *IcaContract) RequiredGas(input []byte) uint64 {
return ICAContractRequiredGas
// base cost to prevent large input size
baseCost := uint64(len(input)) * ic.kvGasConfig.WriteCostPerByte
var methodID [4]byte
copy(methodID[:], input[:4])
requiredGas, ok := icaGasRequiredByMethod[methodID]
if ok {
return requiredGas + baseCost
}
return baseCost
}

func (ic *IcaContract) IsStateful() bool {
Expand All @@ -87,7 +118,7 @@ func (ic *IcaContract) Run(evm *vm.EVM, contract *vm.Contract, readonly bool) ([
converter := cronosevents.IcaConvertEvent
var execErr error
switch method.Name {
case "registerAccount":
case RegisterAccountMethodName:
if readonly {
return nil, errors.New("the method is not readonly")
}
Expand All @@ -109,7 +140,7 @@ func (ic *IcaContract) Run(evm *vm.EVM, contract *vm.Contract, readonly bool) ([
return nil, execErr
}
return method.Outputs.Pack(true)
case "queryAccount":
case QueryAccountMethodName:
args, err := method.Inputs.Unpack(contract.Input[4:])
if err != nil {
return nil, errors.New("fail to unpack input arguments")
Expand All @@ -131,7 +162,7 @@ func (ic *IcaContract) Run(evm *vm.EVM, contract *vm.Contract, readonly bool) ([
icaAddress = response.InterchainAccountAddress
}
return method.Outputs.Pack(icaAddress)
case "submitMsgs":
case SubmitMsgsMethodName:
if readonly {
return nil, errors.New("the method is not readonly")
}
Expand Down
51 changes: 42 additions & 9 deletions x/cronos/keeper/precompiles/relayer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"encoding/binary"
"errors"

storetypes "github.com/cosmos/cosmos-sdk/store/types"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/vm"
Expand All @@ -12,33 +14,64 @@ import (
cronosevents "github.com/crypto-org-chain/cronos/v2/x/cronos/events"
)

// TODO: Replace this const with adjusted gas cost corresponding to input when executing precompile contract.
const RelayerContractRequiredGas = 10000
var (
relayerContractAddress = common.BytesToAddress([]byte{101})
relayerGasRequiredByMethod = map[int]uint64{}
)

var RelayerContractAddress = common.BytesToAddress([]byte{101})
func init() {
relayerGasRequiredByMethod[prefixCreateClient] = 200000
relayerGasRequiredByMethod[prefixUpdateClient] = 400000
relayerGasRequiredByMethod[prefixUpgradeClient] = 400000
relayerGasRequiredByMethod[prefixSubmitMisbehaviour] = 100000
relayerGasRequiredByMethod[prefixConnectionOpenInit] = 100000
relayerGasRequiredByMethod[prefixConnectionOpenTry] = 100000
relayerGasRequiredByMethod[prefixConnectionOpenAck] = 100000
relayerGasRequiredByMethod[prefixConnectionOpenConfirm] = 100000
relayerGasRequiredByMethod[prefixChannelOpenInit] = 100000
relayerGasRequiredByMethod[prefixChannelOpenTry] = 100000
relayerGasRequiredByMethod[prefixChannelOpenAck] = 100000
relayerGasRequiredByMethod[prefixChannelOpenConfirm] = 100000
relayerGasRequiredByMethod[prefixRecvPacket] = 250000
relayerGasRequiredByMethod[prefixAcknowledgement] = 250000
relayerGasRequiredByMethod[prefixTimeout] = 100000
relayerGasRequiredByMethod[prefixTimeoutOnClose] = 100000
}

type RelayerContract struct {
BaseContract

cdc codec.Codec
ibcKeeper *ibckeeper.Keeper
cdc codec.Codec
ibcKeeper *ibckeeper.Keeper
kvGasConfig storetypes.GasConfig
}

func NewRelayerContract(ibcKeeper *ibckeeper.Keeper, cdc codec.Codec) vm.PrecompiledContract {
func NewRelayerContract(ibcKeeper *ibckeeper.Keeper, cdc codec.Codec, kvGasConfig storetypes.GasConfig) vm.PrecompiledContract {
return &RelayerContract{
BaseContract: NewBaseContract(RelayerContractAddress),
BaseContract: NewBaseContract(relayerContractAddress),
ibcKeeper: ibcKeeper,
cdc: cdc,
kvGasConfig: kvGasConfig,
}
}

func (bc *RelayerContract) Address() common.Address {
return RelayerContractAddress
return relayerContractAddress
}

// RequiredGas calculates the contract gas use
func (bc *RelayerContract) RequiredGas(input []byte) uint64 {
return RelayerContractRequiredGas
// base cost to prevent large input size
baseCost := uint64(len(input)) * bc.kvGasConfig.WriteCostPerByte
if len(input) < prefixSize4Bytes {
return 0
}
prefix := int(binary.LittleEndian.Uint32(input[:prefixSize4Bytes]))
requiredGas, ok := relayerGasRequiredByMethod[prefix]
if ok {
return requiredGas + baseCost
}
return baseCost
}

func (bc *RelayerContract) IsStateful() bool {
Expand Down

0 comments on commit 79c911c

Please sign in to comment.