0

I have this function:

    function setAccount(BaseUBI _ubi, address _user, uint256 _startTime, uint _esiaID, bool _setToZero) external {
        uint256 _refund = (gasleft() + 0/*FIXME*/) * tx.gasprice;
        require(msg.sender == server, "System function"); // don't refund otherwise
        require(_refund <= balances[_user], "Not enough balance");
        balances[_user] -= _refund; // must be called before transfer() against reentrancy attack
        server.transfer(_refund); // refund gas to the server
        _ubi.setAccount{gas: balances[_user]}(_user, _startTime, _esiaID, _setToZero);
        // We may be in a wrong state now, don't change any variables here.
    }

Which number should be in place of TODO to ensure that the server will receive back at least as much gas as it spent?

Is there a library that automates this task?

If necessary, rewrite the appropriate part of the above code in assembler.

6
  • Question edited: there was a security bug in the code in the question.
    – porton
    Aug 28, 2020 at 19:15
  • balances[_user] -= _refund; should come before server.transfer(_refund);. Aug 28, 2020 at 20:27
  • @goodvibration Why? In Ethereum the entire transaction succeeds or the entire transaction fails. So the order is not important. Or do I misunderstand?
    – porton
    Aug 28, 2020 at 20:36
  • You must reduce the user's balance before sending the transaction. Otherwise, your contract is susceptible to a reentrancy attack: medium.com/coinmonks/…
    – Eric
    Aug 29, 2020 at 7:58
  • Have a look at the Checks, Effects and Interactions pattern to avoid the possibilities of reentrancy attacks fravoll.github.io/solidity-patterns/…
    – wslyvh
    Aug 29, 2020 at 9:33

1 Answer 1

1

I guess you can use a modifier to do this.

modifier refundGas {
uint256 gasAtStart = gasleft();
_;
uint256 gasSpent = gasAtStart - gasleft() + 28925;
msg.sender.transfer(gasSpent * tx.gasprice);
}

function anyFunction() refundGas public {
    // Do something
}

Note: The number 28925 is an approx. of the gas used for calculations + transfer at the end. I've run on remix and put that here. However, it might be slightly off as well

However, I also think if you would refund gas then someone possibly could waste all the ETH inside the contract in refunds. For e.g., server could intentionally set a very high gas price even if it's unnecessary to do so, knowing that the contract would refund them 100%.

3
  • The question was about a number that warrants that all the gas (or more) will be refunded.
    – porton
    Sep 5, 2020 at 11:32
  • Oh, I see, my code was wrong. Should refund only if the request was sent from server.
    – porton
    Sep 5, 2020 at 11:35
  • 1
    Also, yes the refunds should probably be limited to known/authorized users only. That might reduce misuses. Sep 5, 2020 at 12:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.