Merge pull request ethereumjs#807 from ethereumjs/add-tangerine-whistle-support

TangerineWhistle HF Support

Author: holgerd77

packages/vm/lib/evm/eei.ts

@@ -576,7 +576,7 @@ export default class EEI {
   }
 
   /**
-   * Returns true if account is empty (according to EIP-161).
+   * Returns true if account is empty or non-existent (according to EIP-161).
    * @param address - Address of account
    */
   async isAccountEmpty(address: Buffer): Promise<boolean> {

packages/vm/lib/evm/evm.ts

@@ -221,7 +221,10 @@ export default class EVM {
     await this._vm._emit('newContract', newContractEvent)
 
     toAccount = await this._state.getAccount(message.to)
-    toAccount.nonce = new BN(toAccount.nonce).addn(1).toArrayLike(Buffer)
+    // EIP-161 on account creation and CREATE execution
+    if (this._vm._common.gteHardfork('spuriousDragon')) {
+      toAccount.nonce = new BN(toAccount.nonce).addn(1).toArrayLike(Buffer)
+    }
 
     // Add tx value to the `to` account
     let errorMessage

packages/vm/lib/evm/opFns.ts

@@ -660,9 +660,9 @@ export const handlers: { [k: string]: OpHandler } = {
     if (runState.eei.isStatic() && !value.isZero()) {
       trap(ERROR.STATIC_STATE_CHANGE)
     }
-
     subMemUsage(runState, inOffset, inLength)
     subMemUsage(runState, outOffset, outLength)
+
     if (!value.isZero()) {
       runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callValueTransfer')))
     }
@@ -673,9 +673,17 @@ export const handlers: { [k: string]: OpHandler } = {
       data = runState.memory.read(inOffset.toNumber(), inLength.toNumber())
     }
 
-    const empty = await runState.eei.isAccountEmpty(toAddressBuf)
-    if (empty) {
-      if (!value.isZero()) {
+    if (runState._common.gteHardfork('spuriousDragon')) {
+      // We are at or after Spurious Dragon
+      // Call new account gas: account is DEAD and we transfer nonzero value
+      if ((await runState.stateManager.accountIsEmpty(toAddressBuf)) && !value.isZero()) {
+        runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callNewAccount')))
+      }
+    } else if (!(await runState.stateManager.accountExists(toAddressBuf))) {
+      // We are before Spurious Dragon
+      // Call new account gas: account does not exist (it is not in the state trie, not even as an "empty" account)
+      const accountDoesNotExist = !(await runState.stateManager.accountExists(toAddressBuf))
+      if (accountDoesNotExist) {
         runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callNewAccount')))
       }
     }
@@ -789,13 +797,29 @@ export const handlers: { [k: string]: OpHandler } = {
     }
 
     const selfdestructToAddressBuf = addressToBuffer(selfdestructToAddress)
-    const balance = await runState.eei.getExternalBalance(runState.eei.getAddress())
-    if (balance.gtn(0)) {
-      const empty = await runState.eei.isAccountEmpty(selfdestructToAddressBuf)
-      if (empty) {
-        runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callNewAccount')))
+    let deductGas = false
+    if (runState._common.gteHardfork('spuriousDragon')) {
+      // EIP-161: State Trie Clearing
+      const balance = await runState.eei.getExternalBalance(runState.eei.getAddress())
+      if (balance.gtn(0)) {
+        // This technically checks if account is empty or non-existent
+        // TODO: improve on the API here (EEI and StateManager)
+        const empty = await runState.eei.isAccountEmpty(selfdestructToAddressBuf)
+        if (empty) {
+          const account = await runState.stateManager.getAccount(selfdestructToAddressBuf)
+          deductGas = true
+        }
+      }
+    } else {
+      // Pre EIP-161 (Spurious Dragon) gas semantics
+      const exists = await runState.stateManager.accountExists(selfdestructToAddressBuf)
+      if (!exists) {
+        deductGas = true
       }
     }
+    if (deductGas) {
+      runState.eei.useGas(new BN(runState._common.param('gasPrices', 'callNewAccount')))
+    }
 
     return runState.eei.selfDestruct(selfdestructToAddressBuf)
   },

packages/vm/lib/index.ts

@@ -110,6 +110,7 @@ export default class VM extends AsyncEventEmitter {
       const chain = opts.chain ? opts.chain : 'mainnet'
       const hardfork = opts.hardfork ? opts.hardfork : 'petersburg'
       const supportedHardforks = [
+        'tangerineWhistle',
         'spuriousDragon',
         'byzantium',
         'constantinople',

packages/vm/lib/runTx.ts

@@ -174,17 +174,19 @@ async function _runTx(this: VM, opts: RunTxOpts): Promise<RunTxResult> {
   const minerAccount = await state.getAccount(block.header.coinbase)
   // add the amount spent on gas to the miner's account
   minerAccount.balance = toBuffer(new BN(minerAccount.balance).add(results.amountSpent))
-  if (!new BN(minerAccount.balance).isZero()) {
-    await state.putAccount(block.header.coinbase, minerAccount)
-  }
+
+  // Put the miner account into the state. If the balance of the miner account remains zero, note that
+  // the state.putAccount function puts this into the "touched" accounts. This will thus be removed when
+  // we clean the touched accounts below in case we are in a fork >= SpuriousDragon
+  await state.putAccount(block.header.coinbase, minerAccount)
 
   /*
    * Cleanup accounts
    */
   if (results.execResult.selfdestruct) {
     const keys = Object.keys(results.execResult.selfdestruct)
     for (const k of keys) {
-      await state.putAccount(Buffer.from(k, 'hex'), new Account())
+      await state.deleteAccount(Buffer.from(k, 'hex'))
     }
   }
   await state.cleanupTouchedAccounts()

packages/vm/lib/state/cache.ts

@@ -54,24 +54,30 @@ export default class Cache {
   /**
    * Looks up address in underlying trie.
    * @param address - Address of account
+   * @param create - Create emtpy account if non-existent
    */
-  async _lookupAccount(address: Buffer): Promise<Account> {
+  async _lookupAccount(address: Buffer, create: boolean = true): Promise<Account | undefined> {
     const raw = await this._trie.get(address)
-    const account = new Account(raw)
-    return account
+    if (raw || create) {
+      const account = new Account(raw)
+      return account
+    }
   }
 
   /**
    * Looks up address in cache, if not found, looks it up
    * in the underlying trie.
    * @param key - Address of account
+   * @param create - Create emtpy account if non-existent
    */
-  async getOrLoad(key: Buffer): Promise<Account> {
+  async getOrLoad(key: Buffer, create: boolean = true): Promise<Account | undefined> {
     let account = this.lookup(key)
 
     if (!account) {
-      account = await this._lookupAccount(key)
-      this._update(key, account, false, false)
+      account = await this._lookupAccount(key, create)
+      if (account) {
+        this._update(key, account as Account, false, false)
+      }
     }
 
     return account
@@ -87,7 +93,7 @@ export default class Cache {
       if (addressHex) {
         const address = Buffer.from(addressHex, 'hex')
         const account = await this._lookupAccount(address)
-        this._update(address, account, false, false)
+        this._update(address, account as Account, false, false)
       }
     }
   }

packages/vm/lib/state/interface.ts

@@ -10,7 +10,8 @@ export interface StorageDump {
 export interface StateManager {
   copy(): StateManager
   getAccount(address: Buffer): Promise<Account>
-  putAccount(address: Buffer, account: Account): Promise<void>
+  putAccount(address: Buffer, account: Account | null): Promise<void>
+  deleteAccount(address: Buffer): Promise<void>
   touchAccount(address: Buffer): void
   putContractCode(address: Buffer, value: Buffer): Promise<void>
   getContractCode(address: Buffer): Promise<Buffer>
@@ -28,5 +29,6 @@ export interface StateManager {
   generateCanonicalGenesis(): Promise<void>
   generateGenesis(initState: any): Promise<void>
   accountIsEmpty(address: Buffer): Promise<boolean>
+  accountExists(address: Buffer): Promise<boolean>
   cleanupTouchedAccounts(): Promise<void>
 }

packages/vm/lib/state/stateManager.ts

@@ -71,7 +71,7 @@ export default class DefaultStateManager implements StateManager {
    * @param address - Address of the `account` to get
    */
   async getAccount(address: Buffer): Promise<Account> {
-    const account = await this._cache.getOrLoad(address)
+    const account = (await this._cache.getOrLoad(address)) as Account
     return account
   }
 
@@ -87,7 +87,11 @@ export default class DefaultStateManager implements StateManager {
     // if they have money or a non-zero nonce or code, then write to tree
     this._cache.put(address, account)
     this.touchAccount(address)
-    // this._trie.put(addressHex, account.serialize(), cb)
+  }
+
+  async deleteAccount(address: Buffer) {
+    this._cache.del(address)
+    this.touchAccount(address)
   }
 
   /**
@@ -111,7 +115,7 @@ export default class DefaultStateManager implements StateManager {
     const codeHash = keccak256(value)
 
     if (codeHash.equals(KECCAK256_NULL)) {
-      return
+      //return
     }
 
     const account = await this.getAccount(address)
@@ -468,7 +472,8 @@ export default class DefaultStateManager implements StateManager {
   }
 
   /**
-   * Checks if the `account` corresponding to `address` is empty as defined in
+   * Checks if the `account` corresponding to `address`
+   * is empty or non-existent as defined in
    * EIP-161 (https://eips.ethereum.org/EIPS/eip-161).
    * @param address - Address to check
    */
@@ -477,17 +482,35 @@ export default class DefaultStateManager implements StateManager {
     return account.isEmpty()
   }
 
+  /**
+   * Checks if the `account` corresponding to `address`
+   * exists
+   * @param address - Address of the `account` to check
+   */
+  async accountExists(address: Buffer): Promise<boolean> {
+    const account = await this._cache.lookup(address)
+    if (account) {
+      return true
+    }
+    if (await this._cache._trie.get(address)) {
+      return true
+    }
+    return false
+  }
+
   /**
    * Removes accounts form the state trie that have been touched,
    * as defined in EIP-161 (https://eips.ethereum.org/EIPS/eip-161).
    */
   async cleanupTouchedAccounts(): Promise<void> {
-    const touchedArray = Array.from(this._touched)
-    for (const addressHex of touchedArray) {
-      const address = Buffer.from(addressHex, 'hex')
-      const empty = await this.accountIsEmpty(address)
-      if (empty) {
-        this._cache.del(address)
+    if (this._common.gteHardfork('spuriousDragon')) {
+      const touchedArray = Array.from(this._touched)
+      for (const addressHex of touchedArray) {
+        const address = Buffer.from(addressHex, 'hex')
+        const empty = await this.accountIsEmpty(address)
+        if (empty) {
+          this._cache.del(address)
+        }
       }
     }
     this._touched.clear()

packages/vm/package.json

@@ -14,8 +14,8 @@
     "coverage:test": "npm run build && tape './tests/api/**/*.js' ./tests/tester.js --state --dist",
     "docs:build": "typedoc --options typedoc.js",
     "test:state": "ts-node ./tests/tester --state",
-    "test:state:allForks": "npm run test:state -- --fork=SpuriousDragon && npm run test:state -- --fork=Byzantium && npm run test:state -- --fork=Constantinople && npm run test:state -- --fork=Petersburg && npm run test:state -- --fork=Istanbul && npm run test:state -- --fork=MuirGlacier",
-    "test:state:selectedForks": "npm run test:state -- --fork=Petersburg && npm run test:state -- --fork=SpuriousDragon",
+    "test:state:allForks": "npm run test:state -- --fork=TangerineWhistle && npm run test:state -- --fork=SpuriousDragon && npm run test:state -- --fork=Byzantium && npm run test:state -- --fork=Constantinople && npm run test:state -- --fork=Petersburg && npm run test:state -- --fork=Istanbul && npm run test:state -- --fork=MuirGlacier",
+    "test:state:selectedForks": "npm run test:state -- --fork=Petersburg && npm run test:state -- --fork=TangerineWhistle",
     "test:state:slow": "npm run test:state -- --runSkipped=slow",
     "test:buildIntegrity": "npm run test:state -- --test='stackOverflow'",
     "test:blockchain": "node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain",

packages/vm/tests/api/state/stateManager.js

@@ -113,6 +113,37 @@ tape('StateManager', (t) => {
     },
   )
 
+  t.test(
+    'should return false for a non-existent account',
+    async (st) => {
+      const stateManager = new DefaultStateManager()
+      const address = Buffer.from('a94f5374fce5edbc8e2a8697c15331677e6ebf0b', 'hex')
+
+      let res = await stateManager.accountExists(address)
+
+      st.notOk(res)
+
+      st.end()
+    },
+  )
+
+  t.test(
+    'should return true for an existent account',
+    async (st) => {
+      const stateManager = new DefaultStateManager()
+      const account = createAccount('0x1', '0x1')
+      const address = Buffer.from('a94f5374fce5edbc8e2a8697c15331677e6ebf0b', 'hex')
+
+      await stateManager.putAccount(address, account)
+
+      let res = await stateManager.accountExists(address)
+
+      st.ok(res)
+
+      st.end()
+    },
+  )
+
   t.test(
     'should call the callback with a false boolean representing non-emptiness when the account is not empty',
     async (st) => {