Merge pull request #1141 from ethereumjs/eip2718-eip2930-improvements

EIP-2718/EIP-2930 Typed Tx Improvements and Tests

Author: jochem-brouwer

packages/tx/src/baseTransaction.ts

@@ -8,7 +8,7 @@ import {
   ecsign,
   publicToAddress,
 } from 'ethereumjs-util'
-import { BaseTransactionData, BaseTxOptions, DEFAULT_COMMON, JsonTx } from './types'
+import { TxData, TxOptions, JsonTx } from './types'
 
 export abstract class BaseTransaction<TransactionObject> {
   public readonly nonce: BN
@@ -19,8 +19,12 @@ export abstract class BaseTransaction<TransactionObject> {
   public readonly data: Buffer
   public readonly common: Common
 
-  constructor(txData: BaseTransactionData, txOptions: BaseTxOptions = {}) {
-    const { nonce, gasLimit, gasPrice, to, value, data } = txData
+  public readonly v?: BN
+  public readonly r?: BN
+  public readonly s?: BN
+
+  constructor(txData: TxData, txOptions: TxOptions = {}) {
+    const { nonce, gasLimit, gasPrice, to, value, data, v, r, s } = txData
 
     this.nonce = new BN(toBuffer(nonce))
     this.gasPrice = new BN(toBuffer(gasPrice))
@@ -29,50 +33,62 @@ export abstract class BaseTransaction<TransactionObject> {
     this.value = new BN(toBuffer(value))
     this.data = toBuffer(data)
 
+    this.v = v ? new BN(toBuffer(v)) : undefined
+    this.r = r ? new BN(toBuffer(r)) : undefined
+    this.s = s ? new BN(toBuffer(s)) : undefined
+
     const validateCannotExceedMaxInteger = {
       nonce: this.nonce,
       gasPrice: this.gasPrice,
       gasLimit: this.gasLimit,
       value: this.value,
     }
 
-    this.validateExceedsMaxInteger(validateCannotExceedMaxInteger)
+    this._validateExceedsMaxInteger(validateCannotExceedMaxInteger)
 
     this.common =
       (txOptions.common &&
         Object.assign(Object.create(Object.getPrototypeOf(txOptions.common)), txOptions.common)) ??
-      DEFAULT_COMMON
-  }
-
-  protected validateExceedsMaxInteger(validateCannotExceedMaxInteger: { [key: string]: BN }) {
-    for (const [key, value] of Object.entries(validateCannotExceedMaxInteger)) {
-      if (value && value.gt(MAX_INTEGER)) {
-        throw new Error(`${key} cannot exceed MAX_INTEGER, given ${value}`)
-      }
-    }
+      new Common({ chain: 'mainnet' })
   }
 
   /**
-   * If the tx's `to` is to the creation address
+   * Checks if the transaction has the minimum amount of gas required
+   * (DataFee + TxFee + Creation Fee).
    */
-  toCreationAddress(): boolean {
-    return this.to === undefined || this.to.buf.length === 0
-  }
-
   /**
-   * Computes a sha3-256 hash of the serialized unsigned tx, which is used to sign the transaction.
+   * Checks if the transaction has the minimum amount of gas required
+   * (DataFee + TxFee + Creation Fee).
    */
-  rawTxHash(): Buffer {
-    return this.getMessageToSign()
-  }
+  validate(): boolean
+  /* eslint-disable-next-line no-dupe-class-members */
+  validate(stringError: false): boolean
+  /* eslint-disable-next-line no-dupe-class-members */
+  validate(stringError: true): string[]
+  /* eslint-disable-next-line no-dupe-class-members */
+  validate(stringError: boolean = false): boolean | string[] {
+    const errors = []
 
-  abstract getMessageToSign(): Buffer
+    if (this.getBaseFee().gt(this.gasLimit)) {
+      errors.push(`gasLimit is too low. given ${this.gasLimit}, need at least ${this.getBaseFee()}`)
+    }
+
+    if (this.isSigned() && !this.verifySignature()) {
+      errors.push('Invalid Signature')
+    }
+
+    return stringError ? errors : errors.length === 0
+  }
 
   /**
-   * Returns chain ID
+   * The minimum amount of gas the tx must have (DataFee + TxFee + Creation Fee)
    */
-  getChainId(): number {
-    return this.common.chainId()
+  getBaseFee(): BN {
+    const fee = this.getDataFee().addn(this.common.param('gasPrices', 'tx'))
+    if (this.common.gteHardfork('homestead') && this.toCreationAddress()) {
+      fee.iaddn(this.common.param('gasPrices', 'txCreation'))
+    }
+    return fee
   }
 
   /**
@@ -89,17 +105,6 @@ export abstract class BaseTransaction<TransactionObject> {
     return new BN(cost)
   }
 
-  /**
-   * The minimum amount of gas the tx must have (DataFee + TxFee + Creation Fee)
-   */
-  getBaseFee(): BN {
-    const fee = this.getDataFee().addn(this.common.param('gasPrices', 'tx'))
-    if (this.common.gteHardfork('homestead') && this.toCreationAddress()) {
-      fee.iaddn(this.common.param('gasPrices', 'txCreation'))
-    }
-    return fee
-  }
-
   /**
    * The up front amount that an account must have for this transaction to be valid
    */
@@ -108,44 +113,45 @@ export abstract class BaseTransaction<TransactionObject> {
   }
 
   /**
-   * Checks if the transaction has the minimum amount of gas required
-   * (DataFee + TxFee + Creation Fee).
+   * If the tx's `to` is to the creation address
    */
+  toCreationAddress(): boolean {
+    return this.to === undefined || this.to.buf.length === 0
+  }
+
   /**
-   * Checks if the transaction has the minimum amount of gas required
-   * (DataFee + TxFee + Creation Fee).
+   * Returns the raw `Buffer[]` (Transaction) or `Buffer` (typed transaction).
+   * This is the data which is found in the transactions of the block body.
+   *
+   * Note that if you want to use this function in a tx type independent way
+   * to then use the raw data output for tx instantiation with
+   * `Tx.fromValuesArray()` you should set the `asList` parameter to `true` -
+   * which is ignored on a legacy tx but provides the correct format on
+   * a typed tx.
+   *
+   * To prepare a tx to be added as block data with `Block.fromValuesArray()`
+   * just use the plain `raw()` method.
    */
-  validate(): boolean
-  /* eslint-disable-next-line no-dupe-class-members */
-  validate(stringError: false): boolean
-  /* eslint-disable-next-line no-dupe-class-members */
-  validate(stringError: true): string[]
-  /* eslint-disable-next-line no-dupe-class-members */
-  validate(stringError: boolean = false): boolean | string[] {
-    const errors = []
-
-    if (this.getBaseFee().gt(this.gasLimit)) {
-      errors.push(`gasLimit is too low. given ${this.gasLimit}, need at least ${this.getBaseFee()}`)
-    }
-
-    if (this.isSigned() && !this.verifySignature()) {
-      errors.push('Invalid Signature')
-    }
-
-    return stringError ? errors : errors.length === 0
-  }
+  abstract raw(asList: boolean): Buffer[] | Buffer
 
   /**
    * Returns the encoding of the transaction.
    */
   abstract serialize(): Buffer
 
   /**
-   * Returns an object with the JSON representation of the transaction
+   * Computes a sha3-256 hash of the serialized unsigned tx, which is used to sign the transaction.
    */
-  abstract toJSON(): JsonTx
+  abstract getMessageToSign(): Buffer
+
+  abstract hash(): Buffer
+
+  abstract getMessageToVerifySignature(): Buffer
 
-  abstract isSigned(): boolean
+  public isSigned(): boolean {
+    const { v, r, s } = this
+    return !!v && !!r && !!s
+  }
 
   /**
    * Determines if the signature is valid
@@ -160,22 +166,21 @@ export abstract class BaseTransaction<TransactionObject> {
     }
   }
 
-  /**
-   * Returns the raw `Buffer[]` (Transaction) or `Buffer` (typed transaction).
-   * This is the data which is found in the transactions of the block body.
-   */
-  abstract raw(): Buffer[] | Buffer
-  abstract hash(): Buffer
-
-  abstract getMessageToVerifySignature(): Buffer
   /**
    * Returns the sender's address
    */
   getSenderAddress(): Address {
     return new Address(publicToAddress(this.getSenderPublicKey()))
   }
+
+  /**
+   * Returns the public key of the sender
+   */
   abstract getSenderPublicKey(): Buffer
 
+  /**
+   * Signs a tx and returns a new signed tx object
+   */
   sign(privateKey: Buffer): TransactionObject {
     if (privateKey.length !== 32) {
       throw new Error('Private key must be 32 bytes in length.')
@@ -187,9 +192,22 @@ export abstract class BaseTransaction<TransactionObject> {
     /* eslint-disable-next-line prefer-const */
     let { v, r, s } = ecsign(msgHash, privateKey)
 
-    return this.processSignature(v, r, s)
+    return this._processSignature(v, r, s)
   }
 
+  /**
+   * Returns an object with the JSON representation of the transaction
+   */
+  abstract toJSON(): JsonTx
+
   // Accept the v,r,s values from the `sign` method, and convert this into a TransactionObject
-  protected abstract processSignature(v: number, r: Buffer, s: Buffer): TransactionObject
+  protected abstract _processSignature(v: number, r: Buffer, s: Buffer): TransactionObject
+
+  protected _validateExceedsMaxInteger(validateCannotExceedMaxInteger: { [key: string]: BN }) {
+    for (const [key, value] of Object.entries(validateCannotExceedMaxInteger)) {
+      if (value && value.gt(MAX_INTEGER)) {
+        throw new Error(`${key} cannot exceed MAX_INTEGER, given ${value}`)
+      }
+    }
+  }
 }