-
Notifications
You must be signed in to change notification settings - Fork 38.8k
Skip processing of TXs that we already have. #9414
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
Conversation
Previously TXs were processed more than they needed to be even though we already have them.
| EraseOrphanTx(hash); | ||
| } | ||
| } // if accepted to memory pool | ||
| else if (fMissingInputs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for fMissingInputs to be true AlreadyHave() had to be false
| LogPrint("mempool", "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString()); | ||
| } | ||
| } else { | ||
| } else { // not accepted and not missing inputs, i.e. invalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whereas here AlreadyHave() could have been true still, so for whitelisted nodes, the same tx could be relayed multiple times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is the intended and advertised purpose of the whitelisting, read the comment in the else block!
|
&& is shortcutting so this doesn't change it to skip AcceptToMemoryPool. It does however change it so that it does not run the else clause which is needed for the forced relaying for whitelisted peers (which needs to run already for transactions we already have). So I think the only real behavior change this makes is to break the functionality. |
|
@gmaxwell Ah, yes, my apologies I has missed the whitelist intention there. It was primarily noticing that I am not aware yet of why we would want to relay txs (possibly multiple times) to whitelisted nodes that we already have in our mempool though - is this requirement documented somewhere please? |
Previously TXs were processed more than they needed to be even though we already have them.