[Constantinople] Add Constantinople SSTORE gas/refund prices (EIP1283)

[vpulim]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 99

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 95.909%

---

Totals
Change from base Build 97:	0.0%
Covered Lines:	127
Relevant Lines:	131

---

💛 - Coveralls

[holgerd77]

Haven't looked in detail into the EIP yet, but doesn't any of these values correspond to (respectively are identical) to the old SSTORE values defined in https://github.com/ethereumjs/ethereumjs-common/blob/master/hardforks/chainstart.json? (Or later, not sure, but probably double check would be good)

[vpulim]

Some of them are the same as old SSTORE values but there are several new values and I wanted to keep the naming consistent with geth: https://github.com/ethereum/go-ethereum/pull/17383/files#diff-c3868eb970ebbbf00a0a47a656d12ce0L39

[holgerd77]

Ah, nevertheless that's not good for the consistency of the library if we have floating around same value references differently named. Then also rename the old versions to the new geth-conforming names and we do a breaking release here, more or less no-one is using this library yet anyhow.

[vpulim]

Although some of the new EIP-1283 params may have the same values as existing ones, they have a different semantic meaning, so I thought it would be clearer to create new constants. This is especially true in the context of ethereumjs-common since it provides description fields for each parameter, and using that parameter in a different way than intended can be confusing for developers. For example, take a look at sstoreRefund:

ethereumjs-common:

    "sstoreRefund": {
      "v": 15000,
      "d": "Once per SSTORE operation if the zeroness changes to zero"
    }

geth:

    SstoreRefundGas uint64 = 15000 // Once per SSTORE operation if the zeroness changes to zero.

In EIP-1283, there are two different ways that a value can change to zero depending on what the original value was. This is represented by two new params in geth:

geth (eip-1283):

    NetSstoreClearRefund      uint64 = 15000 // Once per SSTORE operation for clearing an originally existing storage slot
    NetSstoreResetClearRefund uint64 = 19800 // Once per SSTORE operation for resetting to the original zero value

I believe you are proposing that instead of creating these new params, we re-use sstoreRefund for the 15000 value and create a new param (maybe sstoreResetRefund) for the 19800 value? Or if we are trying to avoid creating new params altogether, we could take it a step further and use sstoreSet - sload to get to the value of 19800 (parity takes this approach in their eip-1283 implementation). While this would certainly work, it can make it confusing for developers when they read the description of a parameter in a hardfork json file and then see that parameter being used in a different way.

On the other hand, I can see the appeal of not having to change the ethereumjs-common module at all in order to implement eip-1283 since it would be less work.

I'm totally fine with whatever approach we decide to take. If you have a strong preference for re-using existing parameters, just let me know and I can easily modify ethereumjs/ethereumjs-monorepo#367.

[holgerd77]

Ah no, I am not so deeply into it, if semantics changes it is very much justified to just have new parameters and we can keep this "as is", will directly merge + do a release.

Thanks for the through explanation, really great for getting the context.

[holgerd77]

(P.S: are you up late or early or just in an unexpected timezone? 😄 😄 )

[holgerd77]

Looks good.

[vpulim]

(P.S: are you up late or early or just in an unexpected timezone? 😄 😄 )

Up late... lol. Baby is having a rough night 😞