[Refactoring] Decouple and encapsulate tier two synchronization state #2690

furszy · 2021-12-17T20:21:07Z

Work decoupled from #2647 which has grew a lot lately. So it can get reviewed and merged in parallel of #2607.

Essentially have encapsulated the tier two synchronization status in a separate object which is only in charge of storing and retrieving the sync status members values in a synchronic manner.

Under this new architecture, every manager/object that needs access to it can add a small dependency to the new g_tiertwo_sync_state object only (which has no other dependencies), without depending on the whole masternodesSync object (which is a bad name for the tier two sync manager, but.. renaming will come in another PR), which depends on every other tier two manager and lot other files.
Plus, to not generate confusions, the g_tiertwo_sync_state members update are only, and exclusively, done by the tier two sync manager.

Fixing in this way, at least (because there are surely more), the following problems:

Circular dependencies:
- "activemasternode -> masternode-sync -> activemasternode".
- "budget/budgetmanager -> masternode-sync -> budget/budgetmanager"
- "masternode -> masternode-sync -> masternode"
- "masternode-payments -> masternode-sync -> masternode-payments"
- "masternode-sync -> validation -> masternode-sync"
- "evo/deterministicmns -> masternode -> masternode-sync -> evo/deterministicmns"
Allow and guard concurrent access. (Prior to this work, the word "concurrency" was inexistent for this code).

random-zebra

Very nice refactoring. Code ACK with some notes.

src/masternode-sync.cpp

src/tiertwo/tiertwo_sync_state.h

src/tiertwo/tiertwo_sync_state.cpp

furszy · 2021-12-20T19:48:45Z

Updated per feedback 👍 .

src/masternode-sync.cpp

furszy · 2021-12-22T15:45:09Z

Updated per feedback, squashed it in your 4b3bcfb zebra.

random-zebra

ACK 4b3bcfb95145eb4ee7905899daafa62cb253c7b7

furszy · 2021-12-27T18:08:17Z

Just noticed that need to rebase this one once more, the new test dkg_pose introduced in #2607 is setting the tier two sync asset manually.
So.. if we merge it as is, the final branch will not compile.

furszy · 2021-12-27T18:59:42Z

rebased, ready to go once more.

random-zebra

ACK 24d9fa5a554068a6261328b1b5e60562e9a2fa41

furszy · 2021-12-29T21:46:12Z

due conflicts with recently merged PRs, rebased on master once more.

random-zebra

ACK 75bc24f708b2f6c4d8de1ca53a462ca882b15df1

…:Process function. So the reset call only happens in the mnsync thread and not on every thread/object that calls to a getter function..

…_sync_state class which only stores the final value. And rename it to UpdateBlockchainSync.

… most of the masternode-sync dependencies.

…sync status every 30 seconds.

And remove extra call to SetBlockchainSync(false) from CMasternodeSync after calling CMasternodeSync::Reset()

furszy · 2022-01-04T12:08:02Z

rebased for merge conflicts with 2697, had to dismiss the approval again.

random-zebra

ACK 3e05c82 again. Let's get this merged.

Fuzzbawls

ACK 3e05c82

furszy self-assigned this Dec 17, 2021

furszy changed the title ~~[Refactoring] Decouple and encapsulate tier two synchronization state~~ [WIP][Refactoring] Decouple and encapsulate tier two synchronization state Dec 17, 2021

furszy changed the title ~~[WIP][Refactoring] Decouple and encapsulate tier two synchronization state~~ [Refactoring] Decouple and encapsulate tier two synchronization state Dec 19, 2021

furszy added this to the 6.0.0 milestone Dec 19, 2021

furszy added the Refactoring label Dec 19, 2021

random-zebra reviewed Dec 20, 2021

View reviewed changes

src/masternode-sync.cpp Outdated Show resolved Hide resolved

src/masternode-sync.cpp Outdated Show resolved Hide resolved

src/tiertwo/tiertwo_sync_state.h Outdated Show resolved Hide resolved

src/tiertwo/tiertwo_sync_state.cpp Outdated Show resolved Hide resolved

furszy force-pushed the 2021_tiertwo_decouple_sync_state branch 2 times, most recently from 3c69135 to a4526d1 Compare December 20, 2021 19:39

furszy force-pushed the 2021_tiertwo_decouple_sync_state branch 2 times, most recently from a9a18ac to 762fbd7 Compare December 21, 2021 01:18

random-zebra reviewed Dec 22, 2021

View reviewed changes

src/masternode-sync.cpp Outdated Show resolved Hide resolved

furszy force-pushed the 2021_tiertwo_decouple_sync_state branch from 8d75f27 to 4b3bcfb Compare December 22, 2021 15:44

furszy requested a review from random-zebra December 26, 2021 21:45

random-zebra previously approved these changes Dec 27, 2021

View reviewed changes

furszy dismissed random-zebra’s stale review via 24d9fa5 December 27, 2021 18:58

furszy force-pushed the 2021_tiertwo_decouple_sync_state branch from 4b3bcfb to 24d9fa5 Compare December 27, 2021 18:58

furszy requested a review from random-zebra December 27, 2021 18:59

random-zebra previously approved these changes Dec 28, 2021

View reviewed changes

furszy dismissed random-zebra’s stale review via 7ac83b7 December 29, 2021 21:43

furszy force-pushed the 2021_tiertwo_decouple_sync_state branch from 24d9fa5 to 7ac83b7 Compare December 29, 2021 21:43

furszy force-pushed the 2021_tiertwo_decouple_sync_state branch from 7ac83b7 to 75bc24f Compare December 30, 2021 15:23

furszy requested review from Fuzzbawls and random-zebra January 3, 2022 13:17

random-zebra previously approved these changes Jan 3, 2022

View reviewed changes

furszy mentioned this pull request Jan 3, 2022

[TierTwo] Long Living Masternode Quorums - Part 2, network layer #2647

Merged

furszy and others added 10 commits January 4, 2022 09:03

mnsync: move sync reset due an hibernation client to CMasternodeSync:…

8ac4d91

…:Process function. So the reset call only happens in the mnsync thread and not on every thread/object that calls to a getter function..

mnsync: decouple IsBlockchainSync into a publicly available g_tiertwo…

ca03c76

…_sync_state class which only stores the final value. And rename it to UpdateBlockchainSync.

masternodesync: move sync asset phase to tiertwo_sync_state class.

c1091ab

[Refactoring] Unify AddedMasternode* functions

f29f9af

masternodesync: move seen maps to tiertwo_sync_state class and remove…

7e2f332

… most of the masternode-sync dependencies.

Clean corrected masternode-sync circular dependencies.

1eb58ea

Fix several compiler warning.

e90e27d

TierTwo: Do not lock g_best_block_mutex too often, only update chain …

c63d51c

…sync status every 30 seconds.

TierTwoSyncState: make every getter function const.

0ad6cd8

[TierTwo] Request missing mnw signer during initial sync

3e05c82

And remove extra call to SetBlockchainSync(false) from CMasternodeSync after calling CMasternodeSync::Reset()

furszy dismissed random-zebra’s stale review via 3e05c82 January 4, 2022 12:06

furszy force-pushed the 2021_tiertwo_decouple_sync_state branch from 75bc24f to 3e05c82 Compare January 4, 2022 12:06

random-zebra approved these changes Jan 5, 2022

View reviewed changes

Fuzzbawls approved these changes Jan 5, 2022

View reviewed changes

furszy merged commit 017953e into PIVX-Project:master Jan 5, 2022

Fuzzbawls modified the milestones: 6.0.0, 5.5.0 Sep 11, 2022

[Refactoring] Decouple and encapsulate tier two synchronization state #2690

[Refactoring] Decouple and encapsulate tier two synchronization state #2690

Uh oh!

Conversation

furszy commented Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

random-zebra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

furszy commented Dec 20, 2021

Uh oh!

Uh oh!

furszy commented Dec 22, 2021

Uh oh!

random-zebra left a comment

Choose a reason for hiding this comment

Uh oh!

furszy commented Dec 27, 2021

Uh oh!

furszy commented Dec 27, 2021

Uh oh!

random-zebra left a comment

Choose a reason for hiding this comment

Uh oh!

furszy commented Dec 29, 2021

Uh oh!

random-zebra left a comment

Choose a reason for hiding this comment

Uh oh!

furszy commented Jan 4, 2022

Uh oh!

random-zebra left a comment

Choose a reason for hiding this comment

Uh oh!

Fuzzbawls left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

furszy commented Dec 17, 2021 •

edited

Loading