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

Implement missing logics in on_command function for TransactionsManager #4691

Closed
tcoratger opened this issue Sep 20, 2023 · 2 comments · Fixed by #4772
Closed

Implement missing logics in on_command function for TransactionsManager #4691

tcoratger opened this issue Sep 20, 2023 · 2 comments · Fixed by #4772
Labels
C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started

Comments

@tcoratger
Copy link
Contributor

tcoratger commented Sep 20, 2023

Describe the feature

This issue is created to track the implementation of missing logic in the TransactionsManager's on_command function in the project. The on_command function is responsible for handling various commands related to transaction management, and there are several functionalities that need to be implemented.

fn on_command(&mut self, cmd: TransactionsCommand) {
match cmd {
TransactionsCommand::PropagateHash(hash) => self.on_new_transactions(vec![hash]),
TransactionsCommand::PropagateHashTo(_hash, _peer) => todo!(),
TransactionsCommand::GetActivePeers => todo!(),
TransactionsCommand::PropagateTransactionsTo(_txs, _peer) => todo!(),
TransactionsCommand::GetTransactionHashes(_peers) => todo!(),
TransactionsCommand::GetPeerTransactionHashes(_peer) => todo!(),
}
}

The missing logic to be added includes handling commands to propagate transaction hashes, get active peers, propagate transactions to specific peers, get transaction hashes, and get peer transaction hashes. Implementing these functionalities will enhance the overall capabilities of the TransactionsManager module.

Additional context

No response

@tcoratger tcoratger added C-enhancement New feature or request S-needs-triage This issue needs to be labelled labels Sep 20, 2023
@mattsse mattsse added D-good-first-issue Nice and easy! A great choice to get started and removed S-needs-triage This issue needs to be labelled labels Sep 20, 2023
@SleepingShell
Copy link
Contributor

SleepingShell commented Sep 23, 2023

I would like to attempt implementing some of these. I've worked on PropagateTransactionsTo, am I correct this should have the same logic as this? Just destined for a single peer?

fn propagate_transactions(

@SleepingShell
Copy link
Contributor

@mattsse , I have added implementations of getting the peer's hashes and active peers to #4772 .

Please confirm that this functionality is to get the known tx hashes from the LRU cache. Since there is no eth message for requesting new transaction hashes, I am going to assume that this is the desired functionality.

Additionally, I don't see where this output is consumed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants