Skip to content

Commit ceeedc6

Browse files
holgerd77jochem-brouwer
authored andcommitted
tx -> EIP-2929: started to separate dedicated legacy and base tests, test clean-up, add more tests
tx -> EIP-2930: fixed missing toBuffer conversion for v value in EIP2930Transaction constructor tx -> EIP-2930: added generic API tests for serialize() and raw() tx -> EIP-2930: added generic verifySignature() tests tx -> EIP-2920: added generic verifySignature() error test cases tx -> EIP-2930: small test fix
1 parent 9ececec commit ceeedc6

File tree

7 files changed

+310
-125
lines changed

7 files changed

+310
-125
lines changed

packages/tx/src/eip2930Transaction.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,9 @@ export default class EIP2930Transaction extends BaseTransaction<EIP2930Transacti
170170
this.AccessListJSON = json
171171
}
172172

173-
this.chainId = new BN(toBuffer(chainId))
173+
this.chainId = chainId ? new BN(toBuffer(chainId)) : new BN(this.common.chainId())
174174
this.accessList = usedAccessList
175-
this.v = v ? new BN(v) : undefined
175+
this.v = v ? new BN(toBuffer(v)) : undefined
176176
this.r = r ? new BN(toBuffer(r)) : undefined
177177
this.s = s ? new BN(toBuffer(s)) : undefined
178178

packages/tx/test/api.spec.ts

Lines changed: 8 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,17 @@
11
import tape from 'tape'
22
import { Buffer } from 'buffer'
3-
import {
4-
BN,
5-
rlp,
6-
zeros,
7-
privateToPublic,
8-
toBuffer,
9-
bufferToHex,
10-
unpadBuffer,
11-
} from 'ethereumjs-util'
3+
import { BN, rlp, privateToPublic, toBuffer, bufferToHex, unpadBuffer } from 'ethereumjs-util'
124
import Common from '@ethereumjs/common'
135
import { LegacyTransaction, TxData } from '../src'
146
import { TxsJsonEntry, VitaliksTestsDataEntry } from './types'
157

168
const txFixtures: TxsJsonEntry[] = require('./json/txs.json')
179
const txFixturesEip155: VitaliksTestsDataEntry[] = require('./json/ttTransactionTestEip155VitaliksTests.json')
1810

19-
tape('[Transaction]: Basic functions', function (t) {
11+
tape('[Transaction]', function (t) {
2012
const transactions: LegacyTransaction[] = []
2113

22-
t.test('should initialize correctly', function (st) {
23-
let tx = LegacyTransaction.fromTxData({})
24-
st.equal(tx.common.hardfork(), 'berlin', 'should initialize with correct default HF')
25-
st.ok(Object.isFrozen(tx), 'tx should be frozen by default')
26-
27-
const common = new Common({ chain: 'mainnet', hardfork: 'spuriousDragon' })
28-
tx = LegacyTransaction.fromTxData({}, { common })
29-
st.equal(tx.common.hardfork(), 'spuriousDragon', 'should initialize with correct HF provided')
30-
31-
common.setHardfork('byzantium')
32-
st.equal(
33-
tx.common.hardfork(),
34-
'spuriousDragon',
35-
'should stay on correct HF if outer common HF changes'
36-
)
37-
38-
tx = LegacyTransaction.fromTxData({}, { freeze: false })
39-
st.ok(!Object.isFrozen(tx), 'tx should not be frozen when freeze deactivated in options')
40-
41-
// Perform the same test as above, but now using a different construction method. This also implies that passing on the
42-
// options object works as expected.
43-
const rlpData = tx.serialize()
44-
45-
const zero = Buffer.alloc(0)
46-
const valuesArray = [zero, zero, zero, zero, zero, zero]
47-
48-
tx = LegacyTransaction.fromRlpSerializedTx(rlpData)
49-
st.ok(Object.isFrozen(tx), 'tx should be frozen by default')
50-
51-
tx = LegacyTransaction.fromRlpSerializedTx(rlpData, { freeze: false })
52-
st.ok(!Object.isFrozen(tx), 'tx should not be frozen when freeze deactivated in options')
53-
54-
tx = LegacyTransaction.fromValuesArray(valuesArray)
55-
st.ok(Object.isFrozen(tx), 'tx should be frozen by default')
56-
57-
tx = LegacyTransaction.fromValuesArray(valuesArray, { freeze: false })
58-
st.ok(!Object.isFrozen(tx), 'tx should not be frozen when freeze deactivated in options')
59-
60-
st.end()
61-
})
62-
63-
t.test('should decode transactions', function (st) {
14+
t.test('Initialization -> decode with fromValuesArray()', function (st) {
6415
txFixtures.slice(0, 4).forEach(function (tx: any) {
6516
const txData = tx.raw.map(toBuffer)
6617
const pt = LegacyTransaction.fromValuesArray(txData)
@@ -80,7 +31,7 @@ tape('[Transaction]: Basic functions', function (t) {
8031
st.end()
8132
})
8233

83-
t.test('should serialize', function (st) {
34+
t.test('serialize()', function (st) {
8435
transactions.forEach(function (tx, i) {
8536
const s1 = tx.serialize()
8637
const s2 = rlp.encode(txFixtures[i].raw)
@@ -89,7 +40,7 @@ tape('[Transaction]: Basic functions', function (t) {
8940
st.end()
9041
})
9142

92-
t.test('should hash', function (st) {
43+
t.test('hash()', function (st) {
9344
const common = new Common({
9445
chain: 'mainnet',
9546
hardfork: 'tangerineWhistle',
@@ -112,7 +63,7 @@ tape('[Transaction]: Basic functions', function (t) {
11263
st.end()
11364
})
11465

115-
t.test('should hash with defined chainId', function (st) {
66+
t.test('hash() -> with defined chainId', function (st) {
11667
const tx = LegacyTransaction.fromValuesArray(txFixtures[4].raw.map(toBuffer))
11768
st.equal(
11869
tx.hash().toString('hex'),
@@ -129,43 +80,11 @@ tape('[Transaction]: Basic functions', function (t) {
12980
st.end()
13081
})
13182

132-
t.test('should verify Signatures', function (st) {
133-
transactions.forEach(function (tx) {
134-
st.equals((<LegacyTransaction>tx).verifySignature(), true)
135-
})
136-
st.end()
137-
})
138-
139-
t.test('should not verify invalid signatures', function (st) {
140-
const txs: LegacyTransaction[] = []
141-
142-
txFixtures.slice(0, 4).forEach(function (txFixture: any) {
143-
const txData = txFixture.raw.map(toBuffer)
144-
// set `s` to zero
145-
txData[8] = zeros(32)
146-
const tx = LegacyTransaction.fromValuesArray(txData)
147-
txs.push(tx)
148-
})
149-
150-
txs.forEach(function (tx) {
151-
st.equals(tx.verifySignature(), false)
152-
153-
st.ok(
154-
tx.validate(true).includes('Invalid Signature'),
155-
'should give a string about not verifying signatures'
156-
)
157-
158-
st.notOk(tx.validate(), 'should validate correctly')
159-
})
160-
161-
st.end()
162-
})
163-
164-
t.test('should sign tx', function (st) {
83+
t.test('sign()', function (st) {
16584
transactions.forEach(function (tx, i) {
16685
const { privateKey } = txFixtures[i]
16786
if (privateKey) {
168-
st.ok(tx.sign(Buffer.from(privateKey, 'hex')))
87+
st.ok(tx.sign(Buffer.from(privateKey, 'hex')), 'should sign tx')
16988
}
17089
})
17190
st.end()

packages/tx/test/base.spec.ts

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
import tape from 'tape'
2+
import Common from '@ethereumjs/common'
3+
import { LegacyTransaction, EIP2930Transaction } from '../src'
4+
import { TxsJsonEntry } from './types'
5+
import { BaseTransaction } from '../src/baseTransaction'
6+
7+
tape('[BaseTransaction]', function (t) {
8+
const legacyFixtures: TxsJsonEntry[] = require('./json/txs.json')
9+
const legacyTxs: BaseTransaction<LegacyTransaction>[] = []
10+
legacyFixtures.slice(0, 4).forEach(function (tx: any) {
11+
legacyTxs.push(LegacyTransaction.fromTxData(tx.data))
12+
})
13+
14+
const eip2930Fixtures = require('./json/eip2930txs.json')
15+
const eip2930Txs: BaseTransaction<EIP2930Transaction>[] = []
16+
eip2930Fixtures.forEach(function (tx: any) {
17+
eip2930Txs.push(EIP2930Transaction.fromTxData(tx.data))
18+
})
19+
20+
const zero = Buffer.alloc(0)
21+
const txTypes = [
22+
{
23+
class: LegacyTransaction,
24+
name: 'LegacyTransaction',
25+
values: Array(6).fill(zero),
26+
txs: legacyTxs,
27+
fixtures: legacyFixtures,
28+
},
29+
{
30+
class: EIP2930Transaction,
31+
name: 'EIP2930Transaction',
32+
values: [Buffer.from([1])].concat(Array(7).fill(zero)),
33+
txs: eip2930Txs,
34+
fixtures: eip2930Fixtures,
35+
},
36+
]
37+
38+
t.test('Initialization', function (st) {
39+
for (const txType of txTypes) {
40+
let tx = txType.class.fromTxData({})
41+
st.equal(
42+
tx.common.hardfork(),
43+
'berlin',
44+
`${txType.name}: should initialize with correct default HF`
45+
)
46+
st.ok(Object.isFrozen(tx), `${txType.name}: tx should be frozen by default`)
47+
48+
const common = new Common({
49+
chain: 'mainnet',
50+
hardfork: 'istanbul',
51+
eips: [2718, 2929, 2930],
52+
})
53+
tx = txType.class.fromTxData({}, { common })
54+
st.equal(
55+
tx.common.hardfork(),
56+
'istanbul',
57+
`${txType.name}: should initialize with correct HF provided`
58+
)
59+
60+
common.setHardfork('byzantium')
61+
st.equal(
62+
tx.common.hardfork(),
63+
'istanbul',
64+
`${txType.name}: should stay on correct HF if outer common HF changes`
65+
)
66+
67+
tx = txType.class.fromTxData({}, { freeze: false })
68+
tx = txType.class.fromTxData({}, { freeze: false })
69+
st.ok(
70+
!Object.isFrozen(tx),
71+
`${txType.name}: tx should not be frozen when freeze deactivated in options`
72+
)
73+
74+
// Perform the same test as above, but now using a different construction method. This also implies that passing on the
75+
// options object works as expected.
76+
tx = txType.class.fromTxData({}, { freeze: false })
77+
const rlpData = tx.serialize()
78+
79+
tx = txType.class.fromRlpSerializedTx(rlpData)
80+
st.ok(Object.isFrozen(tx), `${txType.name}: tx should be frozen by default`)
81+
82+
tx = txType.class.fromRlpSerializedTx(rlpData, { freeze: false })
83+
st.ok(
84+
!Object.isFrozen(tx),
85+
`${txType.name}: tx should not be frozen when freeze deactivated in options`
86+
)
87+
88+
tx = txType.class.fromValuesArray(txType.values)
89+
st.ok(Object.isFrozen(tx), `${txType.name}: tx should be frozen by default`)
90+
91+
tx = txType.class.fromValuesArray(txType.values, { freeze: false })
92+
st.ok(
93+
!Object.isFrozen(tx),
94+
`${txType.name}: tx should not be frozen when freeze deactivated in options`
95+
)
96+
}
97+
st.end()
98+
})
99+
100+
t.test('serialize()', function (st) {
101+
for (const txType of txTypes) {
102+
txType.txs.forEach(function (tx: any) {
103+
st.ok(
104+
txType.class.fromRlpSerializedTx(tx.serialize()),
105+
`${txType.name}: should do roundtrip serialize() -> fromRlpSerializedTx()`
106+
)
107+
})
108+
}
109+
st.end()
110+
})
111+
112+
t.test('raw()', function (st) {
113+
for (const txType of txTypes) {
114+
txType.txs.forEach(function (tx: any) {
115+
st.ok(
116+
txType.class.fromValuesArray(tx.raw(true)),
117+
`${txType.name}: should do roundtrip raw() -> fromValuesArray()`
118+
)
119+
})
120+
}
121+
st.end()
122+
})
123+
124+
t.test('verifySignature()', function (st) {
125+
for (const txType of txTypes) {
126+
txType.txs.forEach(function (tx: any) {
127+
st.equals(tx.verifySignature(), true, `${txType.name}: signature should be valid`)
128+
})
129+
}
130+
st.end()
131+
})
132+
133+
t.test('verifySignature() -> invalid', function (st) {
134+
for (const txType of txTypes) {
135+
txType.fixtures.slice(0, 4).forEach(function (txFixture: any) {
136+
// set `s` to zero
137+
txFixture.data.s = `0x` + '0'.repeat(16)
138+
const tx = txType.class.fromTxData(txFixture.data)
139+
st.equals(tx.verifySignature(), false, `${txType.name}: signature should not be valid`)
140+
st.ok(
141+
(<string[]>tx.validate(true)).includes('Invalid Signature'),
142+
`${txType.name}: should return an error string about not verifying signatures`
143+
)
144+
st.notOk(tx.validate(), `${txType.name}: should not validate correctly`)
145+
})
146+
}
147+
148+
st.end()
149+
})
150+
})

packages/tx/test/eip2930.spec.ts

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,28 @@ const validSlot = Buffer.from('01'.repeat(32), 'hex')
1717

1818
const chainId = new BN(1)
1919

20-
tape('[EIP2930 transactions]: Basic functions', function (t) {
20+
tape('[EIP2930Transaction]', function (t) {
21+
t.test('Initialization / Getter', function (t) {
22+
t.throws(() => {
23+
EIP2930Transaction.fromTxData(
24+
{
25+
chainId: chainId.addn(1),
26+
},
27+
{ common }
28+
)
29+
}, 'should reject transactions with wrong chain ID')
30+
31+
t.throws(() => {
32+
EIP2930Transaction.fromTxData(
33+
{
34+
v: 2,
35+
},
36+
{ common }
37+
)
38+
}, 'should reject transactions with invalid yParity (v) values')
39+
t.end()
40+
})
41+
2142
t.test('should allow json-typed access lists', function (st) {
2243
const access: AccessList = [
2344
{
@@ -204,30 +225,6 @@ tape('[EIP2930 transactions]: Basic functions', function (t) {
204225
t.end()
205226
})
206227

207-
t.test('should reject transactions with wrong chain ID', function (t) {
208-
t.throws(() => {
209-
EIP2930Transaction.fromTxData(
210-
{
211-
chainId: chainId.addn(1),
212-
},
213-
{ common }
214-
)
215-
})
216-
t.end()
217-
})
218-
219-
t.test('should reject transactions with invalid yParity (v) values', function (t) {
220-
t.throws(() => {
221-
EIP2930Transaction.fromTxData(
222-
{
223-
v: 2,
224-
},
225-
{ common }
226-
)
227-
})
228-
t.end()
229-
})
230-
231228
// Data from
232229
// https://github.com/INFURA/go-ethlibs/blob/75b2a52a39d353ed8206cffaf68d09bd1b154aae/eth/transaction_signing_test.go#L87
233230

packages/tx/test/index.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,26 @@ import minimist from 'minimist'
22

33
const argv = minimist(process.argv.slice(2))
44

5-
if (argv.a) {
6-
require('./api.spec')
5+
if (argv.b) {
6+
require('./base.spec')
7+
} else if (argv.l) {
8+
require('./legacy.spec')
9+
} else if (argv.e) {
10+
require('./eip2930.spec')
711
} else if (argv.t) {
812
require('./transactionRunner')
13+
} else if (argv.f) {
14+
require('./transactionFactory.spec')
15+
} else if (argv.a) {
16+
// All manual API tests
17+
require('./base.spec')
18+
require('./legacy.spec')
19+
require('./eip2930.spec')
20+
require('./transactionFactory.spec')
921
} else {
10-
require('./api.spec')
1122
require('./transactionRunner')
12-
require('./transactionFactory.spec')
23+
require('./base.spec')
24+
require('./legacy.spec')
1325
require('./eip2930.spec')
26+
require('./transactionFactory.spec')
1427
}

0 commit comments

Comments
 (0)