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

Add new helper trait for Network to get access to TransactionsHandle #5598

Closed
mattsse opened this issue Nov 28, 2023 · 12 comments · Fixed by #6780
Closed

Add new helper trait for Network to get access to TransactionsHandle #5598

mattsse opened this issue Nov 28, 2023 · 12 comments · Fixed by #6780
Labels
A-networking Related to networking in general C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity

Comments

@mattsse
Copy link
Collaborator

mattsse commented Nov 28, 2023

Describe the feature

The TransactionsHandle provides useful functions to interact with the network:

impl TransactionsHandle {

It would be useful to get a TransactionsHandle directly from the

impl NetworkHandle {

TODO

  • add new helper trait: EthNetwork that provides a function to access the TransactionsHandle. This should be solved by adding a Mutex<Option<TransactionHandle>> to the NetworkInner
    struct NetworkInner {

    so it can be set once the transactionmanager is installed:

pub fn set_transactions(&mut self, tx: mpsc::UnboundedSender<NetworkTransactionEvent>) {

see for example:

let (tx, rx) = mpsc::unbounded_channel();
network.set_transactions(tx);
let handle = network.handle().clone();
let transactions = TransactionsManager::new(handle, pool, rx);

then we can also move these functions to the trait:

/// Sends a [`PeerRequest`] to the given peer's session.
pub fn send_request(&self, peer_id: PeerId, request: PeerRequest) {
self.send_message(NetworkHandleMessage::EthRequest { peer_id, request })
}
/// Send transactions hashes to the peer.
pub fn send_transactions_hashes(&self, peer_id: PeerId, msg: NewPooledTransactionHashes) {
self.send_message(NetworkHandleMessage::SendPooledTransactionHashes { peer_id, msg })
}
/// Send full transactions to the peer
pub fn send_transactions(&self, peer_id: PeerId, msg: Vec<Arc<TransactionSigned>>) {
self.send_message(NetworkHandleMessage::SendTransaction {
peer_id,
msg: SharedTransactions(msg),
})
}

Additional context

No response

@mattsse mattsse added C-enhancement New feature or request S-needs-triage This issue needs to be labelled D-good-first-issue Nice and easy! A great choice to get started A-networking Related to networking in general and removed S-needs-triage This issue needs to be labelled labels Nov 28, 2023
@DoTheBestToGetTheBest
Copy link
Contributor

happy to take this :) ty for all this details !

@i-m-aditya
Copy link
Contributor

@DoTheBestToGetTheBest I have already created a PR, so if you haven't started, let me know :)

@DoTheBestToGetTheBest
Copy link
Contributor

@DoTheBestToGetTheBest I have already created a PR, so if you haven't started, let me know :)

i still didn't started working on this but this is my issue so i will work on it, thanks

@mattsse
Copy link
Collaborator Author

mattsse commented Nov 29, 2023

please be excellent to each other :)

@i-m-aditya I can find you something else to work on after #5604

@i-m-aditya
Copy link
Contributor

@mattsse Absolutely, let's keep the good vibes going! 😄

@daniel-huertas
Copy link

I'm eager to try this challenge !

@DoTheBestToGetTheBest
Copy link
Contributor

I'm eager to try this challenge !

please choose another thing, this is my issue, i will work on it when i'am free :)

i don't not abandon this :)

ty for your understand

@mattsse
Copy link
Collaborator Author

mattsse commented Feb 1, 2024

@DoTheBestToGetTheBest are you still on this?

@DoTheBestToGetTheBest
Copy link
Contributor

@DoTheBestToGetTheBest are you still on this?

i was going to work on it last week but since i can't use reth for not ( related with this #6258 ) i'm unable to work on this until fix

probably someone should work on it i don't know how much time it would except for fixing CD

@DoTheBestToGetTheBest DoTheBestToGetTheBest removed their assignment Feb 1, 2024
@DoTheBestToGetTheBest
Copy link
Contributor

@i-m-aditya @daniel-huertas hey feel free to take this if you want, i'm unable to work with reth for the moment until fix

Copy link
Contributor

This issue is stale because it has been open for 21 days with no activity.

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Feb 23, 2024
@mattsse mattsse 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 Feb 23, 2024
@qiweiii
Copy link
Contributor

qiweiii commented Feb 24, 2024

I can try to work on this, but please don't assign me, if I can do it, there will be a PR in 24 hrs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Related to networking in general C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants