Merge pull request #944 from ethereumjs/tx-constructor-improvements

Tx Constructor Restructuring

packages/block/.eslintrc.js

@@ -1,4 +1,7 @@
 module.exports = {
   extends: "@ethereumjs/eslint-config-defaults",
-  ignorePatterns: ["test-build", "karma.conf.js"]
+  ignorePatterns: ["test-build", "karma.conf.js"],
+  rules: {
+    "@typescript-eslint/no-unnecessary-condition": "off"
+  }
 }

packages/blockchain/.eslintrc.js

@@ -1,3 +1,6 @@
 module.exports = {
-  extends: "@ethereumjs/eslint-config-defaults"
+  extends: "@ethereumjs/eslint-config-defaults",
+  rules: {
+    "@typescript-eslint/no-unnecessary-condition": "off"
+  }
 }

packages/common/.eslintrc.js

@@ -1,4 +1,7 @@
 module.exports = {
   extends: "@ethereumjs/eslint-config-defaults",
-  ignorePatterns: ['scripts'],
+  ignorePatterns: ["scripts"],
+  rules: {
+    "@typescript-eslint/no-unnecessary-condition": "off"
+  }
 }

packages/ethash/.eslintrc.js

@@ -1,4 +1,7 @@
 module.exports = {
   extends: "@ethereumjs/eslint-config-defaults",
-  ignorePatterns: ['examples']
+  ignorePatterns: ["examples"],
+  rules: {
+    "@typescript-eslint/no-unnecessary-condition": "off"
+  }
 }

packages/tx/.eslintrc.js

@@ -1,4 +1,7 @@
 module.exports = {
   extends: "@ethereumjs/eslint-config-defaults",
-  ignorePatterns: ["examples", "karma.conf.js", "test-build"]
+  ignorePatterns: ["examples", "karma.conf.js", "test-build"],
+  rules: {
+    "@typescript-eslint/no-unnecessary-condition": "off"
+  }
 }

packages/tx/CHANGELOG.md

@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 ## 3.0.0-beta.2 - UNRELEASED
 
 - Added `freeze` option to allow for transaction freeze deactivation (e.g. to allow for subclassing tx and adding additional parameters), see PR [#941](https://github.com/ethereumjs/ethereumjs-vm/pull/941)
+- **Breaking:** Reworked constructor to take in data as a `TxData` typed dictionary instead of single values, the `Tx.fromTxData()` factory method becomes an alias for the constructor with this change, see PR [#944](https://github.com/ethereumjs/ethereumjs-vm/pull/944)
 
 ## 3.0.0-beta.1 - 2020-10-22
 

packages/tx/src/transaction.ts

@@ -37,20 +37,7 @@ export default class Transaction {
   public readonly s?: BN
 
   public static fromTxData(txData: TxData, opts?: TxOptions) {
-    const { nonce, gasLimit, gasPrice, to, value, data, v, r, s } = txData
-
-    return new Transaction(
-      new BN(toBuffer(nonce)),
-      new BN(toBuffer(gasPrice)),
-      new BN(toBuffer(gasLimit)),
-      to ? new Address(toBuffer(to)) : undefined,
-      new BN(toBuffer(value)),
-      toBuffer(data),
-      new BN(toBuffer(v)),
-      new BN(toBuffer(r)),
-      new BN(toBuffer(s)),
-      opts
-    )
+    return new Transaction(txData, opts)
   }
 
   public static fromRlpSerializedTx(serialized: Buffer, opts?: TxOptions) {
@@ -73,15 +60,17 @@ export default class Transaction {
     const [nonce, gasPrice, gasLimit, to, value, data, v, r, s] = values
 
     return new Transaction(
-      new BN(nonce),
-      new BN(gasPrice),
-      new BN(gasLimit),
-      to && to.length > 0 ? new Address(to) : undefined,
-      new BN(value),
-      data || Buffer.from([]),
-      v ? new BN(v) : undefined,
-      r ? new BN(r) : undefined,
-      s ? new BN(s) : undefined,
+      {
+        nonce: new BN(nonce),
+        gasPrice: new BN(gasPrice),
+        gasLimit: new BN(gasLimit),
+        to: to && to.length > 0 ? new Address(to) : undefined,
+        value: new BN(value),
+        data: data || Buffer.from([]),
+        v: v ? new BN(v) : undefined,
+        r: r ? new BN(r) : undefined,
+        s: s ? new BN(s) : undefined,
+      },
       opts
     )
   }
@@ -91,25 +80,26 @@ export default class Transaction {
    * Use the static factory methods to assist in creating a Transaction object from varying data types.
    * @note Transaction objects implement EIP155 by default. To disable it, pass in an `@ethereumjs/common` object set before EIP155 activation (i.e. before Spurious Dragon).
    */
-  constructor(
-    nonce: BN,
-    gasPrice: BN,
-    gasLimit: BN,
-    to: Address | undefined,
-    value: BN,
-    data: Buffer,
-    v?: BN,
-    r?: BN,
-    s?: BN,
-    opts?: TxOptions
-  ) {
+  constructor(txData: TxData, opts?: TxOptions) {
+    const { nonce, gasPrice, gasLimit, to, value, data, v, r, s } = txData
+
+    this.nonce = new BN(toBuffer(nonce))
+    this.gasPrice = new BN(toBuffer(gasPrice))
+    this.gasLimit = new BN(toBuffer(gasLimit))
+    this.to = to ? new Address(toBuffer(to)) : undefined
+    this.value = new BN(toBuffer(value))
+    this.data = toBuffer(data)
+    this.v = new BN(toBuffer(v))
+    this.r = new BN(toBuffer(r))
+    this.s = new BN(toBuffer(s))
+
     const validateCannotExceedMaxInteger = {
-      nonce,
-      gasPrice,
-      gasLimit,
-      value,
-      r,
-      s,
+      nonce: this.nonce,
+      gasPrice: this.gasPrice,
+      gasLimit: this.gasLimit,
+      value: this.value,
+      r: this.r,
+      s: this.s,
     }
     for (const [key, value] of Object.entries(validateCannotExceedMaxInteger)) {
       if (value && value.gt(MAX_INTEGER)) {
@@ -124,17 +114,7 @@ export default class Transaction {
       this.common = new Common({ chain: DEFAULT_CHAIN })
     }
 
-    this._validateTxV(v)
-
-    this.nonce = nonce
-    this.gasPrice = gasPrice
-    this.gasLimit = gasLimit
-    this.to = to
-    this.value = value
-    this.data = data
-    this.v = v
-    this.r = r
-    this.s = s
+    this._validateTxV(this.v)
 
     const freeze = opts?.freeze ?? true
     if (freeze) {
@@ -264,15 +244,17 @@ export default class Transaction {
     }
 
     return new Transaction(
-      this.nonce,
-      this.gasPrice,
-      this.gasLimit,
-      this.to,
-      this.value,
-      this.data,
-      new BN(v),
-      new BN(r),
-      new BN(s),
+      {
+        nonce: this.nonce,
+        gasPrice: this.gasPrice,
+        gasLimit: this.gasLimit,
+        to: this.to,
+        value: this.value,
+        data: this.data,
+        v: new BN(v),
+        r: new BN(r),
+        s: new BN(s),
+      },
       opts
     )
   }

packages/vm/.eslintrc.js

@@ -2,9 +2,10 @@ module.exports = {
   extends: "@ethereumjs/eslint-config-defaults",
   ignorePatterns: ["scripts", "examples", "karma.conf.js"],
   rules: {
-    '@typescript-eslint/no-use-before-define': 'off',
-    'no-invalid-this': 'off',
-    'no-restricted-syntax': 'off',
-    'no-console': 'off' // supress the console.log warnings in ./tests/
+    "@typescript-eslint/no-use-before-define": "off",
+    "@typescript-eslint/no-unnecessary-condition": "off",
+    "no-invalid-this": "off",
+    "no-restricted-syntax": "off",
+    "no-console": "off" // supress the console.log warnings in ./tests/
   }
 }

packages/vm/tests/api/runBlock.spec.ts

@@ -97,7 +97,10 @@ tape('should fail when tx gas limit higher than block gas limit', async (t) => {
 
   const gasLimit = new BN(Buffer.from('3fefba', 'hex'))
   const opts = { common: block._common }
-  block.transactions[0] = new Transaction(nonce, gasPrice, gasLimit, to, value, data, v, r, s, opts)
+  block.transactions[0] = new Transaction(
+    { nonce, gasPrice, gasLimit, to, value, data, v, r, s },
+    opts
+  )
 
   await suite.p
     .runBlock({ block, skipBlockValidation: true })