Skip to content

Commit

Permalink
Revert "Safe property access / allow transaction with undefined to fi…
Browse files Browse the repository at this point in the history
…elds, in signTransaction"

This reverts commit 3c148ba.
  • Loading branch information
danjm committed Aug 27, 2021
1 parent 3c148ba commit 5a754be
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 36 deletions.
3 changes: 2 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ class LedgerBridgeKeyring extends EventEmitter {
rawTxHex = tx.type === 0 || !tx.type
? ethUtil.rlp.encode(tx.getMessageToSign(false)).toString('hex')
: tx.getMessageToSign(false).toString('hex')
return this._signTransaction(address, rawTxHex, tx.to && tx.to.buf, (payload) => {

return this._signTransaction(address, rawTxHex, tx.to.buf, (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.
Expand Down
35 changes: 0 additions & 35 deletions test/test-eth-ledger-bridge-keyring.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,6 @@ const newFakeTx = TransactionFactory.fromTxData({
data: '0x7f7465737432000000000000000000000000000000000000000000000000000000600057',
}, { common, freeze: false })

const newFakeTxWithoutTo = TransactionFactory.fromTxData({
nonce: '0x00',
gasPrice: '0x09184e72a000',
gasLimit: '0x2710',
value: '0x00',
data: '0x7f7465737432000000000000000000000000000000000000000000000000000000600057',
}, { common, freeze: false })

chai.use(spies)

describe('LedgerBridgeKeyring', function () {
Expand Down Expand Up @@ -491,33 +483,6 @@ describe('LedgerBridgeKeyring', function () {
expect(returnedTx).to.have.property('r')
expect(returnedTx).to.have.property('s')
})

it('should support transactions with undefined to fields', async function () {
await basicSetupToUnlockOneAccount()
// Signature will be invalid due to not having private key / public key
// pair for testing.
sandbox.on(newFakeTxWithoutTo, 'verifySignature', () => true)
sandbox.on(TransactionFactory, 'fromTxData', () => {
// without having a private key/public key pair in this test, we have
// mock out this method and return the original tx because we can't
// replicate r and s values without the private key.
return newFakeTxWithoutTo
})
sandbox.on(keyring, '_sendMessage', (msg, cb) => {
assert.deepStrictEqual(msg.params, {
hdPath: "m/44'/60'/0'/0",
to: ethUtil.bufferToHex(undefined),
tx: ethUtil.rlp.encode(newFakeTxWithoutTo.getMessageToSign(false)).toString('hex'),
})
cb({ success: true, payload: { v: '0x25', r: '0x0', s: '0x0' } })
})

const returnedTx = await keyring.signTransaction(fakeAccounts[0], newFakeTxWithoutTo, common)
expect(keyring._sendMessage).to.have.been.called()
expect(returnedTx).to.have.property('v')
expect(returnedTx).to.have.property('r')
expect(returnedTx).to.have.property('s')
})
})
})

Expand Down

0 comments on commit 5a754be

Please sign in to comment.