diff --git a/packages/tx/src/legacyTransaction.ts b/packages/tx/src/legacyTransaction.ts index 0df16cb579..2ebfe4537d 100644 --- a/packages/tx/src/legacyTransaction.ts +++ b/packages/tx/src/legacyTransaction.ts @@ -136,6 +136,18 @@ export default class Transaction extends BaseTransaction { } } + if (!this.supports(Capability.EIP155ReplayProtection)) { + // In case of a signed non-EIP155 tx, `ecrecover` does not get a `chainId` provided + // Internally, `ecrecover` will calculate the recovery bit by `v = v - 27` + // This recovery bit is either 0 or 1. Thus, any other value than `v = 27` or `v = 28` will + // throw upon calling `getSenderAddress` here. Thus throw early in the constructor + if (this.v !== undefined) { + if (!this.v.eqn(27) && !this.v.eqn(28)) { + throw new Error(`Non-EIP155 txs need either v = 27 or v = 28, got v = ${this.v}`) + } + } + } + if (this.common.isActivatedEIP(3860)) { checkMaxInitCodeSize(this.common, this.data.length) } @@ -382,7 +394,7 @@ export default class Transaction extends BaseTransaction { // No unsigned tx and EIP-155 activated and chain ID included if ( v !== undefined && - !v.eqn(0) && + !v.eqn(0) && // TODO fixme -> why overwrite/return default common when `v` is set to 0? (!common || common.gteHardfork('spuriousDragon')) && !v.eqn(27) && !v.eqn(28) diff --git a/packages/tx/test/legacy.spec.ts b/packages/tx/test/legacy.spec.ts index 3397585b87..b6d30e8d06 100644 --- a/packages/tx/test/legacy.spec.ts +++ b/packages/tx/test/legacy.spec.ts @@ -389,8 +389,6 @@ tape('[Transaction]', function (t) { 'hex' ) - const fixtureTxSignedWithEIP155 = Transaction.fromTxData(txData).sign(privateKey) - const common = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.TangerineWhistle, @@ -400,9 +398,7 @@ tape('[Transaction]', function (t) { common, }).sign(privateKey) - let signedWithEIP155 = Transaction.fromTxData(fixtureTxSignedWithEIP155.toJSON()).sign( - privateKey - ) + let signedWithEIP155 = Transaction.fromTxData(txData).sign(privateKey) st.true(signedWithEIP155.verifySignature()) st.notEqual(signedWithEIP155.v?.toString('hex'), '1c') @@ -416,7 +412,7 @@ tape('[Transaction]', function (t) { st.notEqual(signedWithEIP155.v?.toString('hex'), '1c') st.notEqual(signedWithEIP155.v?.toString('hex'), '1b') - let signedWithoutEIP155 = Transaction.fromTxData(fixtureTxSignedWithEIP155.toJSON(), { + let signedWithoutEIP155 = Transaction.fromTxData(txData, { common, }).sign(privateKey) @@ -427,7 +423,7 @@ tape('[Transaction]', function (t) { "v shouldn't be EIP155 encoded" ) - signedWithoutEIP155 = Transaction.fromTxData(fixtureTxSignedWithoutEIP155.toJSON(), { + signedWithoutEIP155 = Transaction.fromTxData(txData, { common, }).sign(privateKey) @@ -442,6 +438,24 @@ tape('[Transaction]', function (t) { } ) + t.test( + 'constructor: throw on non-EIP155 transactions which have v != 27 and v != 28', + function (st) { + function getTxData(v: number) { + return { + v, + } + } + for (let n = 0; n < 27; n++) { + st.throws(() => Transaction.fromTxData(getTxData(n))) + } + st.throws(() => Transaction.fromTxData(getTxData(29))) + st.doesNotThrow(() => Transaction.fromTxData(getTxData(27))) + st.doesNotThrow(() => Transaction.fromTxData(getTxData(28))) + st.end() + } + ) + t.test('sign(), verifySignature(): sign tx with chainId specified in params', function (st) { const common = new Common({ chain: 42, hardfork: Hardfork.Petersburg }) let tx = Transaction.fromTxData({}, { common })