Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

feat: Add remaining txpool methods #3719

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
94 changes: 69 additions & 25 deletions src/chains/ethereum/ethereum/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3426,39 +3426,83 @@ export default class EthereumApi implements Api {
*/
@assertArgLength(0)
async txpool_content(): Promise<Ethereum.Pool.Content<"private">> {
const { transactions, common } = this.#blockchain;
const { transactions } = this.#blockchain;
const {
transactionPool: { executables, origins }
transactionPool: { executables, origins, processMap }
} = transactions;

const processMap = (map: Map<string, Heap<TypedTransaction>>) => {
let res: Record<
string,
Record<string, Ethereum.Pool.Transaction<"private">>
> = {};
for (let [_, { array, length }] of map) {
for (let i = 0; i < length; ++i) {
const transaction = array[i];
const from = transaction.from.toString();
if (res[from] === undefined) {
res[from] = {};
}
// The nonce keys are actual decimal numbers (as strings) and not
// hex literals (based on what geth returns).
const nonce = transaction.nonce.toBigInt().toString();
res[from][nonce] = transaction.toJSON(
common
) as Ethereum.Pool.Transaction<"private">;
}
}
return res;
};

return {
pending: processMap(executables.pending),
queued: processMap(origins)
};
}


/**
* Returns transactions created by the specified address currently pending inclusion in the next blocks, as well as the ones that are scheduled for future execution.
*
* @param address - The account address
* @returns The transactions currently pending or queued in the transaction pool by address.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @returns The transactions currently pending or queued in the transaction pool by address.
* @returns The transactions from the specified address that are currently pending or queued in the transaction pool.

* @example
* ```javascript
Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason these examples aren't working in the generated docs - but the txpool_content example isn't working either. I can look into this!

Copy link
Contributor

Choose a reason for hiding this comment

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

I found it!
We'll need to add this to the top of all three txpool examples to keep the transactions from being mined so they actually stay in the pool:

await provider.send("miner_stop")

Also, for the txpool_contentFrom example can we send another transaction from a different address to showcase the fact that it filters by address?

* const [from] = await provider.request({ method: "eth_accounts", params: [] });
* const pendingTx = await provider.request({ method: "eth_sendTransaction", params: [{ from, gas: "0x5b8d80", nonce:"0x0" }] });
* const queuedTx = await provider.request({ method: "eth_sendTransaction", params: [{ from, gas: "0x5b8d80", nonce:"0x2" }] });
* const pool = await provider.send("txpool_contentFrom", [from]);
* console.log(pool);
* ```
*/
@assertArgLength(1)
async txpool_contentFrom(address: DATA): Promise<Ethereum.Pool.Content<"private">> {
const { transactions } = this.#blockchain;
const {
transactionPool: { executables, origins, processMap }
} = transactions;

const fromAddress = Address.from(address);

return {
pending: processMap(executables.pending, fromAddress),
queued: processMap(origins, fromAddress)
};
}

/**
* Returns the number of transactions currently pending for inclusion in the next block(s), as well as the ones that are being scheduled for future execution only.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Returns the number of transactions currently pending for inclusion in the next block(s), as well as the ones that are being scheduled for future execution only.
* Returns the number of transactions currently pending inclusion in the next blocks, as well as the ones that are scheduled for future execution.

*
* @returns The count of transactions currently pending or queued in the transaction pool.
* @example
* ```javascript
* const [from] = await provider.request({ method: "eth_accounts", params: [] });
* const pendingTx = await provider.request({ method: "eth_sendTransaction", params: [{ from, gas: "0x5b8d80", nonce:"0x0" }] });
* const queuedTx = await provider.request({ method: "eth_sendTransaction", params: [{ from, gas: "0x5b8d80", nonce:"0x2" }] });
* const pool = await provider.send("txpool_status");
* console.log(pool);
* ```
*/
@assertArgLength(0)
async txpool_status(): Promise<{pending: number, queued: number}> {
const { transactions } = this.#blockchain;
const {
transactionPool: { executables, origins }
} = transactions;

let pending = 0;
let queued = 0;

for (const [_, transactions] of executables.pending) {
pending += transactions?.array.length || 0;
}

for (const [_, transactions] of origins) {
queued += transactions?.array.length || 0;
}

return {
pending,
queued
};
}

//#endregion
}
29 changes: 29 additions & 0 deletions src/chains/ethereum/ethereum/src/transaction-pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
import { EthereumInternalOptions } from "@ganache/ethereum-options";
import { Executables } from "./miner/executables";
import { TypedTransaction } from "@ganache/ethereum-transaction";
import { Ethereum } from "./api-types";
import { Address } from "@ganache/ethereum-address";

/**
* Checks if the `replacer` is eligible to replace the `replacee` transaction
Expand Down Expand Up @@ -453,6 +455,33 @@ export default class TransactionPool extends Emittery<{ drain: undefined }> {
return null;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll need a js-doc comment here. Could you add something like this? (feel free to refine)

Suggested change
/**
* Iterates over all transactions in each Heap for each key in the map.
* Returns the transactions grouped by origin and nonce in the format: `{
* origin1: { nonceA: ...txData, ... }, ...}`
*
* If `address` is specified, filters the result to only show transactions
* from `address`
* @param map
* @param address
* @returns Transactions grouped by origin and nonce.
*/

public processMap(map: Map<string, Heap<TypedTransaction>>, address?: Address) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably should have come up with a better name for this function when we first reviewed it. But especially now that it's being used elsewhere, I think we should rename this. The trouble is deciding on the name! Perhaps groupTransactionsByOriginAndNonce? @davidmurdoch opinions?

let res: Record<
string,
Record<string, Ethereum.Pool.Transaction<"private">>
> = {};

for (let [_, { array, length }] of map) {
for (let i = 0; i < length; ++i) {
const transaction = array[i];

if(address && transaction.from.toString() != address.toString()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We're getting transaction.from.toString() twice, could we store this as a variable?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also store address.toString() outside of the loop so it's only computed once?

continue;
}

const from = transaction.from.toString();
if (res[from] === undefined) {
res[from] = {};
}
// The nonce keys are actual decimal numbers (as strings) and not
// hex literals (based on what geth returns).
const nonce = transaction.nonce.toBigInt().toString();
res[from][nonce] = transaction.toJSON() as Ethereum.Pool.Transaction<"private">;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we will still need to pass in a Common to the transaction's toJSON field. This is used in the legacy transaction type to determine if the user is running Ganache with EIP-2718 activated. If not, transaction types did not yet exist and should not be included on the return object.

}
}
return res;
};

readonly drain = () => {
// notify listeners (the blockchain, then the miner, eventually) that we
// have executable transactions ready
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,4 +120,110 @@ describe("txpool", () => {
assert.deepStrictEqual(queued, {});
});
});

Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason github isn't formatting this file as typescript. I wonder what's going on with that 🤔

describe("contentFrom", () => {
let provider: EthereumProvider;
let accounts: string[];

beforeEach(async () => {
provider = await getProvider({
miner: { blockTime: 1000 }
});
accounts = await provider.send("eth_accounts");
});

it("handles pending and queued transactions", async () => {
const pendingTransactions = await Promise.all([
provider.send("eth_sendTransaction", [
{
from: accounts[1],
to: accounts[2]
}
]),
provider.send("eth_sendTransaction", [
{
from: accounts[2],
to: accounts[3]
}
]),
provider.send("eth_sendTransaction", [
{
from: accounts[1],
to: accounts[2]
}
])
]);

const queuedTransactions = await Promise.all([
provider.send("eth_sendTransaction", [
{
from: accounts[1],
to: accounts[2],
nonce: "0x123",
}
])
]);

const { pending, queued } = await provider.send("txpool_contentFrom", [accounts[1]]);

const pendingAddresses = Object.keys(pending);
const queuedAddresses = Object.keys(queued);

assert(pendingAddresses.findIndex((value) => value === accounts[1]) == 0)
assert(pendingAddresses.findIndex((value) => value === accounts[2]) == -1)
assert(pendingAddresses.length == 1)

assert(queuedAddresses.findIndex((value) => value === accounts[1]) == 0)
assert(queuedAddresses.findIndex((value) => value === accounts[2]) == -1)
assert(queuedAddresses.length == 1)
})
})

describe("status", () => {
let provider: EthereumProvider;
let accounts: string[];
beforeEach(async () => {
provider = await getProvider({
miner: { blockTime: 1000 }
});
accounts = await provider.send("eth_accounts");
});

it("handles pending and queued transactions", async () => {
const pendingTransactions = await Promise.all([
provider.send("eth_sendTransaction", [
{
from: accounts[1],
to: accounts[2]
}
]),
provider.send("eth_sendTransaction", [
{
from: accounts[2],
to: accounts[3]
}
]),
provider.send("eth_sendTransaction", [
{
from: accounts[1],
to: accounts[2]
}
])
]);

const queuedTransactions = await Promise.all([
provider.send("eth_sendTransaction", [
{
from: accounts[1],
to: accounts[2],
nonce: "0x123",
}
])
]);

const { pending, queued } = await provider.send("txpool_status");
assert.strictEqual(pending, pendingTransactions.length);
assert.strictEqual(queued, queuedTransactions.length);
});
});
});