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

networking::TransactionPool should accept Arc. #5978

Open
Tracked by #5472
michalkucharczyk opened this issue Oct 8, 2024 · 4 comments · May be fixed by #7042
Open
Tracked by #5472

networking::TransactionPool should accept Arc. #5978

michalkucharczyk opened this issue Oct 8, 2024 · 4 comments · May be fixed by #7042
Assignees
Labels
T0-node This PR/Issue is related to the topic “node”.

Comments

@michalkucharczyk
Copy link
Contributor

          We should not do this here. Please create a follow up. The `TransactionPool` trait from networking should also just take an `Arc`. No need to clone the pure tx data for sending/encoding it.

Originally posted by @bkchr in #4639 (comment)

@dharjeezy
Copy link
Contributor

Hello @michalkucharczyk

should all functions defined in the pub trait TransactionPool<H: ExHashT, B: BlockT>: Send + Sync take an Arc of transaction?

@bkchr
Copy link
Member

bkchr commented Dec 28, 2024

Yes.

@dharjeezy dharjeezy linked a pull request Jan 4, 2025 that will close this issue
@michalkucharczyk
Copy link
Contributor Author

For the record: #7042 (comment)

@bkchr
Copy link
Member

bkchr commented Jan 6, 2025

Yeah, apparently I did not had checked the trait properly :P

@michalkucharczyk michalkucharczyk added the T0-node This PR/Issue is related to the topic “node”. label Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants