Skip to content

Change chainlocks enforcing/notification flow - #2762

Closed
UdjinM6 wants to merge 1 commit into
dashpay:developfrom
UdjinM6:clsigflow
Closed

Change chainlocks enforcing/notification flow#2762
UdjinM6 wants to merge 1 commit into
dashpay:developfrom
UdjinM6:clsigflow

Conversation

@UdjinM6

@UdjinM6 UdjinM6 commented Mar 11, 2019

Copy link
Copy Markdown

Should enforce/notify when we have the block, not when we have only the header or trying to sign the tip (the later won't even work for non-masternodes).

Also, fix few related (dead)locks.

NOTE: This includes #2759 atm, will rebase after it's merged. The actual changes are in 081a40c.

@UdjinM6 UdjinM6 added this to the 14.0 milestone Mar 11, 2019
Comment thread src/llmq/quorums_chainlocks.cpp Outdated

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

081a40caeb8f2d14f8e38a3820267fd86cf3d0b2 utACK LGTM
We should probably add ChainLocks ZMQ if we haven't so far

@UdjinM6

UdjinM6 commented Mar 11, 2019

Copy link
Copy Markdown
Author

Rebased. Changes without whitespaces https://github.com/dashpay/dash/pull/2762/files?w=1

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure what the opinion is on stuff like this. Again we do it a lot in the code base, but I'm definitely not a fan

Comment thread src/llmq/quorums_chainlocks.cpp Outdated
@codablock

Copy link
Copy Markdown

Hmm, this will conflict quite a bit with what I'm working on right now (what I mentioned on Slack, the rework of how EnforceBestChainLock is called to make spork12 completely obsolete)

Should enforce/notify when we have the block, not when we have only the header or trying to sign the tip (the later won't even work for non-masternodes).

Also, fix few related (dead)locks.
@UdjinM6

UdjinM6 commented Mar 11, 2019

Copy link
Copy Markdown
Author

@codablock I don't see why/how these two would conflict cause this one just fixes it in such a way that it would actually work on all nodes and for both block->lock->notify and lock->block->notify paths but maybe you came up with a better approach which also fixes these issues while working on spork12 obsoletion, I'm fine with that, le't see :)

fyi: rebased and force-pushed to trigger Travis build, no code changes.

@UdjinM6

UdjinM6 commented Mar 12, 2019

Copy link
Copy Markdown
Author

closing in fav of #2765

@UdjinM6 UdjinM6 closed this Mar 12, 2019
@UdjinM6
UdjinM6 deleted the clsigflow branch November 26, 2020 13:25
@UdjinM6 UdjinM6 removed this from the 14.0 milestone Mar 1, 2021
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