Tx: Throw when hash() is called on unsigned legacy transaction (#1894)

* tx: edit legacyTransaction to throw when hash() called on unsigned tx

* tx: adapt legacy.spec test to throw on unsigned tx.hash()

* tx: edit tests which call hash() on unsigned tx

Author: ScottyPoi

packages/tx/src/legacyTransaction.ts

@@ -258,20 +258,10 @@ export default class Transaction extends BaseTransaction<Transaction> {
    * Use {@link Transaction.getMessageToSign} to get a tx hash for the purpose of signing.
    */
   hash(): Buffer {
-    // In contrast to the tx type transaction implementations the `hash()` function
-    // for the legacy tx does not throw if the tx is not signed.
-    // This has been considered for inclusion but lead to unexpected backwards
-    // compatibility problems (no concrete reference found, needs validation).
-    //
-    // For context see also https://github.com/ethereumjs/ethereumjs-monorepo/pull/1445,
-    // September, 2021 as well as work done before.
-    //
-    // This should be updated along the next major version release by adding:
-    //
-    //if (!this.isSigned()) {
-    //  const msg = this._errorMsg('Cannot call hash method if transaction is not signed')
-    //  throw new Error(msg)
-    //}
+    if (!this.isSigned()) {
+      const msg = this._errorMsg('Cannot call hash method if transaction is not signed')
+      throw new Error(msg)
+    }
 
     if (Object.isFrozen(this)) {
       if (!this.cache.hash) {

packages/tx/test/inputValue.spec.ts

@@ -1,12 +1,5 @@
 import tape from 'tape'
-import {
-  Address,
-  AddressLike,
-  BigIntLike,
-  BufferLike,
-  bufferToHex,
-  toBuffer,
-} from 'ethereumjs-util'
+import { Address, AddressLike, BigIntLike, BufferLike, toBuffer } from 'ethereumjs-util'
 import Common, { Chain, Hardfork } from '@ethereumjs/common'
 import { Transaction } from '../src'
 
@@ -112,12 +105,10 @@ tape('[Transaction Input Values]', function (t) {
     const legacyTxData = generateCombinations({
       options,
     })
-    const expectedHash = Transaction.fromTxData(legacyTxData[0]).hash()
     const randomSample = getRandomSubarray(legacyTxData, 100)
     for (const txData of randomSample) {
       const tx = Transaction.fromTxData(txData, { common })
-      const hash = tx.hash()
-      st.deepEqual(hash, expectedHash, `correct tx hash (0x${bufferToHex(hash)})`)
+      t.throws(() => tx.hash(), 'tx.hash() throws if tx is unsigned')
     }
     st.end()
   })
@@ -133,14 +124,11 @@ tape('[Transaction Input Values]', function (t) {
     const eip1559TxData = generateCombinations({
       options,
     })
-    const expectedHash = Transaction.fromTxData(eip1559TxData[0]).hash()
     const randomSample = getRandomSubarray(eip1559TxData, 100)
 
     for (const txData of randomSample) {
       const tx = Transaction.fromTxData(txData, { common })
-      const hash = tx.hash()
-
-      st.deepEqual(hash, expectedHash, `correct tx hash (0x${bufferToHex(hash)})`)
+      t.throws(() => tx.hash(), 'tx.hash() should throw if unsigned')
     }
     st.end()
   })

packages/tx/test/legacy.spec.ts

@@ -230,21 +230,13 @@ tape('[Transaction]', function (t) {
       chain: Chain.Mainnet,
       hardfork: Hardfork.TangerineWhistle,
     })
-    // Test currently commented out, see comment on legacy tx hash() function
-    /*let tx = Transaction.fromValuesArray(txFixtures[3].raw.slice(0, 6).map(toBuffer), {
+
+    let tx = Transaction.fromValuesArray(txFixtures[3].raw.slice(0, 6).map(toBuffer), {
       common,
     })
     st.throws(() => {
       tx.hash()
-    }, 'should throw calling hash with unsigned tx')*/
-    let tx = Transaction.fromValuesArray(txFixtures[3].raw.map(toBuffer), {
-      common,
-      freeze: false,
-    })
-    st.deepEqual(
-      tx.hash(),
-      Buffer.from('375a8983c9fc56d7cfd118254a80a8d7403d590a6c9e105532b67aca1efb97aa', 'hex')
-    )
+    }, 'should throw calling hash with unsigned tx')
     tx = Transaction.fromValuesArray(txFixtures[3].raw.map(toBuffer), {
       common,
     })