Update Output Manager UTXO query to split into multiple queries #1949
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
philipr-za
force-pushed
the
philip-split-utxo-query
branch
3 times, most recently
from
b59a1ce to
34502d3
Compare
12 tasks
sdbondi
reviewed
Jun 5, 2020
There was a problem hiding this comment.
This should be pretty efficient because requests and responses are sent/received as fast as each side is able. Unfortunately, the code complexity to do this is rather high - something to improve on in the lower layer.
Just one comment which I think points out a bug.
| } | ||
|
|
||
| impl Default for OutputManagerServiceConfig { | ||
| fn default() -> Self { | ||
| Self { | ||
| base_node_query_timeout: Duration::from_secs(30), | ||
| max_utxo_query_size: 5000, |
There was a problem hiding this comment.
Noting the max request message size would be 160kb and each response would be roughly less than 2Mb
There was a problem hiding this comment.
Yes that lines up with what I observed.
philipr-za
force-pushed
the
philip-split-utxo-query
branch
2 times, most recently
from
188b656 to
0d2c07c
Compare
Previously when the output manager service would send its UTXO validity query to its Base Node it would send a single query with all the UTXO hashes in it. The Base Node then responds with a single response message with all the valid UTXO’s that match a hash in the queries list. In large wallets this caused a problem where the response message could become large enough to be rejected by the frame size limits in the comms stack. In order to mitigate this issue the Output Manager service will now send queries consisting of up to a maximum number of outputs (specified in the config) and will send multiple queries until all the outputs have been queried so that any one response does not hit the frame size limit.
philipr-za
force-pushed
the
philip-split-utxo-query
branch
from
0d2c07c to
7ca23d6
Compare
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.
Description
Previously when the output manager service would send its UTXO validity query to its Base Node it would send a single query with all the UTXO hashes in it. The Base Node then responds with a single response message with all the valid UTXO’s that match a hash in the queries list. In large wallets this caused a problem where the response message could become large enough to be rejected by the frame size limits in the comms stack.
In order to mitigate this issue the Output Manager service will now send queries consisting of up to a maximum number of outputs (specified in the config) and will send multiple queries until all the outputs have been queried so that any one response does not hit the frame size limit.
How Has This Been Tested?
test_startup_utxo_scanwas updated to have more UTXO's than the max query limit and tested response to the 2 part query that was generated.Types of changes
Checklist:
developmentbranch.cargo-fmt --allbefore pushing.