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.

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

feat(rpc): Add address_ namespace and bindings #4146

Closed
wants to merge 2 commits into from

Conversation

perama-v
Copy link

Description

Progress toward:

Changes

address_ namespace addition including:
- add AddressApi trait in rpc-api
- add to RPC builder
- add placeholder implementation for address_getAppearances

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

supportive, since this doesn't add any overhead,

but like to see some serde examples how the request response objects should look like

crates/rpc/rpc-api/src/address.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-api/src/address.rs Outdated Show resolved Hide resolved
@onbjerg onbjerg added C-enhancement New feature or request M-changelog This change should be included in the changelog A-rpc Related to the RPC implementation labels Aug 10, 2023
Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

Would want this to be documented fully in the book (e.g. in book/jsonrpc) in the same PR since it isn't a standard namespace

@perama-v
Copy link
Author

Would want this to be documented fully in the book (e.g. in book/jsonrpc) in the same PR since it isn't a standard namespace

Docs added and examples included that mirror the spec/spectests

@gakonst
Copy link
Member

gakonst commented Aug 14, 2023

Just to make this explicit would like to be sensitive with merging this upstream and maintaining the code..so I am happy we're iterating on it and would like to ideally figure out how to run it as a separate binary as well?

@perama-v
Copy link
Author

Just to make this explicit would like to be sensitive with merging this upstream and maintaining the code..so I am happy we're iterating on it and would like to ideally figure out how to run it as a separate binary as well?

Thanks, appreciate that.

I consider this family (4054) an exploration of "what could this feature look like, and how could it be implemented?", with the outcome being discovery of whether it is a good feature and what a good implementation could look like.

Happy to explore the design space including specific ideas anyone has.

Could you expand the concept of a separate binary more? On first take it sounds like you are describing the following:

Currently, the Execution stage has inspectors that can gather data. The result of the inspector could be stored directly in to a table alongside other tables in reth, alternatively it could call a separate binary and pass the inspector data over. The separate binary listens for and accepts the data, processes and stores in some representation that reth is unaware of. Then to serve the method reth and the separate binary again call each other (depending on where the call is set up to be served from) and the data aggregated for the response.

@perama-v perama-v marked this pull request as draft August 16, 2023 01:44
@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Oct 16, 2023
@rkrasiuk rkrasiuk added M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity and removed S-stale This issue/PR is stale and will close with no further activity labels Oct 19, 2023
@gakonst
Copy link
Member

gakonst commented Jan 9, 2024

Closing this as stale, we'll re-incorporate these ideas as we venture into custom stages with Reth CLI Extensions and the Declarative Node Builder API that @mattsse is spearheading.

@gakonst gakonst closed this Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request M-changelog This change should be included in the changelog M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants