Keep track of recently rejected transactions. #484
Conversation
| // limitNumRejectedTxns limits the number of rejected transactions by evicting | ||
| // a random rejected transaction if adding a new one would cause it to overflow | ||
| // the max allowed. | ||
| func (b *blockManager) limitNumRejectedTxns() error { |
There was a problem hiding this comment.
I'd prefer this be defined above the only function it's called from so it's consistent with the vast majority of the code base.
|
Other than my comment this looks good. I'll do some manual testing on it as well. |
|
I will wait on this as I'd like to add limits to requestedTxns and requestedBlocks as well. |
|
I wonder if this should wait til after the #445? |
| delete(tmsg.peer.requestedTxns, *txHash) | ||
| delete(b.requestedTxns, *txHash) | ||
|
|
||
| if err != nil { | ||
| // Do not request this transactions again until a new block |
There was a problem hiding this comment.
We're working with one tx here. Update comment to reflect. transactions transaction.
|
#445 has been merged. Please update this accordingly. |
|
rebased. |
This prevents the node from repeatedly requesting and rejecting the same transaction as different peers inv the same transaction. Idea from Bitcoin Core commit 0847d9cb5fcd2fdd5a21bde699944d966cf5add9 Also, limit the number of both requested blocks and transactions.
| if len(m)+1 > limit { | ||
| // Generate a cryptographically random hash. | ||
| randHashBytes := make([]byte, wire.HashSize) | ||
| _, err := rand.Read(randHashBytes) |
There was a problem hiding this comment.
So, I'm not sure we really need strong cryptographic randomness here. I suspect reading from /dev/urandom may become an unnecessary bottleneck when processing transactions. In order to speed up the execution of this function, we may want to instead use a fast PRNG such as xorshift. There exists a pure-go implementation, which can be found here.
NOTE: I haven't benchmarked this yet, but this is a recommendation based on past experience when optimizing the SigCache in #598.
There was a problem hiding this comment.
Yeah, you have a fair point and I think that's probably a reasonable place to look at for optimizing.
However, given this is really just a refactor of the existing code, I don't suspect it should hold up this PR.
This prevents the node from repeatedly requesting and rejecting the
same transaction as different peers inv the same transaction.
Idea from Bitcoin Core commit 0847d9cb5fcd2fdd5a21bde699944d966cf5add9.