Add EIP-1559 transaction support

Creating an unfrozen transaction (added in MetaMask#88) seems to be a change
that was only required in eth-ledger-keyring, not in Trezor,
and is fixed by @ethereumjs/tx: v3.1.4 anyway.

I've removed this part,
since it was causing issues with EIP-1559 transactions, and does
not seem necessary for non-EIP-1559 transactions either.

index.js

@@ -18,6 +18,28 @@ function wait(ms) {
   return new Promise((resolve) => setTimeout(resolve, ms));
 }
 
+/**
+ * @typedef {import('@ethereumjs/tx').TypedTransaction} TypedTransaction
+ * @typedef {InstanceType<import("ethereumjs-tx")>} OldEthJsTransaction
+ */
+
+/**
+ * Check if the given transaction is made with ethereumjs-tx or @ethereumjs/tx
+ *
+ * Transactions built with older versions of ethereumjs-tx have a
+ * getChainId method that newer versions do not.
+ * Older versions are mutable
+ * while newer versions default to being immutable.
+ * Expected shape and type
+ * of data for v, r and s differ (Buffer (old) vs BN (new)).
+ *
+ * @param {TypedTransaction | OldEthJsTransaction} tx
+ * @returns {tx is OldEthJsTransaction} Returns `true` if tx is an old-style ethereumjs-tx transaction.
+ */
+function isOldStyleEthereumjsTx(tx) {
+  return typeof tx.getChainId === 'function';
+}
+
 class TrezorKeyring extends EventEmitter {
   constructor(opts = {}) {
     super();
@@ -169,13 +191,20 @@ class TrezorKeyring extends EventEmitter {
     );
   }
 
-  // tx is an instance of the ethereumjs-transaction class.
+  /**
+   * Signs a transaction using Trezor.
+   *
+   * Accepts either an ethereumjs-tx or @ethereumjs/tx transaction, and returns
+   * the same type.
+   *
+   * @template {TypedTransaction | OldEthJsTransaction} Transaction
+   * @param {string} address - Hex string address.
+   * @param {Transaction} tx - Instance of either new-style or old-style ethereumjs transaction.
+   * @returns {Promise<Transaction>} The signed transaction, an instance of either new-style or old-style
+   * ethereumjs transaction.
+   */
   signTransaction(address, tx) {
-    // transactions built with older versions of ethereumjs-tx have a
-    // getChainId method that newer versions do not. Older versions are mutable
-    // while newer versions default to being immutable. Expected shape and type
-    // of data for v, r and s differ (Buffer (old) vs BN (new))
-    if (typeof tx.getChainId === 'function') {
+    if (isOldStyleEthereumjsTx(tx)) {
       // In this version of ethereumjs-tx we must add the chainId in hex format
       // to the initial v value. The chainId must be included in the serialized
       // transaction which is only communicated to ethereumjs-tx in this
@@ -188,32 +217,17 @@ class TrezorKeyring extends EventEmitter {
         return tx;
       });
     }
-    // For transactions created by newer versions of @ethereumjs/tx
-    // Note: https://github.com/ethereumjs/ethereumjs-monorepo/issues/1188
-    // It is not strictly necessary to do this additional setting of the v
-    // value. We should be able to get the correct v value in serialization
-    // if the above issue is resolved. Until then this must be set before
-    // calling .serialize(). Note we are creating a temporarily mutable object
-    // forfeiting the benefit of immutability until this happens. We do still
-    // return a Transaction that is frozen if the originally provided
-    // transaction was also frozen.
-    const unfrozenTx = TransactionFactory.fromTxData(tx.toJSON(), {
-      common: tx.common,
-      freeze: false,
-    });
-    unfrozenTx.v = new ethUtil.BN(
-      ethUtil.addHexPrefix(tx.common.chainId()),
-      'hex',
-    );
     return this._signTransaction(
       address,
       tx.common.chainIdBN().toNumber(),
-      unfrozenTx,
+      tx,
       (payload) => {
         // Because tx will be immutable, first get a plain javascript object that
         // represents the transaction. Using txData here as it aligns with the
         // nomenclature of ethereumjs/tx.
         const txData = tx.toJSON();
+        // The fromTxData utility expects a type to support transactions with a type other than 0
+        txData.type = tx.type;
         // The fromTxData utility expects v,r and s to be hex prefixed
         txData.v = ethUtil.addHexPrefix(payload.v);
         txData.r = ethUtil.addHexPrefix(payload.r);
@@ -228,22 +242,42 @@ class TrezorKeyring extends EventEmitter {
     );
   }
 
-  // tx is an instance of the ethereumjs-transaction class.
+  /**
+   * 
+   * @template {TypedTransaction | OldEthJsTransaction} Transaction
+   * @param {string} address - Hex string address.
+   * @param {number} chainId - Chain ID
+   * @param {Transaction} tx - Instance of either new-style or old-style ethereumjs transaction.
+   * @param {(import('trezor-connect').EthereumSignedTx) => Transaction} handleSigning - Converts signed transaction
+   * to the same new-style or old-style ethereumjs-tx.
+   * @returns {Promise<Transaction>} The signed transaction, an instance of either new-style or old-style
+   * ethereumjs transaction.
+   */
   async _signTransaction(address, chainId, tx, handleSigning) {
+    let transaction;
+    if (isOldStyleEthereumjsTx(tx)) {
+      // legacy transaction from ethereumjs-tx package has no .toJSON() function,
+      // so we need to convert to hex-strings manually manually
+      transaction = {
+        to: this._normalize(tx.to),
+        value: this._normalize(tx.value),
+        data: this._normalize(tx.data),
+        chainId,
+        nonce: this._normalize(tx.nonce),
+        gasLimit: this._normalize(tx.gasLimit),
+        gasPrice: this._normalize(tx.gasPrice),
+      }; 
+    } else {
+      // new-style transaction from @ethereumjs/tx package
+      // we can just copy tx.toJSON() for everything except chainId, which must be a number
+      transaction = {...tx.toJSON(), chainId};
+    }
     try {
       const status = await this.unlock();
       await wait(status === 'just unlocked' ? DELAY_BETWEEN_POPUPS : 0);
       const response = await TrezorConnect.ethereumSignTransaction({
         path: this._pathFromAddress(address),
-        transaction: {
-          to: this._normalize(tx.to),
-          value: this._normalize(tx.value),
-          data: this._normalize(tx.data),
-          chainId,
-          nonce: this._normalize(tx.nonce),
-          gasLimit: this._normalize(tx.gasLimit),
-          gasPrice: this._normalize(tx.gasPrice),
-        },
+        transaction,
       });
       if (response.success) {
         const newOrMutatedTx = handleSigning(response.payload);

test/test-eth-trezor-keyring.js

@@ -429,7 +429,13 @@ describe('TrezorKeyring', function () {
         return tx;
       });
 
-      sinon.stub(TrezorConnect, 'ethereumSignTransaction').callsFake(() => {
+      sinon.stub(TrezorConnect, 'ethereumSignTransaction').callsFake((params) => {
+        expect(params.transaction).to.be.an("object");
+        // chainId must be a number, unlike other variables which can be hex-strings
+        expect(params.transaction).to.have.property("chainId").to.satisfy(Number.isInteger);
+        expect(params.transaction).to.have.property("maxFeePerGas");
+        expect(params.transaction).to.have.property("maxPriorityFeePerGas");
+        expect(params.transaction).to.not.have.property("gasPrice");
         return Promise.resolve({
           success: true,
           payload: expectedRSV,