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

ignore known tx in pending tx subscription #595

Merged
merged 3 commits into from
Jul 14, 2023

Conversation

libotony
Copy link
Member

No description provided.

@qianbin
Copy link
Member

qianbin commented Jul 12, 2023

I think per-connection LRU is more reasonable, or let the client do the filtering.

@libotony
Copy link
Member Author

libotony commented Jul 12, 2023

I think per-connection LRU is more reasonable, or let the client do the filtering.

Client filtering might not be a good idea, since duplicated tx happens a lot, caused by multiple peer-to-peer node broadcasting new tx.

As a hub for re-distributing events, it might be efficient to filter in place. Per-connection LRU is perfect at the design level, but it might cost more memories, do you consider it affordable?

@qianbin
Copy link
Member

qianbin commented Jul 12, 2023

As a hub for re-distributing events, it might be efficient to filter in place. Per-connection LRU is perfect at the design level, but it might cost more memories, do you consider it affordable?

Sorry, I misunderstood the code.

A few minor changes are made within the last two commits.

@libotony
Copy link
Member Author

As a hub for re-distributing events, it might be efficient to filter in place. Per-connection LRU is perfect at the design level, but it might cost more memories, do you consider it affordable?

Sorry, I misunderstood the code.

A few minor changes are made within the last two commits.

Changes LGTM, still needs your review, I can't review it due to I am the author.

@libotony libotony merged commit e664012 into vechain:master Jul 14, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants