server: Optimize map limiting in block manager. #259
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
This simply exports and adds some comments to the fields of the BlockTemplate struct. This is primarily being done as a step toward being able to separate the mining code into its own package, but also it makes sense on its own because code that requests new block template necessarily examines the returned fields which implies they should be exported.
This optimizes the way in which the maps are limited by the block manager. Previously the code would read a cryptographically random value large enough to construct a hash, find the first entry larger than that value, and evict it. That approach is quite inefficient and could easily become a bottleneck when processing transactions due to the need to read from a source such as /dev/urandom and all of the subsequent hash comparisons. Luckily, strong cryptographic randomness is not needed here. The primary intent of limiting the maps is to control memory usage with a secondary concern of making it difficult for adversaries to force eviction of specific entries. Consequently, this changes the code to make use of the pseudorandom iteration order of Go's maps along with the preimage resistance of the hashing function to provide the desired functionality. It has previously been discussed that the specific pseudorandom iteration order is not guaranteed by the Go spec even though in practice that is how it is implemented. This is not a concern however because even if the specific compiler doesn't implement that, the preimage resistance of the hashing function alone is enough. Thanks to @Roasbeef for pointing out the efficiency concerns and the fact that strong cryptographic randomness is not necessary.
davecgh
force-pushed
the
merge_server_improve_limitmap_efficiency
branch
from
d63276a
to
c21db7a
Compare
|
OK |
|
tACK |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upstream commit 23f5914.