Incorrect gas cost for SSTORE opcode when using fork feature

[nebojsa94]

Executing transactions that use SSTORE opcode costs less gas when using fork feature.

Expected Behavior

SSTORE that sets the value v in the slot s should cost 20k gas when the current value in slot s is 0x0000000000000000000000000000000000000000000000000000000000000000 and v is not 0x0000000000000000000000000000000000000000000000000000000000000000.

Current Behavior

SSTORE mentioned above costs 5k gas.

Possible Solution

This is probably an issue with the gas calculator, SSTORE should cost 5k gas when the current value in the slot is not
0x0000000000000000000000000000000000000000000000000000000000000000

Steps to Reproduce (for bugs)

1. ganache-cli -f mainnet_node --networkId 1
2. truffle migrate
3. Saving migration to the chain costs 27363 gas, but it should cost 42363 gas.

Your Environment

• Version used: Ganache CLI v6.10.1-beta.2 (ganache-core: 2.11.3-beta.0)
• Operating System and version: macOS Catalina 10.15.4

[mikeseese]

@nebojsa94 do you have a project that I can recreate this with? I'll give it a shot myself, but my hunch is that I'm not going to be able to reproduce since gas costs are pretty transparent (they're calculated upstream in https://github.com/ethereumjs/ethereumjs-vm)

Do you only observe this during forking (you've tested it when not forking and see it provides the expected value)?

Remember that SSTORE gas costs also costs 5k if the zeroness remains unchanged; I gotta do some more research on what what bit-level this pertains to, however if it's at the nibble level then changing 0x0000000000000000000000000000000000000000000000000000000000000001 to 0x0000000000000000000000000000000000000000000000000000000000000002 would cost 5k

[nebojsa94]

Hey @seesemichaelj, thank you for the quick reply!

do you have a project that I can recreate this with?

You can recreate with an empty truffle project using Migration.sol contract, just run truffle migrate, you'll see two transactions, one that creates Migration contract and the second one that saves completed migration.

Do you only observe this during forking (you've tested it when not forking and see it provides the expected value)?`

I've tested in a nonfork environment and both transactions are spending 15k more gas than in a fork environment (contract creation: 210453 -> 225453, migration saving: 27363-> 42363). Using debug_traceTransaction I was able to confirm that the issue is in fact with gas spent in SSTORE opcode.

Remember that SSTORE gas costs also costs 5k if the zeroness remains unchanged; I gotta do some more research on what what bit-level this pertains to, however if it's at the nibble level then changing 0x0000000000000000000000000000000000000000000000000000000000000001 to 0x0000000000000000000000000000000000000000000000000000000000000002 would cost 5k

This is the case with the newly deployed contract, so every storage slot is empty (0x0000000000000000000000000000000000000000000000000000000000000000), and since the owner of the contract that is set during Migration contract creation is never 0x0000000000000000000000000000000000000000, the SSTORE should cost 20k. When setting completed migrations in the second transaction, the input is not 0, therefore this should also cost 20k.

[mikeseese]

Awesome, thanks @nebojsa94 for the clarification

[nebojsa94]

Hey @seesemichaelj,

Wanna team-up on resolving this issue?
I can investigate why this is happening and submit a description of the solution

[mikeseese]

Hey @nebojsa94! I'm out of office until Oct 23rd, so plenty of time for you to do some initial investigation :) I definitely would appreciate the help. I may be moving off of forking bugs in the coming weeks

[mikeseese]

Hey @nebojsa94, I'm back in the office and wanted to touch base to see if you got anywhere with this?

[nebojsa94]

Hey @seesemichaelj, I started looking into this during the weekend, hopefully, I will have a proposal by the end of tomorrow.

[mikeseese]

awesome, thanks!

[nebojsa94]

Hey @seesemichaelj, sorry for the delays,

It was pretty hard to find the actual issue that led to the incorrect SSTORE gas usage, but I think I figured it out :)

We can use the test in forking.js on line 179 with modified Example.sol where the constructor function doesn't set the value property, the gas usage in the receipt in this test should be 42535, but atm it is 27535, the reason is that the SSTORE gas calculator doesn't realize that the original value is empty but instead goes to storage-slot-update, this is because original field has the value [0] instead of [].

StateManager is responsible for storage fetching during the execution of the transaction, this line will call ForkedStorageTrie implementation which is going to invoke web3.eth.getStorageAt that is going to return 0x0, unfortunately, this is going to encode into [0] and won't trigger slot creation.

The fix should be pretty straight forward, just make sure 0x0 encodes to [] buffer

[sz-piotr]

This is still not fixed. I encountered the issue also on 2.13.2. Could this be reopened @davidmurdoch?

[nebojsa94]

Hey @sz-piotr, can you provide steps for reproduction?

[sz-piotr]

This is the contract that I used.

pragma solidity ^0.7.0;

contract GasTest {
    event GasUsed(uint256 value);

    uint256 public x = 0;

    function set (uint value) public {
        x = value;
    }

    function setNonZero() public {
        uint256 start = gasleft();
        set(1337);
        uint256 used = start - gasleft();
        emit GasUsed(used);
    }
}

In the tests i forked mainnet from infura and checked the logs for GasUsed in setNonZero. The value was ~5000 when forking and ~20000 when not forking.

I am very sorry that I cannot provide a full working example for you.

[nebojsa94]

I can confirm that the issue is still present if the state variable is set to the default value in the declaration statement; in this case, if you remove = 0 in uint256 public x = 0; the SSTORE opcode will cost 20k, which is correct behavior.

[gnidan]

We're currently revamping forking for the upcoming Ganache v7.0.0 release. Adding this to our plate (David's plate) for inclusion there. Thanks David ❤️

And thanks everyone for reporting / getting to the bottom of this!