chainlocks: Rely more on clsigs with known blocks instead of pure clsigs#3978
chainlocks: Rely more on clsigs with known blocks instead of pure clsigs#3978UdjinM6 wants to merge 3 commits intodashpay:developfrom
Conversation
There was a problem hiding this comment.
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 |
|
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.