From 047b2caf6f31ac040c9f892fc985c70331429483 Mon Sep 17 00:00:00 2001 From: David Murdoch <187813+davidmurdoch@users.noreply.github.com> Date: Tue, 7 Sep 2021 16:31:37 -0400 Subject: [PATCH] fix: make sure zero address is always signed with the same key (#1117) --- .../ethereum/ethereum/src/transaction-pool.ts | 9 +- .../tests/api/eth/sendTransaction.test.ts | 96 +++++++++++++++++++ .../ethereum/tests/temp-tests.test.ts | 55 ----------- 3 files changed, 102 insertions(+), 58 deletions(-) diff --git a/src/chains/ethereum/ethereum/src/transaction-pool.ts b/src/chains/ethereum/ethereum/src/transaction-pool.ts index fc336f832f..c4593af70e 100644 --- a/src/chains/ethereum/ethereum/src/transaction-pool.ts +++ b/src/chains/ethereum/ethereum/src/transaction-pool.ts @@ -208,10 +208,13 @@ export default class TransactionPool extends Emittery.Typed<{}, "drain"> { let fakePrivateKey: Buffer; if (from.equals(ACCOUNT_ZERO)) { - fakePrivateKey = Buffer.allocUnsafe(32); - // allow signing with the 0x0 address + // allow signing with the 0x0 address... + // always sign with the same fake key, a 31 `0`s followed by a single + // `1`. The key is arbitrary. It just must not be all `0`s and must be + // deterministic. // see: https://github.com/ethereumjs/ethereumjs-monorepo/issues/829#issue-674385636 - fakePrivateKey[0] = 1; + fakePrivateKey = Buffer.allocUnsafe(32).fill(0, 0, 31); + fakePrivateKey[31] = 1; } else { fakePrivateKey = Buffer.concat([from, from.slice(0, 12)]); } diff --git a/src/chains/ethereum/ethereum/tests/api/eth/sendTransaction.test.ts b/src/chains/ethereum/ethereum/tests/api/eth/sendTransaction.test.ts index 72deeeb1b9..b5a6fb973c 100644 --- a/src/chains/ethereum/ethereum/tests/api/eth/sendTransaction.test.ts +++ b/src/chains/ethereum/ethereum/tests/api/eth/sendTransaction.test.ts @@ -167,6 +167,102 @@ describe("api", () => { }); }); }); + + describe("unlocked accounts", () => { + it("can send transactions from an unlocked 0x0 address", async () => { + const ZERO_ADDRESS = "0x" + "0".repeat(40); + const provider = await getProvider({ + miner: { + defaultGasPrice: 0 + }, + wallet: { + unlockedAccounts: [ZERO_ADDRESS] + } + }); + const [from] = await provider.send("eth_accounts"); + await provider.send("eth_subscribe", ["newHeads"]); + const initialZeroBalance = "0x1234"; + await provider.send("eth_sendTransaction", [ + { from: from, to: ZERO_ADDRESS, value: initialZeroBalance } + ]); + await provider.once("message"); + const initialBalance = await provider.send("eth_getBalance", [ + ZERO_ADDRESS + ]); + assert.strictEqual( + initialBalance, + initialZeroBalance, + "Zero address's balance isn't correct" + ); + const removeValueFromZeroAmount = "0x123"; + await provider.send("eth_sendTransaction", [ + { from: ZERO_ADDRESS, to: from, value: removeValueFromZeroAmount } + ]); + await provider.once("message"); + const afterSendBalance = BigInt( + await provider.send("eth_getBalance", [ZERO_ADDRESS]) + ); + assert.strictEqual( + BigInt(initialZeroBalance) - BigInt(removeValueFromZeroAmount), + afterSendBalance, + "Zero address's balance isn't correct" + ); + }); + + it("unlocks accounts via unlock_accounts (both string and numbered numbers)", async () => { + const p = await getProvider({ + wallet: { + secure: true, + unlockedAccounts: ["0", 1] + } + }); + + const accounts = await p.send("eth_accounts"); + const balance1_1 = await p.send("eth_getBalance", [accounts[1]]); + const badSend = async () => { + return p.send("eth_sendTransaction", [ + { + from: accounts[2], + to: accounts[1], + value: "0x7b" + } + ]); + }; + await assert.rejects( + badSend, + "Error: authentication needed: password or unlock" + ); + + await p.send("eth_subscribe", ["newHeads"]); + await p.send("eth_sendTransaction", [ + { + from: accounts[0], + to: accounts[1], + value: "0x7b" + } + ]); + + await p.once("message"); + + const balance1_2 = await p.send("eth_getBalance", [accounts[1]]); + assert.strictEqual(BigInt(balance1_1) + 123n, BigInt(balance1_2)); + + const balance0_1 = await p.send("eth_getBalance", [accounts[0]]); + + await p.send("eth_sendTransaction", [ + { + from: accounts[1], + to: accounts[0], + value: "0x7b" + } + ]); + + await p.once("message"); + + const balance0_2 = await p.send("eth_getBalance", [accounts[0]]); + assert.strictEqual(BigInt(balance0_1) + 123n, BigInt(balance0_2)); + }); + }); }); }); }); diff --git a/src/chains/ethereum/ethereum/tests/temp-tests.test.ts b/src/chains/ethereum/ethereum/tests/temp-tests.test.ts index 2eab4704c4..02e847b7ac 100644 --- a/src/chains/ethereum/ethereum/tests/temp-tests.test.ts +++ b/src/chains/ethereum/ethereum/tests/temp-tests.test.ts @@ -87,61 +87,6 @@ describe("Random tests that are temporary!", () => { ); }); - it("unlocks accounts via unlock_accounts (both string and numbered numbers)", async () => { - const p = await getProvider({ - wallet: { - mnemonic, - secure: true, - unlockedAccounts: ["0", 1] - } - }); - - const accounts = await p.send("eth_accounts"); - const balance1_1 = await p.send("eth_getBalance", [accounts[1]]); - const badSend = async () => { - return p.send("eth_sendTransaction", [ - { - from: accounts[2], - to: accounts[1], - value: "0x7b" - } - ]); - }; - await assert.rejects( - badSend, - "Error: authentication needed: password or unlock" - ); - - await p.send("eth_subscribe", ["newHeads"]); - await p.send("eth_sendTransaction", [ - { - from: accounts[0], - to: accounts[1], - value: "0x7b" - } - ]); - - await p.once("message"); - - const balance1_2 = await p.send("eth_getBalance", [accounts[1]]); - assert.strictEqual(BigInt(balance1_1) + 123n, BigInt(balance1_2)); - - const balance0_1 = await p.send("eth_getBalance", [accounts[0]]); - - await p.send("eth_sendTransaction", [ - { - from: accounts[1], - to: accounts[0], - value: "0x7b" - } - ]); - - await p.once("message"); - - const balance0_2 = await p.send("eth_getBalance", [accounts[0]]); - assert.strictEqual(BigInt(balance0_1) + 123n, BigInt(balance0_2)); - }); - it("deploys contracts", async () => { const contract = compile(join(__dirname, "./contracts/HelloWorld.sol"));