-
Notifications
You must be signed in to change notification settings - Fork 58
DIP0008: Replace signing attempts with a majority of quorums signing the same height #85
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
Conversation
3b2ae05 to
7830738
Compare
xdustinface
left a comment
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.
Good idea 👍 I didn't think that much about the details/impact yet but overall i think it makes sense, see comments below for some thoughts after a first read + table-of-contents section also needs some adjustments :) e.g. this dead link -> signing-attempts
Also, did you think about just making a separate DIP out of this? I honestly don't have a strong opinion on wether it should be separate or not but i think it could make sense to make a new and refer to this one. Thoughts?
Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
7cbeaf3 to
950f412
Compare
950f412 to
ac4defb
Compare
I thought about it initially but imo it's ok to have this as an update because the core idea is the same - use (potentially) multiple LLMQs to secure the chain. The difference is basically that in signing attempts it was (up to) 2 quorums - one was signing the attempt and (potentially) another one was finalizing CLSIG while in MQ CLs every LLMQ just signs its own CLSIG for whatever block it considers the right one and then anyone can aggregate these CLSIGs to agree afterwards. So it's more like an evolution of the initial proposal rather than a radically new idea imo. |
|
I'm not sure I understand the reasoning behind this change. Could you elaborate more @UdjinM6? Taking control of a 400 member quorum requires close to 60% of the network or 50% and a lot of hash power. If you are afraid this is not enough we could use a higher threshold quorum, and/or have more members. |
|
The idea is to make ChainLocks as robust as possible basically. We can avoid some issues by tweaking the way we handle ChainLocks internally by applying stricter rules to CLSIG messages (see dashpay/dash#3978) but it can potentially make ChainLocks more fragile in edge cases e.g. when network is under high load and blocks aren't propagated fast enough. The solution proposed here solves that. |
xdustinface
left a comment
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.
For me the step stuff looks a bit confusing tbh, thoughts about c064801? See xdustinface/dash@36628cf for an implementation of it.
Might be only me but it think this makes it easier to understand and its imo just cleaner.
|
@xdustinface Makes sense, picked both. Thanks! 👍 |
xdustinface
left a comment
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.
Some more (and my last) points:
-
Let's maybe just move the signer stuff into the table like in 3f02341?
-
Maybe we should mention somewhere that the value of
SPORK_19_CHAINLOCKS_ENABLEDis used to decide if multi LLMQ signing is enabled or not. -
It looks like there is a mismatch between dashpay/dash#4016 and the changes here. For me when i read it here it sounds like:
- If single LLMQ signing is enabled, i.e. the spork is active and != 1, the
CLSIGhas structureA(signers fields are not present) - If multi LLMQ signing is enabled, i.e. the spork value is 1, the
CLSIGhas structureB(signer fields present).
But it's actually always structure
Bwith protocol version70220but with signers size 0, no? So we could maybe merge those both tables, put them somewhere (maybe a separate message section) below the signing section and somehow mark the new fields introduced by the new version. Thoughts? - If single LLMQ signing is enabled, i.e. the spork is active and != 1, the
d97b8a7 to
0f32517
Compare
0f32517 to
7557704
Compare
… and multiple LLMQs
xdustinface
left a comment
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 all good to me now 👍
|
Is the "Calculations" section still accurate with this change? Also, I was thinking it might be nice to include a link to the original version of the document (whatever the most recent change was) so people can easily refer back to it. I think that could be added to the "Prior Work" section. |
Co-authored-by: thephez <thephez@users.noreply.github.com>
e9cf478 to
db181c9
Compare
Should be accurate for a single LLMQ mode but yeah, for a multi-quorum mode you'd need to disrupt 2 quorums (or control 3 quorums to produce invalid clsig) at the same time, so I guess you should take all these probabilities roughly to the power of 2 (or 3 respectively) or so? Let's ping @DarrenTa :)
Hmm... current version in master is https://github.com/dashpay/dips/blob/81e81b52799114191729364ed36dc0e372062fbe/dip-0008.md but I'm not sure if it makes a lot of sense to link back to itself, especially in a git repo.. |
|
Thank you so much for the ping. I can provide the following:
I have cited DIP0008 in my academic work, it would make my citations cleaner if DIP0008 was not changed significantly. I was lazy and did not put date accessed. I've also cited (withdrawn) 0005 and think the citations provide for a robust discussion, even if not live. No reason 0008 has to be edited to agree with the live implementation. I'll circle back here in a week and report back with a proper analysis. |
|
Hmm.. ok, you guys convinced me to at least create an alternative PR to have smth to compare to :) Pls see #89. |
|
closing in fav of #89 |
The main idea is to let all active ChainLock quorums create new CLSIG messages at every block and to require the majority of these quorums to vote for the same hash to actually lock the chain. Unlike in current system, having a malicious quorum will no longer be enough to disrupt ChainLocks via withholding attack or by producing "fake" CLSIGs (CLSIGs for some future height with some random hashes). This also deprecates the "signing attempts" part cause it feels redundant in this setup.
PoC for Dash Core: dashpay/dash#4016