Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardize 'txpool' namespace #353

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Standardize 'txpool' namespace #353

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7234967
add txpool namespace
wslyvh Jan 11, 2023
a79f132
add tests
wslyvh Jan 13, 2023
8b85ed8
fix json format
wslyvh Jan 13, 2023
60bdeb5
fix spec types
wslyvh Jan 23, 2023
1870c1c
add txpool to dictionary
wslyvh Jan 23, 2023
a87f507
update txpool dictionary
wslyvh Jan 24, 2023
78e1869
update filter spec
wslyvh Jan 24, 2023
7c4b774
add maxFeePerGas and maxPriorityFeePerGas
wslyvh Aug 21, 2023
638a270
update one-time schemas
wslyvh Aug 21, 2023
473c5c7
Merge remote-tracking branch 'upstream/main'
wslyvh Aug 21, 2023
5ba9ddd
remove maxsize
wslyvh Aug 29, 2023
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
12 changes: 12 additions & 0 deletions scripts/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,18 @@ methodFiles.forEach(file => {
];
});

methodsBase = "src/txpool/";
methodFiles = fs.readdirSync(methodsBase);
methodFiles.forEach(file => {
console.log(file);
let raw = fs.readFileSync(methodsBase + file);
let parsed = yaml.load(raw);
methods = [
...methods,
...parsed,
];
});

let schemas = {};
let schemasBase = "src/schemas/"
let schemaFiles = fs.readdirSync(schemasBase);
Expand Down
10 changes: 10 additions & 0 deletions src/schemas/filter.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,13 @@ FilterTopic:
type: array
items:
$ref: '#/components/schemas/bytes32'
FilterOperators:
title: operator
type: object
properties:
eq:
$ref: '#/components/schemas/uint'
gt:
$ref: '#/components/schemas/uint'
lt:
$ref: '#/components/schemas/uint'
69 changes: 69 additions & 0 deletions src/txpool/getters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
- name: txpool_transactions
summary: Returns a list of all transactions that match the supplied filter conditions that are either pending for inclusion in the next block(s) or scheduled for future execution.
params:
- name: Filter
schema:
title: txpool filter
Copy link

Choose a reason for hiding this comment

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

Having now different kind of txs, probably a type filter could be useful

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean 4844/blobs transactions? if so, also see other comment that from my understanding 4844/blobs will have its own transaction pool and will not be included for this proposal.

Copy link

Choose a reason for hiding this comment

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

I mean that the filter could accept a type field where you can specify any of the transaction types: Frontier, EIP2930, EIP1559, EIP4844

type: object
properties:
from:
title: from address
$ref: '#/components/schemas/address'
to:
title: to address
$ref: '#/components/schemas/address'
gas:
title: gas limit
$ref: '#/components/schemas/FilterOperators'
gasPrice:
title: gas price
$ref: '#/components/schemas/FilterOperators'
maxFeePerGas:
title: max fee
$ref: '#/components/schemas/FilterOperators'
maxPriorityFeePerGas:
title: max priority fee
$ref: '#/components/schemas/FilterOperators'
value:
title: value
$ref: '#/components/schemas/FilterOperators'
nonce:
title: nonce
$ref: '#/components/schemas/FilterOperators'
result:
name: Result
schema:
title: Result
schema:
$ref: '#/components/schemas/TxPoolTransactions'
title: transactions
type: object
properties:
pending:
title: pending
Copy link

Choose a reason for hiding this comment

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

If not yet present elsewhere, I think we should define which criteria make a tx pending and queue, to avoid interpretations that could results in different implementations

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you know if this is specified somewhere already?
From my understanding pending means it's ready to be included in the next block
Queued is for the ones which are planned for future processing.

Copy link

Choose a reason for hiding this comment

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

I am thinking about the distinction of the two sets, so what makes a tx ready to be included or not.
At first it seems easier to find the reasons why a tx in pool is not ready for inclusion, and the main reasons are:

  • a previous tx is missing for the sender (nonce gap)
  • not enough funds to pay for the gas

at the moment I only see these two, but I could miss something.

type: array
items:
$ref: '#/components/schemas/GenericTransaction'
Copy link

Choose a reason for hiding this comment

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

Just to start a discussion, since specially with EIP-4844, we could have quite big network representation (that includes blob content) of a tx, so a way to select what to return and/or to limit the amount of data returned should be considered.

Copy link
Member Author

Choose a reason for hiding this comment

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

From my understanding, 4844/blobs will have its own transaction pool and do not use the generic tx pool. So not sure if this should be part of this proposal.

Copy link

Choose a reason for hiding this comment

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

Quoting the previous answer, where I think blob txs should be included, setting some kind of limits make sense

queued:
title: queued
type: array
items:
$ref: '#/components/schemas/GenericTransaction'
- name: txpool_statistics
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: txpool_statistics
- name: txpool_stats

Copy link
Member Author

Choose a reason for hiding this comment

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

I think introducing txpool_stats next to the existing txpool_status might cause confusion and potential typos/errors.
txpool_statistics would avoid that

But again, no strong opinion

summary: Returns statistics about the node's transaction pool.
params: []
result:
name: Statistics
schema:
title: statistics
type: object
properties:
maxSize:
wslyvh marked this conversation as resolved.
Show resolved Hide resolved
title: max size
$ref: '#/components/schemas/uint'
pending:
title: pending
$ref: '#/components/schemas/uint'
queued:
title: queued
$ref: '#/components/schemas/uint'
2 changes: 2 additions & 0 deletions tests/txpool_statistics/get-statistics.io
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
>> {"jsonrpc":"2.0","id":31,"method":"txpool_statistics"}
<< {"jsonrpc":"2.0","id":31,"result":{"maxSize":"0x1","pending":"0x1","queued":"0x1"}}
2 changes: 2 additions & 0 deletions tests/txpool_transactions/get-default-transactions.io
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
>> {"jsonrpc":"2.0","id":31,"method":"txpool_transactions"}
Copy link
Member

Choose a reason for hiding this comment

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

So I think what we should here for the tests is have two eth_sendRawTransactions before txpool_transactions so it actually seeds the pool with the txs. This way we can still use these tests in hive to verify the response of each client. You can do this by defining the interaction as a test generator here: https://github.com/lightclient/rpctestgen/blob/main/testgen/generators.go

<< {"jsonrpc":"2.0","id":31,"result":{"pending":[{"blockHash":null,"blockNumber":null,"from":"0x658bdf435d810c91414ec09147daa6db62406379","gas":"0x5208","gasPrice":"0x342770c1","hash":"0x74e41d593675913d6d5521f46523f1bd396dff1891bdb35f59be47c7e5e0b34b","input":"0x","nonce":"0x0","to":"0x658bdf435d810c91414ec09147daa6db62406379","transactionIndex":"0x0","value":"0x3e8","type":"0x0","chainId":"0x539","v":"0xa95","r":"0xaf5fc351b9e457a31f37c84e5cd99dd3c5de60af3de33c6f4160177a2c786a60","s":"0x201da7a21046af55837330a2c52fc1543cd4d9ead00ddf178dd96935b607ff9b"}],"queued":[{"blockHash":null,"blockNumber":null,"from":"0x658bdf435d810c91414ec09147daa6db62406379","gas":"0x5208","gasPrice":"0x342770c1","hash":"0x74e41d593675913d6d5521f46523f1bd396dff1891bdb35f59be47c7e5e0b34b","input":"0x","nonce":"0x0","to":"0x658bdf435d810c91414ec09147daa6db62406379","transactionIndex":"0x0","value":"0x3e8","type":"0x0","chainId":"0x539","v":"0xa95","r":"0xaf5fc351b9e457a31f37c84e5cd99dd3c5de60af3de33c6f4160177a2c786a60","s":"0x201da7a21046af55837330a2c52fc1543cd4d9ead00ddf178dd96935b607ff9b"}]}}
2 changes: 2 additions & 0 deletions tests/txpool_transactions/get-empty-transactions-list.io
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
>> {"jsonrpc":"2.0","id":31,"method":"txpool_transactions"}
<< {"jsonrpc":"2.0","id":31,"result":{"pending":[],"queued":[]}}
2 changes: 2 additions & 0 deletions tests/txpool_transactions/get-filtered-transactions-by-address.io
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
>> {"jsonrpc":"2.0","id":31,"method":"txpool_transactions","params":[{"from":"0xfe3b557e8fb62b89f4916b721be55ceb828dbd73"}]}
<< {"jsonrpc":"2.0","id":31,"result":{"pending":[{"blockHash":null,"blockNumber":null,"from":"0xfe3b557e8fb62b89f4916b721be55ceb828dbd73","gas":"0x5208","gasPrice":"0x342770c1","hash":"0x74e41d593675913d6d5521f46523f1bd396dff1891bdb35f59be47c7e5e0b34b","input":"0x","nonce":"0x0","to":"0x658bdf435d810c91414ec09147daa6db62406379","transactionIndex":"0x0","value":"0x3e8","type":"0x0","chainId":"0x539","v":"0xa95","r":"0xaf5fc351b9e457a31f37c84e5cd99dd3c5de60af3de33c6f4160177a2c786a60","s":"0x201da7a21046af55837330a2c52fc1543cd4d9ead00ddf178dd96935b607ff9b"}],"queued":[{"blockHash":null,"blockNumber":null,"from":"0xfe3b557e8fb62b89f4916b721be55ceb828dbd73","gas":"0x5208","gasPrice":"0x342770c1","hash":"0x74e41d593675913d6d5521f46523f1bd396dff1891bdb35f59be47c7e5e0b34b","input":"0x","nonce":"0x0","to":"0x658bdf435d810c91414ec09147daa6db62406379","transactionIndex":"0x0","value":"0x3e8","type":"0x0","chainId":"0x539","v":"0xa95","r":"0xaf5fc351b9e457a31f37c84e5cd99dd3c5de60af3de33c6f4160177a2c786a60","s":"0x201da7a21046af55837330a2c52fc1543cd4d9ead00ddf178dd96935b607ff9b"}]}}
2 changes: 2 additions & 0 deletions tests/txpool_transactions/get-filtered-transactions-gt-gas.io
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
>> {"jsonrpc":"2.0","id":31,"method":"txpool_transactions","params":[{"gas":{"lt":"0x0"}}]}
<< {"jsonrpc":"2.0","id":31,"result":{"pending":[{"blockHash":null,"blockNumber":null,"from":"0xfe3b557e8fb62b89f4916b721be55ceb828dbd73","gas":"0x5208","gasPrice":"0x342770c1","hash":"0x74e41d593675913d6d5521f46523f1bd396dff1891bdb35f59be47c7e5e0b34b","input":"0x","nonce":"0x0","to":"0x658bdf435d810c91414ec09147daa6db62406379","transactionIndex":"0x0","value":"0x3e8","type":"0x0","chainId":"0x539","v":"0xa95","r":"0xaf5fc351b9e457a31f37c84e5cd99dd3c5de60af3de33c6f4160177a2c786a60","s":"0x201da7a21046af55837330a2c52fc1543cd4d9ead00ddf178dd96935b607ff9b"}],"queued":[{"blockHash":null,"blockNumber":null,"from":"0xfe3b557e8fb62b89f4916b721be55ceb828dbd73","gas":"0x5208","gasPrice":"0x342770c1","hash":"0x74e41d593675913d6d5521f46523f1bd396dff1891bdb35f59be47c7e5e0b34b","input":"0x","nonce":"0x0","to":"0x658bdf435d810c91414ec09147daa6db62406379","transactionIndex":"0x0","value":"0x3e8","type":"0x0","chainId":"0x539","v":"0xa95","r":"0xaf5fc351b9e457a31f37c84e5cd99dd3c5de60af3de33c6f4160177a2c786a60","s":"0x201da7a21046af55837330a2c52fc1543cd4d9ead00ddf178dd96935b607ff9b"}]}}
5 changes: 5 additions & 0 deletions wordlist.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,8 @@ randao
src
https
forkchoiceupdatedresponsev
txpool
TxPool
eq
lt
gt