Merge pull request #1042 from ethereumjs/tx-fix-isSigned

Fix isSigned returning true for unsigned txs
ethereumjs · Jan 13, 2021 · 83e591a · 83e591a
2 parents 5953e09 + 7a804e8
commit 83e591a
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 8 deletions.
diff --git a/packages/tx/src/transaction.ts b/packages/tx/src/transaction.ts
@@ -59,17 +59,19 @@ export default class Transaction {
 
     const [nonce, gasPrice, gasLimit, to, value, data, v, r, s] = values
 
+    const emptyBuffer = Buffer.from([])
+
     return new Transaction(
       {
         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,
+        data: data ?? emptyBuffer,
+        v: !v?.equals(emptyBuffer) ? new BN(v) : undefined,
+        r: !r?.equals(emptyBuffer) ? new BN(r) : undefined,
+        s: !s?.equals(emptyBuffer) ? new BN(s) : undefined,
       },
       opts
     )
@@ -89,9 +91,9 @@ export default class Transaction {
     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))
+    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,

diff --git a/packages/tx/test/api.ts b/packages/tx/test/api.ts
@@ -335,6 +335,43 @@ tape('[Transaction]: Basic functions', function (t) {
     st.end()
   })
 
+  t.test('returns correct values for isSigned', function (st) {
+    let tx = Transaction.fromTxData({})
+    st.notOk(tx.isSigned())
+
+    const txData: TxData = {
+      data: '0x7cf5dab00000000000000000000000000000000000000000000000000000000000000005',
+      gasLimit: '0x15f90',
+      gasPrice: '0x1',
+      nonce: '0x01',
+      to: '0xd9024df085d09398ec76fbed18cac0e1149f50dc',
+      value: '0x0',
+    }
+    const privateKey = Buffer.from(
+      '4646464646464646464646464646464646464646464646464646464646464646',
+      'hex'
+    )
+    tx = Transaction.fromTxData(txData)
+    st.notOk(tx.isSigned())
+    tx = tx.sign(privateKey)
+    st.ok(tx.isSigned())
+
+    tx = new Transaction(txData)
+    st.notOk(tx.isSigned())
+    const rawUnsigned = tx.serialize()
+    tx = tx.sign(privateKey)
+    const rawSigned = tx.serialize()
+    st.ok(tx.isSigned())
+
+    tx = Transaction.fromRlpSerializedTx(rawUnsigned)
+    st.notOk(tx.isSigned())
+    tx = tx.sign(privateKey)
+    st.ok(tx.isSigned())
+    tx = Transaction.fromRlpSerializedTx(rawSigned)
+    st.ok(tx.isSigned())
+    st.end()
+  })
+
   t.test(
     'throws when creating a a transaction with incompatible chainid and v value',
     function (st) {

diff --git a/packages/vm/tests/api/runTx.spec.ts b/packages/vm/tests/api/runTx.spec.ts
@@ -33,7 +33,7 @@ tape('runTx', (t) => {
   t.test('should fail to run without signature', async (st) => {
     const tx = getTransaction(false)
     shouldFail(st, suite.runTx({ tx }), (e: Error) =>
-      st.ok(e.message.includes('Invalid Signature'), 'should fail with appropriate error')
+      st.ok(e.message.includes('not signed'), 'should fail with appropriate error')
     )
     st.end()
   })