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

[R4R]-fix: contract call estimate gas bug #79

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

asdv23
Copy link

@asdv23 asdv23 commented Apr 23, 2024

Bug case:
When calling across contracts, an opCall call error out of gas will occur. However, this error is not execution reverted, causing the error and cannot be captured by the call stack. Therefore, when subsequent opcode is executed, opRevert is triggered because the return value is not the expected value. But if this error is triggered during the bisection process, it can be tolerated because smaller gas is used in the estimation.

How to fix
Therefore, the function of this flag is that as long as estimateGas is successful once during the bisection process, it will not terminate. The process of bisection and throwing an evm error (drawing on the idea of ​​Ethereum, using the largest balance to estimate once, if successful, no subsequent judgment errors will be made, if failed, an exception will be thrown directly)

Refund codes

// 1_Storage.sol
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.7.6;

import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

interface Bank {
    function deposit(uint256) external payable returns (bool);
}

contract Storage {
    address owner;
    address otherCON;

    using SafeERC20 for IERC20;

    Bank b;

    event OwnerSet(address indexed oldOwner, address indexed newOwner);

    constructor(address o, address con) {
        owner = o; // 'msg.sender' is sender of current call, contract deployer for a constructor
        emit OwnerSet(address(0), owner);
        otherCON = con;
    }

    function callDepositETH(uint256 _amount) public payable returns (bool) {
        b = Bank(otherCON);
        bool success = b.deposit{value: msg.value}(_amount);
        require(success, "deposit failed");
        return success;
    }
}

// 4_Bank.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;

contract Bank {
    mapping(address => uint256) public balances;
    address owner;

    constructor(address o) {
        owner = o;
    }

    function deposit(uint256 val) external payable returns (bool) {
        require(tx.origin == owner, "Caller is not owner");
        require(val == msg.value, "val is not equal to val");
        balances[tx.origin] += msg.value;
        return true;
    }
}

Steps:

  1. deploy 4_Bank.sol and 1_Storage.sol
  2. call 1_Storage.sol callDepositETH, input _amount=1

@Tri-stone Tri-stone merged commit 776202e into release/v1.0.0 Apr 23, 2024
2 checks passed
// not terminate. The process of bisection and throwing an evm error (drawing on the idea of ​​Ethereum, using the largest balance
// to estimate once, if successful, no subsequent judgment errors will be made, if failed, an exception will be thrown directly)
var success bool

// Execute the binary search and hone in on an executable gas limit
for lo+1 < hi {
mid := (hi + lo) / 2

Choose a reason for hiding this comment

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

if do call error with first mid gas, this PR don't fix the problem
it is better to do call with hi gas value before into binary search

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants