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

feat: Add txpool_content RPC method #1539

Merged
merged 2 commits into from
Jan 4, 2022
Merged

feat: Add txpool_content RPC method #1539

merged 2 commits into from
Jan 4, 2022

Conversation

domob1812
Copy link
Contributor

@domob1812 domob1812 commented Nov 11, 2021 •
edited
Loading

This adds support for the txpool_content RPC method, which is a Geth extension to the JSON-RPC API. For more details, see the specification at https://geth.ethereum.org/docs/rpc/ns-txpool.

This implements a part of #763.

@domob1812
Copy link
Contributor Author

I'm not very experienced with TypeScript, so please let me know of any coding style or other issues and I'll be happy to fix them.

@davidmurdoch
Copy link
Member

At a quick glance this looks really great! We've got a busy week this week putting out a new alpha release, but hopefully we can review further and merge this next week!

@L-F-Escobar
Copy link

Hey y'all! Wonder if this mr will address a similar issue I am runnig into when using web3.py & a local ganache node.

w3 = Web3(HTTPProvider("http://127.0.0.1:7545"))
w3.geth.txpool.content()
w3.geth.txpool.status()

yields this error

*** ValueError: {'message': 'Method txpool_content not supported.', 'code': -32000, 'data': {'stack': 'Error: Method txpool_content not supported.\

@L-F-Escobar L-F-Escobar mentioned this pull request Nov 20, 2021
Closed
@davidmurdoch
Copy link
Member

@L-F-Escobar yes, this addresses txpool.content()

MicaiahReid
MicaiahReid suggested changes Dec 1, 2021
View reviewed changes
Copy link
Contributor

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @domob1812! Really great work.

I've left a few small suggestions, and there are a some merge conflicts that need to be resolved. Please let me or @davidmurdoch know if you have any questions or if you won't have time for this. We could get this past the finish line for you if you needed.

Thanks again, this is a huge help to us and those in the community wanting this RPC method.

src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
@domob1812 domob1812 force-pushed the txpool-content branch 2 times, most recently from 3d2b84e to aadec27 Compare December 2, 2021 08:49
@domob1812
Copy link
Contributor Author

Thanks for the suggestions! I've applied them and then rebased onto latest develop to resolve the conflict (and squash the follow-up commits in for a clean history).

davidmurdoch
davidmurdoch suggested changes Dec 6, 2021
View reviewed changes
Copy link
Member

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

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

This looks really great!

I have a few suggestions and a request for extending the tests a little bit.

Thanks again for putting this together!

src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/src/api.ts Outdated Show resolved Hide resolved
src/chains/ethereum/ethereum/tests/api/txpool/content.test.ts Show resolved Hide resolved
@domob1812
feat: Add txpool_content RPC method.
b1de189 
This adds support for the txpool_content RPC method, which is a Geth
extension to the JSON-RPC API.  For more details, see the specification
at https://geth.ethereum.org/docs/rpc/ns-txpool.
@domob1812
Copy link
Contributor Author

Thanks again for the valuable feedback! I've applied all suggestions and tried to address the other comments now.

@davidmurdoch davidmurdoch self-requested a review December 17, 2021 17:31
@davidmurdoch davidmurdoch merged commit bdf6a64 into trufflesuite:develop Jan 4, 2022
@domob1812 domob1812 deleted the txpool-content branch January 5, 2022 05:48
amandesai01
amandesai01 reviewed Sep 23, 2022
View reviewed changes
src/chains/ethereum/ethereum/src/api.ts
Comment on lines +3157 to +3173
const processMap = (map: Map<string, Heap<TypedTransaction>>) => {
let res = new Map<string, Map<string, TypedTransactionJSON>>();
for (let [_, transactions] of map) {
const arr = transactions.array;
for (let i = 0; i < transactions.length; ++i) {
const tx = arr[i];
const from = tx.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 = tx.nonce.toBigInt().toString();
res[from][nonce] = tx.toJSON(common);
}
}
return res;
Copy link

@amandesai01 amandesai01 Sep 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Regarding to #763

Can this be shifted to a separate utility? since other txpool_* methods might need it. If yes, which place is most suitable?

@amandesai01 amandesai01 mentioned this pull request Sep 23, 2022
Open
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@amandesai01 amandesai01 amandesai01 left review comments

@MicaiahReid MicaiahReid MicaiahReid approved these changes

@davidmurdoch davidmurdoch davidmurdoch approved these changes

@cds-amal cds-amal Awaiting requested review from cds-amal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@domob1812 @davidmurdoch @L-F-Escobar @MicaiahReid @amandesai01