Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/client/src/rpc/modules/eth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ type JSONRPCLog = {
transactionHash: string | null // DATA, 32 Bytes - hash of the transactions this log was created from. null when it's pending.
blockHash: string | null // DATA, 32 Bytes - hash of the block where this log was in. null when it's pending.
blockNumber: string | null // QUANTITY - the block number where this log was in. null when it's pending.
blockTimestamp: string | null // QUANTITY - the block timestamp where this log was in. null when it's pending.
address: string // DATA, 20 Bytes - address from which this log originated.
data: string // DATA - contains one or more 32 Bytes non-indexed arguments of the log.
topics: string[] // Array of DATA - Array of 0 to 4 32 Bytes DATA of indexed log arguments.
Expand Down Expand Up @@ -174,6 +175,7 @@ const toJSONRPCLog = async (
transactionHash: tx !== undefined ? bytesToHex(tx.hash()) : null,
blockHash: block ? bytesToHex(block.hash()) : null,
blockNumber: block ? bigIntToHex(block.header.number) : null,
blockTimestamp: block ? bigIntToHex(block.header.timestamp) : null,
address: bytesToHex(log[0]),
topics: log[1].map(bytesToHex),
data: bytesToHex(log[2]),
Expand Down
58 changes: 56 additions & 2 deletions packages/client/test/rpc/eth/getLogs.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { createLegacyTx } from '@ethereumjs/tx'
import { bytesToHex, createContractAddress, hexToBytes } from '@ethereumjs/util'
import {
bigIntToHex,
bytesToHex,
createContractAddress,
hexToBigInt,
hexToBytes,
} from '@ethereumjs/util'
import { assert, describe, it } from 'vitest'

import { INVALID_PARAMS } from '../../../src/rpc/error-code.ts'
Expand Down Expand Up @@ -75,7 +81,7 @@ describe(method, async () => {
{ common },
).sign(dummy.privKey)

await runBlockWithTxs(chain, execution, [tx1, tx2, tx3, tx4])
let block = await runBlockWithTxs(chain, execution, [tx1, tx2, tx3, tx4])

// compare the logs
let res = await rpc.request(method, [{ fromBlock: 'earliest', toBlock: 'latest' }])
Expand All @@ -100,6 +106,17 @@ describe(method, async () => {
assert.fail(`should return the correct logs (fromBlock/toBlock as 'earliest' and 'latest')`)
}

if (hexToBigInt(res.result[0].blockTimestamp) === block.header.timestamp) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test should assert.isEqual or something similar here, right? The if/else statement together with assert.fail / assert.isTrue(true simulates the behavior of the equal check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, was just following the style of the surrounding tests, which did this a fair bit

assert.isTrue(
true,
`should return the correct blockTimestamp (fromBlock/toBlock as 'earliest' and 'latest')`,
)
} else {
assert.fail(
`should return the correct blockTimestamp (fromBlock/toBlock as 'earliest' and 'latest')`,
)
}

// get the logs using fromBlock/toBlock as numbers
res = await rpc.request(method, [{ fromBlock: '0x0', toBlock: '0x1' }])
assert.strictEqual(
Expand Down Expand Up @@ -206,6 +223,43 @@ describe(method, async () => {
20,
'should return the correct logs (filter by blockHash)',
)

// test adding more logs and checking timestamps against multiple blocks
const tx5 = createLegacyTx(
{
...txData,
data,
to: contractAddr1,
nonce: 4,
},
{ common },
).sign(dummy.privKey)
const tx6 = createLegacyTx(
{
...txData,
data,
to: contractAddr2,
nonce: 5,
},
{ common },
).sign(dummy.privKey)

const oldTimestamp = bigIntToHex(block.header.timestamp)
block = await runBlockWithTxs(chain, execution, [tx5, tx6])
const newTimestamp = bigIntToHex(block.header.timestamp)

res = await rpc.request(method, [{ fromBlock: 'earliest', toBlock: 'latest' }])

if (
res.result[0].blockTimestamp === oldTimestamp &&
res.result[10].blockTimestamp === oldTimestamp &&
res.result[20].blockTimestamp === newTimestamp &&
res.result[30].blockTimestamp === newTimestamp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is not very intuitive. Why should these values be these values? It seems like a range for eth_getLogs is tested but it is not clear to me why index 10 is oldTimestamp and 20 newTimestamp for instance

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed some changes, hopefully its clearer

) {
assert.isTrue(true, 'should return the correct log timestamps across multiple blocks')
} else {
assert.fail('should return the correct log timestamps across multiple blocks')
}
})

it('call with invalid params', async () => {
Expand Down