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: Feat/add on rpc server started hook #4947

Merged
merged 6 commits into from
Oct 10, 2023

Conversation

allnil
Copy link
Contributor

@allnil allnil commented Oct 7, 2023

Hey! @mattsse

I work on #4917

I really need your help!

I prepared a new function, I use the new type RethRpcComponents but I've got a borrowing issue on the line below which I don't know how to resolve in a clear manner YET:

cannot move out ofmodulesbecause it is borrowed move out ofmodulesoccurs hererustc[Click for full compiler diagnostic](rust-analyzer-diagnostics-view:/diagnostic%20message%20%5B0%5D?0#file%3A%2F%2F%2FUsers%2Fnil.medvedev%2FProjects%2Fmy%2Freth%2Fbin%2Freth%2Fsrc%2Fargs%2Frpc_server_args.rs) rpc_server_args.rs(206, 82): borrow ofmodulesoccurs here rpc_server_args.rs(198, 14): bindingmodulesdeclared here rpc_server_args.rs(233, 54): borrow later used here

https://github.com/allnil/reth/blob/2fc27313e4b2c381514168d2731af20eaa9c3c33/bin/reth/src/args/rpc_server_args.rs#L211C6-L211C6

I played with it in different ways but still don't know how to do it better
I will continue to work on it but maybe you could just give me a hint or show me a direction to proceed?
Thank you!

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.

this is great, only a few style nits

bin/reth/src/cli/ext.rs Outdated Show resolved Hide resolved
bin/reth/src/cli/ext.rs Outdated Show resolved Hide resolved
bin/reth/src/args/rpc_server_args.rs Outdated Show resolved Hide resolved
allnil and others added 2 commits October 9, 2023 16:52
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
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.

this is great, I only made one smol change by introducing a new type

@mattsse mattsse marked this pull request as ready for review October 10, 2023 14:15
@mattsse mattsse added M-changelog This change should be included in the changelog A-cli Related to the reth CLI labels Oct 10, 2023
@allnil
Copy link
Contributor Author

allnil commented Oct 10, 2023

aaa
got it
you resolved borrowings with clone
thank you!

@mattsse mattsse added this pull request to the merge queue Oct 10, 2023
Merged via the queue into paradigmxyz:main with commit c6531b4 Oct 10, 2023
22 checks passed
mattsse added a commit that referenced this pull request Nov 8, 2023
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Related to the reth CLI M-changelog This change should be included in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants