Setting storage variables in fallback reverts

[illuzen]

Ok so I finally narrowed this down to a simple example. If I set a storage variable in a fallback in a contract that I deployed, it works as expected. If I set it via a transfer from another contract, same permissions, it reverts.

Expected Behavior

It should let me set storage variables in fallback functions called via other contracts. This may be an out of gas issue, but if so it isn't being reported as such.

Current Behavior

It reverts when changing storage variables via fallbacks called by other contracts.

Steps to Reproduce (for bugs)

contracts/A.sol:

pragma solidity ^0.4.23;

contract A {

   B public b;
   C public c;

   constructor() public {
      b = new B();
      c = new C();
   }

   function sendB() payable
   {
      address(b).transfer(msg.value);
   }

   function sendC() payable
   {
      address(c).transfer(msg.value);
   }

   function setC(uint8 n) {
      b.setC(n);
   }

}

contract B {

   uint8 public c;

   constructor() public {}

   function () payable {
      c = 1;
   }

   function setC(uint8 n) {
      c = n;
   }
}

contract C {


   constructor() public {}

   function () payable {
      uint a = 0;
      for (var i = 0; i < 25; i++) {
         a += 1;
      }
   }

}

test/a.js

var A = artifacts.require('A')
var B = artifacts.require('B')

let a

contract('A', function(accounts) {

   beforeEach(async () => {
      a = await A.new({gas: 5e6, gasPrice:2e9})
   })


   it('breaks', async function () {
      await a.sendB({from: accounts[0], value:web3.toWei(1), gas:1e6})
   })

   it('no breaky', async function () {
      b = await B.new({gas: 5e6, gasPrice:2e9})
      hsh = await web3.eth.sendTransaction({ from: accounts[0], to: b.address, value:web3.toWei(1), gas:1e6})
      rcpt = await web3.eth.getTransactionReceipt(hsh)
      console.log('gasUsed:', rcpt.gasUsed)
   })

   it('no breaky again', async function () {
      await a.setC(1)
   })

   it('no breaky bc gas', async function () {
      rcpt = await a.sendC({from: accounts[0], value:web3.toWei(1), gas:1e6})
      console.log('gasUsed:', rcpt.receipt.gasUsed)
   })

})

Context

Trying to have two linked contracts handle ether separately and synchronously.

Your Environment

Ganache CLI v6.1.3 (ganache-core: 2.1.2)
Truffle v4.1.11 (core: 4.1.11)
Solidity v0.4.24 (solc-js)
OSX 10.12.6

Here's the truffle project if you want it. Just run

truffle test

BugProof.zip

[mikeseese]

Hey @illuzen! Sorry for the delay.

The revert is happening due to an out of gas error. The transfer() function sends a CALL to the fallback function with a unchangeable gas limit of 2300 to help prevent reentrancy attacks.

Setting to storage is 5000 (nonzero->zero or value->zero) or 20000 (zero->nonzero) for just 32 bytes. Either way, this would be more than 2300 and revert. The a.setC() is fine because Cs fallback function does some real simple instructions with a variable on the stack.

As far as why the error message wasn't more helpful (i.e. why not say out of gas instead of revert?), I believe this confusion comes from some check (either ethereumjs-vm or solidity compiled code) where it reverts if it knows there wont be enough gas rather than executing, then getting OOG, and paying the gas that could have been saved with this check.

I'm going to close this issue as the revert message instead of OOG is outside of ganche's realm. I'm going to look into this a little further however to see if I can track down the real reason for the revert instead of out of gas. Feel free to open this if you know of a way which we could determine the reason was actually because of an out of gas error instead of a revert opcode.

Thanks! I'll let you know if I find any more details.