Solidity-coverage: 0.6 receive fallback function instrumented

[elenadimitrova]

transfer call to instrumented receive fallback function fails as it probably exceeds the transfer call gas stipend. Similar to https://github.com/sc-forks/solidity-coverage/issues/106

[cgewecke]

@elenadimitrova

Am looking at this right now... the test we have with a fallback that looks like this

    receive() external payable
    {
        if (msg.value > 0)
            emit Deposit(msg.sender, msg.value);
    }

...is passing. Client is ganache-core v2.10.1 (MuirGlacier).

It probably works because it's too simple and doesn't use much gas.

Can you show a receive example that's failing for you?

[elenadimitrova]

The updated Proxy contract here is failing to receive value. I'll try to get a min working example for you.

[cgewecke]

@elenadimitrova

Adding a third parameter to the event (bytes data) was enough to get it to fail on this side.

Is the fallback with the assembly working ok though?

[elenadimitrova]

Seems that way. There are ~70 occurrences of invokeWallet that does the low level call and they are working. That has sufficient gas passed to it though so it's different to doing a transfer. Why do you suspect it might be failing?

[cgewecke]

Why do you suspect it might be failing?

There's some gas distortion caused by Solidity statements the tool injects to track the execution path. Hopefully it's possible to minimize those for receive and keep things under the stipend limit though.

Will try to fix this tomorrow and publish an experimental patch, and maybe you could check that it works for those contracts.

[cgewecke]

@elenadimitrova 0.7.7 hopefully fixes. Just lmk if not and will revisit.

[elenadimitrova]

Thanks @cgewecke but still having the same problem. receive was instrumented as follows:

receive() external payable {
coverage_0x583770bd(0x067553a7ab4566e3d100d5dd51a4d4d7c1dd78170cd2de2ede027b3713d158ca); /* line */ 
        if (msg.value > 0) {coverage_0x583770bd(0x6f53e18b49c038dc954f681591a954eba8d5f4cfb34caeeaa63ac41fda82672c); /* branch */ 

coverage_0x583770bd(0x76f304403c4c60d131d831b10dbe528a8037658735d6680c87b0f63310580e12); /* line */ 
            emit Received(msg.value, msg.sender, msg.data);
        }else { coverage_0x583770bd(0x62bc3173bb736c7ddf6032536275c27f047a20ac743535b1a36d2e4191f6f6d8); /* branch */ 
}
    }

[cgewecke]

@elenadimitrova Oh I'm sorry. That's strange.

We have very similar instrumentation in the test here and it's passing. These send & transfer examples:

• Solidity
• JS tests

...run this...

  event Deposit(address indexed _sender, uint _value, bytes data);

  receive() external payable
  {
    coverage_0xf2a277f5(0xb2baf8814b88a8e10df7811cfe4de699f4dfa82fee21030cef466b986ce61cb7); /* line */ 
    if (msg.value > 0){
      coverage_0xf2a277f5(0xa4b8e24690a0c94debffedca61a226696034c80f40f925c74914509c12313b8b); /* branch */ 
      coverage_0xf2a277f5(0xbbd5fa732f9422ad22c57d343d767a696fb98bd2e856cc657c377d4ebe8fe8ea); /* line */ 
      emit Deposit(msg.sender, msg.value, msg.data);
    } else { 
      coverage_0xf2a277f5(0xc454deeb9e6220ae9751a1a45b1b5f6cf82972ab7c93be8d76a188b9bc8a5c9e); /* branch */ 
    }
  }

Have also experimented with removing the indexed modifier in the event definition and it works.

What is the value of msg.data for you when that event fires normally? Here it's null, which I thought might be expected because the Solidity docs mention receive not accepting calldata.