-
Notifications
You must be signed in to change notification settings - Fork 1.2k
chainlocks: Rely more on clsigs with known blocks instead of pure clsigs #3978
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
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 catch! 👍 See suggestions-3978 for a few potential (trivial) tweaks in the test. In any case it looks like we need a rebase here after #3976 before an approval (if we merge that one first).
EDIT: I guess you can also just skip the MSG_CLSIG commits. Looks like thats something we could anyway refactor in an other PR by adding islock/clsig support to mininode because its also used in other places i.e. duplicated code.
…igs when it makes sense A malicious quorum can sign a clsig with some (random) hash and some future height. This will basically disable clsig logic for all heights up to that future one without actually locking the chain. Note that the best known clsig with a known block is not affected by the issue, no conflicts below that block are allowed. Using the best clsig with a known block instead of a pure best clsig (with no known blocks) fixes the issue.
…ng new legit clsigs
cf981bb to
88fe4b5
Compare
|
Rebased |
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.
utACK
|
I'm not sure how I feel about this... Really, I'd say this is the best case for a malicious chainlock. If someone forms a malicious chainlock, the best outcome is the chain halting, because that prevents any attacker from actually making any money or anything off of the attack. |
|
@PastaPastaPasta I don't understand... can you elaborate pls? |
A malicious quorum can sign a clsig with some (random) hash and some future height, up to
tip + SIGN_HEIGHT_OFFSET(anything above it won't passVerifyRecoveredSig/SelectQuorumForSigning, see tests forfake_clsig2). Such a clsig not only does not lock the chain but it also prevents new clsigs (for heights up to that future one) from forming or being accepted, see tests forfake_clsig3(fails without b010bc9). Chances to form such a quorum are close to 0 e.g. 1.29e-38 if you control 1500 MNs (seeSuccess probability of creating a malicious ChainLockhere for more info), so it's not a critical issue by any means but I feel like it still makes sense to reduce any potential impact of such an event when possible.Note: the issue is only for future clsigs, the best clsig with a known block is not affected, no reorgs below the already locked block are possible.