Unable to estimate gas with ethers and metamask if amount being sent in value exceeds 20 TFUEL

[rjx18]

I've been trying to debug this strange error. I'm trying to call a contract with ethers and metamask, and it seems like the estimateGas function fails every time if the amount being sent in value exceeds 20 TFUEL. Here is a sample code to help replicate the problem:

    let estimate = contract.estimateGas.mintTicket;
    let method = contract.mintTicket;
    const tokensToMint = BigNumber.from(numTokens)
    let args = [
      tokensToMint 
    ];
    let value = tokenPrice.mul(tokensToMint)
    estimate(...args, { value }).then((estimatedGasLimit: any) => {
      return method(...args, {
        value,
        gasLimit: calculateGasMargin(estimatedGasLimit, GAS_MARGIN)
      })
    }).then((tx: any) => {
      addTransaction(tx);
    })

Let's say tokenPrice is 5 TFUEL, then the above function works with tokensToMint being 3 or less. If tokenPrice is 20 TFUEL, the above function fails even with tokensToMint being 1. Which puzzles me because technically, less tokens to mint should execute less code, resulting in less gas being used.

The RPC adaptor returns the following error message: "WARN[0049] eth_estimateGas: EVM execution failed: evm: execution reverted prefix=ethrpc"

I think it might be a problem with the RPC adaptor estimate gas function, because calling the method with a manually set gas limit does not revert. It seems like there is a hardcoded gas limit here ./rpc/ethrpc/eth_get_block_by_hash.go: result.GasLimit = 20000000, though I did not check if this is what is causing it. This is also on a local Theta privatenet. Perhaps the value of the transaction is being mixed up with gas used in the transaction?

[jieyilong]

Thanks for reporting the issue! Basically eth_estimateGas executes the "mintTicket" smart contract call in a sandbox environment to estimate the gas cost. This error messages "WARN[0049] eth_estimateGas: EVM execution failed: evm: execution reverted prefix=ethrpc" indicates the contract execution failed for some reason. Could you double check your smart contract code to see any error could get triggered if the tokenPrice is higher than 20 TFuel? Thanks

[rjx18]

Thanks for the reply! I've checked again and found out which requirement triggered the revert in the estimateGas function. It seems like this is the offending line in the mintTicket function:

uint256 ticketSaleValue = SafeMath.mul(metadata.ticketPrice, numberOfTickets); require(msg.value == ticketSaleValue || (metadata.acceptDonations && msg.value > ticketSaleValue), TICKET_SALE_ERROR);

However, even though the estimateGas function reverted in the sandbox environment, manually setting the gas fee and calling the smart contract directly with the same parameter does not revert in the actual transaction, and the tickets are successfully minted. Which means the error only occurs in the estimateGas sandbox for some reason.

I have also replaced the line with the following: require(msg.value < ticketSaleValue, TICKET_SALE_ERROR); and now estimateGas only reverts when TFUEL sent is less than 20 TFUEL, which is the opposite of before. So somewhere, ticketSaleValue that we calculate no longer matches the msg.value being sent, and this only happens in estimateGas, not in the actual transaction.

Here are the number of ways I can think of on my end that causes this revert:

1. msg.value is not being set correctly
2. metadata.ticketPrice was not saved properly
3. numberOfTickets is not being set corrently
4. There is some overflow that happens in SafeMath.mul

However, I've doubled checked and the value being sent to the smart contract is multiplied correctly without any overflows. And I've also read metadata.ticketPrice from the smart contract, and it seems to be properly saved. I've also checked that numberOfTickets is set correctly on the method call too.

I'm also using this SafeMath.mul function and it seems to protect against overflow:

    function mul(uint256 a, uint256 b) internal pure returns (uint256) {
        // Gas optimization: this is cheaper than requiring 'a' not being zero, but the
        // benefit is lost if 'b' is also tested.
        // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
        if (a == 0) {
            return 0;
        }

        uint256 c = a * b;
        require(c / a == b, "SafeMath: multiplication overflow");

        return c;
    }

I'm quite puzzled to know what exactly went wrong, because the revert only happens in estimateGas, but the actual transaction goes through fine with the gas manually set. If only there was a way to emit some event during the estimateGas call, to see what msg.value and ticketSaleValue is! Could you please advice on what could possibly be the issue? Thank you!

[rjx18]

Hello! I believe I have found the error.

It seems like replacing the requirement above to: require(msg.value == 18446744073709551615); makes the estimateGas function not revert anymore when calling with msg.value greater than 20 TFUEL (or rather, 18.446 TFUEL exactly).

This number also happens to be the max uint64 value of Go, which the RPC adaptor is written in.

This revert does not occur with the actual method call with manually set gas fees, and the msg.value sent only overflows in the estimateGas function call.

Could you please check if the estimateGas function is properly parsing uint256 values for msg.value? I think that may be the source of the error. Thank you!

[jieyilong]

Good catch! We'll check the relevant code and get back to you. Thanks

[jieyilong]

Hey @rjx18 we have just implemented a fix (https://github.com/thetatoken/theta-eth-rpc-adaptor/commit/baca21f14f51acd207c212506fbe8d8300fc0c56) and updated the RPC adapter for both the testnet and mainnet. Please let us know if this addressed the issue. Thanks!

[rjx18]

Hello! Yes I can confirm that it works now. Thanks a lot! By the way, while you're here and so sorry for hijacking this thread, but is the Testnet for the RPC adaptor referring to this one: https://beta-explorer.thetatoken.org, and not the smart contracts sandbox?

If so, I've been asking Wes to send me 1000 TFUEL for the RPC testnet instead for this address 0x56d63a03c9FEDe7CfdEB839e5AFB33f76842358C, could you help me with that instead? Thank you so much for your help!

[jieyilong]

Thanks for confirming the fix! Yes sorry for the confusion, but the Testnet indeed refers to https://beta-explorer.thetatoken.org. I've just sent you 1,000 Testnet TFuel:

https://testnet-explorer.thetatoken.org/tx/0x40ffba2a48403a3d27e87f9753f0d41b40be5c6b8e3a1ded80fd0ac02d030899