-
Notifications
You must be signed in to change notification settings - Fork 118
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
[universe]: fix proof sync for large asset minting batches #1093
Conversation
Pull Request Test Coverage Report for Build 10470347293Details
💛 - Coveralls |
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.
Looks good! Just need that one fix + possible logging improvements.
This commit fixes an issue with large sets of asset issuances: When there are more assets in a group than the current batch sync limit (200), then there's a chance that the group anchor (the one asset containing the group key reveal record) isn't in the first batch. So we wouldn't be able to insert the first batch at all, since those assets would all reference a group key that isn't yet known. To avoid this problem, we insert any proofs with group key reveal records immediately and only start batching the others after going through all of them. This means we'll keep more assets in memory and can start to process them later. But at least the process won't fail anymore.
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.
LGTM 👍🏽
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.
LGTM 🚜
I encountered this on
testnet
while trying to reproduce #675.Can be reproduced with:
And
tapcli u s --universe_host testnet.universe.lightning.finance:10029
.This PR fixes the issue by making sure any group anchors are inserted into the DB first.