Skip to content

Conversation

@msieczko
Copy link
Member

@msieczko msieczko commented May 28, 2020

This PR is not fully finished yet. The code sufficient to run all of the transactions and calls used in benchmarks against raw VM can be found in src and test directories. The existing benchmarks are yet to be adjusted to make use of that code.

In order to install dependencies:

npm run verdaccio:prepare
npm run verdaccio:run

in separate terminal window:

npm run verdaccio:install

btw don't try running benchmarks because they don't work at the moment (start, start:dirty and benchmark scripts)

@msieczko msieczko requested a review from sz-piotr May 28, 2020 15:11

async sendTransaction (signedTransaction: HexData): Promise<Hash> {
const hash = await this.vm.addPendingTransaction(signedTransaction)
if (this.options.autoMining) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of things like this could be stripped, since you don't even have mine block in the provider.
Of course doing this is probably not worth the effort.

// eslint-disable-next-line
import { RunTxResult } from '@nomiclabs/ethereumjs-vm/dist/runTx'

export function getReceiptsAndResponses (block: Block, transactions: Transaction[], results: RunTxResult[]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably get rid of this, unless we send transactions from ethers directly

Copy link
Contributor

Choose a reason for hiding this comment

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

Not calling this function might have a performance impact, because it does a lot of buffer stringifying.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I don't send any transactions from ethers, so I think it's safe to remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants