Skip to content

Fix mempool sync#3725

Merged
UdjinM6 merged 2 commits into
dashpay:developfrom
UdjinM6:fixmempoolsync
Sep 24, 2020
Merged

Fix mempool sync#3725
UdjinM6 merged 2 commits into
dashpay:developfrom
UdjinM6:fixmempoolsync

Conversation

@UdjinM6
Copy link
Copy Markdown

@UdjinM6 UdjinM6 commented Sep 22, 2020

Currently mempool sync isn't quite helpful because we ask for it while still syncing the chain and mempool might look completely different by the time we finish syncing blocks. This PR ensures we do it at the right time.

(build fails because of the issue fixed by #3724 , will rebase later)

@UdjinM6 UdjinM6 added this to the 16 milestone Sep 22, 2020
Copy link
Copy Markdown

@xdustinface xdustinface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK for the idea :) But still have some questions/thoughts.. Actually i like the idea of giving the mempool its own asset but im not sure if it makes a lot sense to introduce the overhead which comes with it (at least with the current implementation) its not a huge overhead but if it makes no sense we probably shouldn't? :D. I guess it would make sense if we would wait for some "mempool sync done" trigger or if we would wait at least X seconds for the MASTERNODE_SYNC_MEMPOOL to do something but with the implementation of this PR it just instantly tries to switch over to MASTERNODE_SYNC_GOVERNANCE in the same tick where it switches from MASTERNODE_SYNC_BLOCKCHAIN into MASTERNODE_SYNC_MEMPOOL? Please correct me if i missed something. If i didn't, wouldn't then something like 60192e4 not be good enough?

Also see comments attached below.

Comment thread src/masternode/masternode-sync.cpp Outdated
Comment thread src/masternode/masternode-sync.cpp Outdated
Comment thread src/masternode/masternode-sync.cpp Outdated
xdustinface and others added 2 commits September 23, 2020 09:40
Make sure the mempool sync requests only happen after the blockchain
sync is done.
Loop only if `-syncmempool`=true, make `if` a bit more readable
@UdjinM6 UdjinM6 changed the title Move initial mempool sync into its own "asset" type Fix mempool sync Sep 23, 2020
@UdjinM6
Copy link
Copy Markdown
Author

UdjinM6 commented Sep 23, 2020

Rebased, dropped my initial commit, applied 60192e4 (29c95e6) and refactored it a bit (6864cee).

Copy link
Copy Markdown

@xdustinface xdustinface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@UdjinM6 UdjinM6 merged commit e552c89 into dashpay:develop Sep 24, 2020
xdustinface added a commit to xdustinface/dash that referenced this pull request Sep 29, 2020
* masternode: Fix mempool sync

Make sure the mempool sync requests only happen after the blockchain
sync is done.

* Refactor

Loop only if `-syncmempool`=true, make `if` a bit more readable

Co-authored-by: xdustinface <xdustinfacex@gmail.com>
@UdjinM6 UdjinM6 deleted the fixmempoolsync branch November 26, 2020 13:26
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Apr 1, 2022
* masternode: Fix mempool sync

Make sure the mempool sync requests only happen after the blockchain
sync is done.

* Refactor

Loop only if `-syncmempool`=true, make `if` a bit more readable

Co-authored-by: xdustinface <xdustinfacex@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants