Fix Constantinople tests

.github/workflows/vm-pr.yml

@@ -51,7 +51,18 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        fork: ['Berlin', 'MuirGlacier', 'Petersburg', 'Istanbul', 'Byzantium', 'SpuriousDragon', 'TangerineWhistle', 'Homestead', 'Chainstart']
+        fork: [
+          'Berlin',
+          'MuirGlacier',
+          'Istanbul',
+          'Petersburg',
+          'Constantinople',
+          'Byzantium',
+          'SpuriousDragon',
+          'TangerineWhistle',
+          'Homestead',
+          'Chainstart'
+        ]
       fail-fast: false
     steps:
       - uses: actions/setup-node@v1
@@ -83,7 +94,17 @@ jobs:
     strategy:
       matrix:
         # Args to pass to the tester. Note that some have splitted the slow tests and only running those: these are only on forks where that is applicable (see PR #489 for numbers on these)
-        args: ['--fork=Chainstart --expected-test-amount=4385', '--fork=Homestead --expected-test-amount=6997', '--fork=Petersburg --excludeDir=stTimeConsuming --expected-test-amount=17174', '--fork=Petersburg --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561', '--fork=Istanbul --excludeDir=stTimeConsuming --expected-test-amount=19817', '--fork=Istanbul --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561', '--fork=Berlin --expected-test-amount=33']
+        args: [
+          '--fork=Chainstart --expected-test-amount=4385',
+          '--fork=Homestead --expected-test-amount=6997',
+          '--fork=Petersburg --excludeDir=stTimeConsuming --expected-test-amount=17174',
+          '--fork=Petersburg --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
+          '--fork=Constantinople --excludeDir=stTimeConsuming --expected-test-amount=17193',
+          '--fork=Constantinople --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
+          '--fork=Istanbul --excludeDir=stTimeConsuming --expected-test-amount=19817',
+          '--fork=Istanbul --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561',
+          '--fork=Berlin --expected-test-amount=33'
+        ]
       fail-fast: false
     steps:
       - uses: actions/setup-node@v1
@@ -159,4 +180,4 @@ jobs:
 
       # Re-apply git stash to prepare for saving back to cache.
       # Avoids exit code 1 by checking if there are changes to be stashed first
-      - run: STASH_LIST=`git stash list` && [ ! -z $STASH_LIST ] && git stash apply || echo "No files to stash-apply. Skipping…"
+      - run: STASH_LIST=`git stash list` && [ ! -z $STASH_LIST ] && git stash apply || echo "No files to stash-apply. Skipping…"

packages/vm/lib/evm/opcodes/EIP1283.ts

@@ -9,51 +9,48 @@ import { RunState } from './../interpreter'
  * @param {Buffer}   value
  */
 export function updateSstoreGasEIP1283(runState: RunState, found: any, value: Buffer) {
-  if (runState._common.hardfork() === 'constantinople') {
-    const original = found.original
-    const current = found.current
-    if (current.equals(value)) {
-      // If current value equals new value (this is a no-op), 200 gas is deducted.
-      runState.eei.useGas(new BN(runState._common.param('gasPrices', 'netSstoreNoopGas')))
-      return
+  const { original, current } = found
+  if (current.equals(value)) {
+    // If current value equals new value (this is a no-op), 200 gas is deducted.
+    runState.eei.useGas(new BN(runState._common.param('gasPrices', 'netSstoreNoopGas')))
+    return
+  }
+  // If current value does not equal new value
+  if (original.equals(current)) {
+    // If original value equals current value (this storage slot has not been changed by the current execution context)
+    if (original.length === 0) {
+      // If original value is 0, 20000 gas is deducted.
+      return runState.eei.useGas(new BN(runState._common.param('gasPrices', 'netSstoreInitGas')))
     }
-    // If current value does not equal new value
-    if (original.equals(current)) {
-      // If original value equals current value (this storage slot has not been changed by the current execution context)
-      if (original.length === 0) {
-        // If original value is 0, 20000 gas is deducted.
-        return runState.eei.useGas(new BN(runState._common.param('gasPrices', 'netSstoreInitGas')))
-      }
-      if (value.length === 0) {
-        // If new value is 0, add 15000 gas to refund counter.
-        runState.eei.refundGas(new BN(runState._common.param('gasPrices', 'netSstoreClearRefund')))
-      }
-      // Otherwise, 5000 gas is deducted.
-      return runState.eei.useGas(new BN(runState._common.param('gasPrices', 'netSstoreCleanGas')))
+    if (value.length === 0) {
+      // If new value is 0, add 15000 gas to refund counter.
+      runState.eei.refundGas(new BN(runState._common.param('gasPrices', 'netSstoreClearRefund')))
     }
-    // If original value does not equal current value (this storage slot is dirty), 200 gas is deducted. Apply both of the following clauses.
-    if (original.length !== 0) {
-      // If original value is not 0
-      if (current.length === 0) {
-        // If current value is 0 (also means that new value is not 0), remove 15000 gas from refund counter. We can prove that refund counter will never go below 0.
-        runState.eei.subRefund(new BN(runState._common.param('gasPrices', 'netSstoreClearRefund')))
-      } else if (value.length === 0) {
-        // If new value is 0 (also means that current value is not 0), add 15000 gas to refund counter.
-        runState.eei.refundGas(new BN(runState._common.param('gasPrices', 'netSstoreClearRefund')))
-      }
+    // Otherwise, 5000 gas is deducted.
+    return runState.eei.useGas(new BN(runState._common.param('gasPrices', 'netSstoreCleanGas')))
+  }
+  // If original value does not equal current value (this storage slot is dirty), 200 gas is deducted. Apply both of the following clauses.
+  if (original.length !== 0) {
+    // If original value is not 0
+    if (current.length === 0) {
+      // If current value is 0 (also means that new value is not 0), remove 15000 gas from refund counter. We can prove that refund counter will never go below 0.
+      runState.eei.subRefund(new BN(runState._common.param('gasPrices', 'netSstoreClearRefund')))
+    } else if (value.length === 0) {
+      // If new value is 0 (also means that current value is not 0), add 15000 gas to refund counter.
+      runState.eei.refundGas(new BN(runState._common.param('gasPrices', 'netSstoreClearRefund')))
     }
-    if (original.equals(value)) {
-      // If original value equals new value (this storage slot is reset)
-      if (original.length === 0) {
-        // If original value is 0, add 19800 gas to refund counter.
-        runState.eei.refundGas(
-          new BN(runState._common.param('gasPrices', 'netSstoreResetClearRefund'))
-        )
-      } else {
-        // Otherwise, add 4800 gas to refund counter.
-        runState.eei.refundGas(new BN(runState._common.param('gasPrices', 'netSstoreResetRefund')))
-      }
+  }
+  if (original.equals(value)) {
+    // If original value equals new value (this storage slot is reset)
+    if (original.length === 0) {
+      // If original value is 0, add 19800 gas to refund counter.
+      runState.eei.refundGas(
+        new BN(runState._common.param('gasPrices', 'netSstoreResetClearRefund'))
+      )
+    } else {
+      // Otherwise, add 4800 gas to refund counter.
+      runState.eei.refundGas(new BN(runState._common.param('gasPrices', 'netSstoreResetRefund')))
     }
-    return runState.eei.useGas(new BN(runState._common.param('gasPrices', 'netSstoreDirtyGas')))
   }
+  return runState.eei.useGas(new BN(runState._common.param('gasPrices', 'netSstoreDirtyGas')))
 }

packages/vm/lib/evm/opcodes/EIP2200.ts

@@ -12,82 +12,65 @@ import { trap } from './util'
  * @param {Buffer}   value
  */
 export function updateSstoreGasEIP2200(runState: RunState, found: any, value: Buffer, key: Buffer) {
-  if (runState._common.gteHardfork('istanbul')) {
-    const original = found.original
-    const current = found.current
-    // Fail if not enough gas is left
-    if (
-      runState.eei.getGasLeft().lten(runState._common.param('gasPrices', 'sstoreSentryGasEIP2200'))
-    ) {
-      trap(ERROR.OUT_OF_GAS)
-    }
+  const { original, current } = found
+  // Fail if not enough gas is left
+  if (
+    runState.eei.getGasLeft().lten(runState._common.param('gasPrices', 'sstoreSentryGasEIP2200'))
+  ) {
+    trap(ERROR.OUT_OF_GAS)
+  }
 
-    // Noop
-    if (current.equals(value)) {
-      const sstoreNoopCost = runState._common.param('gasPrices', 'sstoreNoopGasEIP2200')
+  // Noop
+  if (current.equals(value)) {
+    const sstoreNoopCost = runState._common.param('gasPrices', 'sstoreNoopGasEIP2200')
+    return runState.eei.useGas(
+      new BN(adjustSstoreGasEIP2929(runState, key, sstoreNoopCost, 'noop'))
+    )
+  }
+  if (original.equals(current)) {
+    // Create slot
+    if (original.length === 0) {
       return runState.eei.useGas(
-        new BN(adjustSstoreGasEIP2929(runState, key, sstoreNoopCost, 'noop'))
+        new BN(runState._common.param('gasPrices', 'sstoreInitGasEIP2200'))
       )
     }
-    if (original.equals(current)) {
-      // Create slot
-      if (original.length === 0) {
-        return runState.eei.useGas(
-          new BN(runState._common.param('gasPrices', 'sstoreInitGasEIP2200'))
-        )
-      }
-      // Delete slot
-      if (value.length === 0) {
-        runState.eei.refundGas(
-          new BN(runState._common.param('gasPrices', 'sstoreClearRefundEIP2200'))
-        )
-      }
-      // Write existing slot
-      return runState.eei.useGas(
-        new BN(runState._common.param('gasPrices', 'sstoreCleanGasEIP2200'))
+    // Delete slot
+    if (value.length === 0) {
+      runState.eei.refundGas(
+        new BN(runState._common.param('gasPrices', 'sstoreClearRefundEIP2200'))
       )
     }
-    if (original.length > 0) {
-      if (current.length === 0) {
-        // Recreate slot
-        runState.eei.subRefund(
-          new BN(runState._common.param('gasPrices', 'sstoreClearRefundEIP2200'))
-        )
-      } else if (value.length === 0) {
-        // Delete slot
-        runState.eei.refundGas(
-          new BN(runState._common.param('gasPrices', 'sstoreClearRefundEIP2200'))
-        )
-      }
-    }
-    if (original.equals(value)) {
-      if (original.length === 0) {
-        // Reset to original non-existent slot
-        const sstoreInitRefund = runState._common.param('gasPrices', 'sstoreInitRefundEIP2200')
-        runState.eei.refundGas(
-          new BN(adjustSstoreGasEIP2929(runState, key, sstoreInitRefund, 'initRefund'))
-        )
-      } else {
-        // Reset to original existing slot
-        const sstoreCleanRefund = runState._common.param('gasPrices', 'sstoreCleanRefundEIP2200')
-        runState.eei.refundGas(
-          new BN(adjustSstoreGasEIP2929(runState, key, sstoreCleanRefund, 'cleanRefund'))
-        )
-      }
+    // Write existing slot
+    return runState.eei.useGas(new BN(runState._common.param('gasPrices', 'sstoreCleanGasEIP2200')))
+  }
+  if (original.length > 0) {
+    if (current.length === 0) {
+      // Recreate slot
+      runState.eei.subRefund(
+        new BN(runState._common.param('gasPrices', 'sstoreClearRefundEIP2200'))
+      )
+    } else if (value.length === 0) {
+      // Delete slot
+      runState.eei.refundGas(
+        new BN(runState._common.param('gasPrices', 'sstoreClearRefundEIP2200'))
+      )
     }
-    // Dirty update
-    return runState.eei.useGas(new BN(runState._common.param('gasPrices', 'sstoreDirtyGasEIP2200')))
-  } else {
-    const sstoreResetCost = runState._common.param('gasPrices', 'sstoreReset')
-    if (value.length === 0 && !found.length) {
-      runState.eei.useGas(new BN(adjustSstoreGasEIP2929(runState, key, sstoreResetCost, 'reset')))
-    } else if (value.length === 0 && found.length) {
-      runState.eei.useGas(new BN(adjustSstoreGasEIP2929(runState, key, sstoreResetCost, 'reset')))
-      runState.eei.refundGas(new BN(runState._common.param('gasPrices', 'sstoreRefund')))
-    } else if (value.length !== 0 && !found.length) {
-      runState.eei.useGas(new BN(runState._common.param('gasPrices', 'sstoreSet')))
-    } else if (value.length !== 0 && found.length) {
-      runState.eei.useGas(new BN(adjustSstoreGasEIP2929(runState, key, sstoreResetCost, 'reset')))
+  }
+  if (original.equals(value)) {
+    if (original.length === 0) {
+      // Reset to original non-existent slot
+      const sstoreInitRefund = runState._common.param('gasPrices', 'sstoreInitRefundEIP2200')
+      runState.eei.refundGas(
+        new BN(adjustSstoreGasEIP2929(runState, key, sstoreInitRefund, 'initRefund'))
+      )
+    } else {
+      // Reset to original existing slot
+      const sstoreCleanRefund = runState._common.param('gasPrices', 'sstoreCleanRefundEIP2200')
+      runState.eei.refundGas(
+        new BN(adjustSstoreGasEIP2929(runState, key, sstoreCleanRefund, 'cleanRefund'))
+      )
     }
   }
+  // Dirty update
+  return runState.eei.useGas(new BN(runState._common.param('gasPrices', 'sstoreDirtyGasEIP2200')))
 }

packages/vm/lib/evm/opcodes/functions.ts

@@ -20,6 +20,7 @@ import {
   subMemUsage,
   trap,
   writeCallOutput,
+  updateSstoreGas,
 } from './util'
 import { updateSstoreGasEIP1283 } from './EIP1283'
 import { updateSstoreGasEIP2200 } from './EIP2200'
@@ -745,8 +746,15 @@ export const handlers: Map<number, OpHandler> = new Map([
       // TODO: Replace getContractStorage with EEI method
       const found = await getContractStorage(runState, runState.eei.getAddress(), keyBuf)
       accessStorageEIP2929(runState, keyBuf, true)
-      updateSstoreGasEIP1283(runState, found, setLengthLeftStorage(value))
-      updateSstoreGasEIP2200(runState, found, setLengthLeftStorage(value), keyBuf)
+
+      if (runState._common.hardfork() === 'constantinople') {
+        updateSstoreGasEIP1283(runState, found, setLengthLeftStorage(value))
+      } else if (runState._common.gteHardfork('istanbul')) {
+        updateSstoreGasEIP2200(runState, found, setLengthLeftStorage(value), keyBuf)
+      } else {
+        updateSstoreGas(runState, found, setLengthLeftStorage(value), keyBuf)
+      }
+
       await runState.eei.storageStore(keyBuf, value)
     },
   ],

packages/vm/lib/evm/opcodes/util.ts

@@ -1,6 +1,7 @@
 import { Address, BN, keccak256, setLengthRight, setLengthLeft } from 'ethereumjs-util'
 import { ERROR, VmError } from './../../exceptions'
 import { RunState } from './../interpreter'
+import { adjustSstoreGasEIP2929 } from './EIP2929'
 
 const MASK_160 = new BN(1).shln(160).subn(1)
 
@@ -239,3 +240,21 @@ export function writeCallOutput(runState: RunState, outOffset: BN, outLength: BN
     runState.memory.write(memOffset, dataLength, data)
   }
 }
+
+/** The first rule set of SSTORE rules, which are the rules pre-Constantinople and in Petersburg
+ * @param {RunState} runState
+ * @param {any}      found
+ * @param {Buffer}   value
+ * @param {Buffer}   keyBuf
+ */
+export function updateSstoreGas(runState: RunState, found: any, value: Buffer, keyBuf: Buffer) {
+  const sstoreResetCost = runState._common.param('gasPrices', 'sstoreReset')
+  if ((value.length === 0 && !found.length) || (value.length !== 0 && found.length)) {
+    runState.eei.useGas(new BN(adjustSstoreGasEIP2929(runState, keyBuf, sstoreResetCost, 'reset')))
+  } else if (value.length === 0 && found.length) {
+    runState.eei.useGas(new BN(adjustSstoreGasEIP2929(runState, keyBuf, sstoreResetCost, 'reset')))
+    runState.eei.refundGas(new BN(runState._common.param('gasPrices', 'sstoreRefund')))
+  } else if (value.length !== 0 && !found.length) {
+    runState.eei.useGas(new BN(runState._common.param('gasPrices', 'sstoreSet')))
+  }
+}