Skip to content

Commit

Permalink
🐛 Fix: add support for incrementing nonces (#1074)
Browse files Browse the repository at this point in the history
## Description

Fixes a bug where nonces would not increment if more than 0 tx from the
same address are in the tx pool

## Testing

Explain the quality checks that have been done on the code changes

## Additional Information

- [ ] I read the [contributing docs](../docs/contributing.md) (if this
is your first contribution)

Your ENS/address:



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Added the ability to retrieve all mempool transactions from a specific
sender address.

- **Bug Fixes**
- Improved error handling in transaction processing and contract
interactions.

- **Tests**
- Introduced new test cases for multiple transactions and mining
scenarios.

- **Documentation**
  - Updated method locations and descriptions in `TxPool` documentation.

- **Refactor**
  - Enhanced error decoding logic in contract handling.
  - Adjusted nonce and gas limit calculations for transactions.

- **Chores**
- Updated dependencies and expanded exported entities across various
modules.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: William Cory <[email protected]>
  • Loading branch information
roninjin10 and William Cory authored May 15, 2024
1 parent 77dc398 commit 2ba2c27
Show file tree
Hide file tree
Showing 22 changed files with 188 additions and 69 deletions.
6 changes: 6 additions & 0 deletions .changeset/young-actors-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@tevm/txpool": minor
"tevm": minor
---

Added bySenderAddress method to return all mempool tx from a single sender address
58 changes: 31 additions & 27 deletions packages/actions/src/tevm/callHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,31 @@ const forkAndCacheBlock = async (client, block) => {
client.logger.warn(`Detected block tag for a call was set to a different block tag than the chain was forked from.
Tevm can peform slowly when this happens from needing to process that blocks entire transaction list.
This will be fixed in future versions.`)
client.logger.debug('Forking a new block based on block tag...')
// if no state root fork the block with a fresh state manager
const vm = await client.getVm()
const evm = vm.evm.shallowCopy()
// this can't happen making ts happy
if (!client.forkUrl) {
throw new Error('Cannot fork without a fork url')
}
const stateManager = createStateManager({
...vm.evm.stateManager._baseState.options,
fork: {
url: client.forkUrl,
blockTag: block.header.number
}
})
// TODO this type is too narrow
evm.stateManager = /** @type any*/(stateManager)
const transactions = /** @type {import('@tevm/block').Block}*/(block).transactions
client.logger.debug({count: transactions.length}, 'Processing transactions')
transactions.map(async (tx, i) => {
client.logger.debug({txNumber: i, tx}, 'Processing transaction')
await evm.runCall(tx)
})
client.logger.debug('Finished processing block transactions and saving state root')
vm.stateManager.saveStateRoot(block.header.stateRoot, await stateManager.dumpCanonicalGenesis())
client.logger.debug('Forking a new block based on block tag...')
// if no state root fork the block with a fresh state manager
const vm = await client.getVm()
const evm = vm.evm.shallowCopy()
// this can't happen making ts happy
if (!client.forkUrl) {
throw new Error('Cannot fork without a fork url')
}
const stateManager = createStateManager({
...vm.evm.stateManager._baseState.options,
fork: {
url: client.forkUrl,
blockTag: block.header.number
}
})
// TODO this type is too narrow
evm.stateManager = /** @type any*/(stateManager)
const transactions = /** @type {import('@tevm/block').Block}*/(block).transactions
client.logger.debug({ count: transactions.length }, 'Processing transactions')
transactions.map(async (tx, i) => {
client.logger.debug({ txNumber: i, tx }, 'Processing transaction')
await evm.runCall(tx)
})
client.logger.debug('Finished processing block transactions and saving state root')
vm.stateManager.saveStateRoot(block.header.stateRoot, await stateManager.dumpCanonicalGenesis())
}

/**
Expand Down Expand Up @@ -149,7 +149,9 @@ export const callHandler =
? e
: e instanceof Error
? e.message
: 'unknown error',
: typeof e === 'object' && e !== null && 'message' in e
? e.message
: 'unknown error',
},
],
executionGasUsed: 0n,
Expand Down Expand Up @@ -215,7 +217,9 @@ export const callHandler =
? e
: e instanceof Error
? e.message
: 'unknown error',
: typeof e === 'object' && e !== null && 'message' in e
? e.message
: 'unknown error',
},
],
executionGasUsed: 0n,
Expand Down
78 changes: 77 additions & 1 deletion packages/actions/src/tevm/callHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { getAccountHandler } from './getAccountHandler.js'
import { setAccountHandler } from './setAccountHandler.js'
import { createBaseClient } from '@tevm/base-client'
import { EvmErrorMessage } from '@tevm/evm'
import { EthjsAddress, encodeFunctionData, hexToBytes } from '@tevm/utils'
import { EthjsAddress, encodeFunctionData, hexToBytes, parseEther, type Address } from '@tevm/utils'
import { describe, expect, it } from 'bun:test'
import { mineHandler } from './mineHandler.js'

Expand Down Expand Up @@ -432,4 +432,80 @@ describe('callHandler', () => {
selfdestruct: new Set(),
})
})

it('should be able to send multiple tx from same account and then mine it', async () => {
const client = createBaseClient()
const from = EthjsAddress.fromString(`0x${'69'.repeat(20)}`)
const to = `0x${'42'.repeat(20)}` as const
const { errors } = await setAccountHandler(client)({
address: from.toString() as Address,
nonce: 69n,
balance: parseEther('1000')
})
expect(errors).toBeUndefined()
// send value
expect(
await callHandler(client)({
createTransaction: true,
to,
value: 1n,
skipBalance: true,
from: from.toString() as Address,
}),
).toEqual({
executionGasUsed: 0n,
rawData: '0x',
txHash: "0xa5be8692fbb39d79a9d2aa2e87333d6620ceeec3cf52da8bef4d3dec3743145e",
})
expect(
await callHandler(client)({
createTransaction: true,
to,
value: 2n,
skipBalance: true,
from: from.toString() as Address,
}),
).toEqual({
executionGasUsed: 0n,
rawData: '0x',
txHash: "0xc4b3576c1bbdda23cf40aa5b6efe08d4c881d569820b6d996cfd611e323af6a9",
})
expect(
await callHandler(client)({
createTransaction: true,
to,
value: 3n,
skipBalance: true,
from: from.toString() as Address,
}),
).toEqual({
executionGasUsed: 0n,
rawData: '0x',
txHash: "0x27a596c1e6c26b8fc84f4dc07337b75300e29ab0ba5918fe7509414e62ff9fe9",
})
const txPool = await client.getTxPool()
// ts hashes are in pool
expect((await txPool.getBySenderAddress(from)).map(tx => tx.hash)).toEqual([
"a5be8692fbb39d79a9d2aa2e87333d6620ceeec3cf52da8bef4d3dec3743145e",
"c4b3576c1bbdda23cf40aa5b6efe08d4c881d569820b6d996cfd611e323af6a9",
"27a596c1e6c26b8fc84f4dc07337b75300e29ab0ba5918fe7509414e62ff9fe9"
])
await mineHandler(client)()
// the value should be sent
expect(
(
await getAccountHandler(client)({
address: to,
})
).balance,
).toEqual(1n + 2n + 3n)
// nonce should be increased by 3
expect(
(
await getAccountHandler(client)({
address: from.toString() as Address,
})
).nonce,
).toEqual(69n + 3n)
})
})
37 changes: 26 additions & 11 deletions packages/actions/src/tevm/createTransaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,24 +58,21 @@ export const createTransaction = (client, defaultThrowOnFail = true) => {

const sender = evmInput.origin ?? evmInput.caller ?? EthjsAddress.fromString(`0x${'00'.repeat(20)}`)

const txPool = await client.getTxPool()
const txs = await txPool.getBySenderAddress(sender)

// TODO known bug here we should be allowing unlimited code size here based on user providing option
// Just lazily not looking up how to get it from client.getVm().evm yet
// Possible we need to make property public on client
/**
* @param {bigint} gasLimit
*/
const getTx = async (gasLimit) => FeeMarketEIP1559Transaction.fromTxData(
const tx = FeeMarketEIP1559Transaction.fromTxData(
{
// TODO tevm_call should take nonce
// TODO should write tests that this works with multiple tx nonces
// TODO we should take into account the txPool already having tx in it with same nonce
nonce:
(
(await vm.stateManager.getAccount(
sender,
)) ?? { nonce: 0n }
).nonce,
gasLimit,
).nonce + BigInt(txs.length),
gasLimit: gasLimitWithExecutionBuffer,
maxFeePerGas: parentBlock.header.calcNextBaseFee() + priorityFee,
maxPriorityFeePerGas: 0n,
...(evmInput.to !== undefined ? { to: evmInput.to } : {}),
Expand All @@ -90,7 +87,6 @@ export const createTransaction = (client, defaultThrowOnFail = true) => {
freeze: false,
},
)
const tx = await getTx(gasLimitWithExecutionBuffer)
client.logger.debug(
tx,
'callHandler: Created a new transaction from transaction data',
Expand Down Expand Up @@ -145,6 +141,23 @@ export const createTransaction = (client, defaultThrowOnFail = true) => {
txHash
}
} catch (e) {
if (typeof e === 'object' && e !== null && '_tag' in e && e._tag === 'AccountNotFoundError') {
return maybeThrowOnFail(throwOnFail ?? defaultThrowOnFail, {
errors: [
{
name: 'NoBalanceError',
_tag: 'NoBalanceError',
message: 'Attempting to create a transaction with an uninitialized account with no balance',
},
],
executionGasUsed: 0n,
/**
* @type {`0x${string}`}
*/
rawData: '0x',
})

}
client.logger.error(
e,
'callHandler: Unexpected error adding transaction to mempool and checkpointing state. Removing transaction from mempool and reverting state',
Expand All @@ -162,7 +175,9 @@ export const createTransaction = (client, defaultThrowOnFail = true) => {
? e
: e instanceof Error
? e.message
: 'unknown error',
: typeof e === 'object' && e !== null && 'message' in e
? e.message
: 'unknown error',
},
],
executionGasUsed: 0n,
Expand Down
22 changes: 18 additions & 4 deletions packages/txpool/docs/classes/TxPool.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion packages/txpool/src/TxPool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
isFeeMarketEIP1559Tx,
isLegacyTx,
} from '@tevm/tx'
import { EthjsAccount, bytesToHex, bytesToUnprefixedHex, equalsBytes } from '@tevm/utils'
import { EthjsAccount, EthjsAddress, bytesToHex, bytesToUnprefixedHex, equalsBytes } from '@tevm/utils'
import type { Vm } from '@tevm/vm'

import type { Block } from '@tevm/block'
Expand Down Expand Up @@ -432,6 +432,11 @@ export class TxPool {
throw new Error(`tx of type ${(tx as TypedTransaction).type} unknown`)
}

async getBySenderAddress(address: EthjsAddress): Promise<Array<TxPoolObject>> {
const unprefixedAddress = address.toString().slice(2)
return this.pool.get(unprefixedAddress) ?? []
}

/**
* Returns eligible txs to be mined sorted by price in such a way that the
* nonce orderings within a single account are maintained.
Expand Down
Loading

0 comments on commit 2ba2c27

Please sign in to comment.