Raise transfer stipend

[nlordell]

This PR just raises the transfer stipend to that it is enough for a Gnosis Safe.

For the chosen stipend amount, I simulated a transaction on Tenderly and got the following numbers:

Of the 7389 gas for the transfer, 2600 was the COLD_ACCOUNT_ACCESS_COST for calling a contract for the first time:

When an address is either the target of a (EXTCODESIZE (0x3B), EXTCODECOPY (0x3C), EXTCODEHASH (0x3F) or BALANCE (0x31)) opcode or the target of a (CALL (0xF1), CALLCODE (0xF2), DELEGATECALL (0xF4), STATICCALL (0xFA)) opcode, the gas costs are computed as follows:

• If the target is not in accessed_addresses, charge COLD_ACCOUNT_ACCESS_COST gas, and add the address to accessed_addresses.
• Otherwise, charge WARM_STORAGE_READ_COST gas.

The actual gas limit of the call context is 7389 - 2600 = 4789. This makes sense since the proxy will:

• Read slot 0 (cold) for the implementaiton address (2100 for COLD_SLOAD_COST):

  For SLOAD, if the (address, storage_key) pair (where address is the address of the contract whose storage is being read) is not yet in accessed_storage_keys, charge COLD_SLOAD_COST gas and add the pair to accessed_storage_keys. If the pair is already in accessed_storage_keys, charge WARM_STORAGE_READ_COST gas.

• Delegate call to the implementation (cold) contract (2600 for COLD_ACCOUNT_ACCESS_COST)
• 89 gas for the rest of things - copying call data and return data, etc. (Note that this is for Safe v1.1.1).

1000 additional gas was chosen so it is enough to log an even on receiving Ether for the Save v1.3.

Test Plan

Added a test to make sure the Eth transfer works for a Gnosis Safe v1.3 on Berlin hardfork 🎉.

Issues

Currently there is an issue that the with the increased stipend, SSTOREs are actually allowed and can be used to modify storage, as made evident by the failing should revert when transferring Ether to contract writes storage test. THis is because EIP-2200 makes storage writes on non-zero storage only cost 5000k gas.

[fedgiac]

Looks good to me!
Another test that could be interesting is checking that no storage write can be performed in this call.

[fedgiac]

I see this should be the default value, is this to "freeze" the fork in case there are incompatible changes in a future fork?

[nlordell]

Ah, that was left over when I was trying some stuff out. I'll remove it. In fact, the block gas limit setting can also be removed as the default is the same here.

[rmeissner]

why not equal 2 instead of equal 1?

[nlordell]

🤷 - equal 1 is more specific so will change to this.

[rmeissner]

These seem to be the 1.2.0 contract. With 1.3.0 there will also be an event that is emitted in the receive case. Is this taken into consideration?

Mentioned in the PR description ... Everything looks good

[rmeissner]

Note: you could pull the ABI from https://github.com/gnosis/safe-deployments but we are currently missing the deployment code in the deployments which might be interesting for tests ... I opened an issue for this: safe-global/safe-deployments#8

[rmeissner]

Edit: was not even aware that we include the hardhat build artifacts in the npm release 😬

[nlordell]

Closing this because of the aforementioned storage write issue. We will instead add access list support in order to properly support SC wallets.