Skip to content

Quorum modifier fallback should mirror Core's invalid-signature check, not only all-zero bytes #942

Description

@xdustinface

Context

PR #934 made LLMQModifierType::new_quorum_modifier_type fall back to the block-hash quorum modifier when the work block's ChainLock signature is zeroed, matching the common case of Core's behavior. Core keys this fallback on CBLSSignature::IsValid() being false (GetNonNullCoinbaseChainlock, src/evo/cbtx.cpp), which is a strictly wider condition than all-zero bytes: it also covers 96-byte strings that fail G2 deserialization and the identity element (for example 0xc0 followed by 95 zero bytes).

Problem

Consensus permits a coinbase carrying such an invalid-but-nonzero bestCLSignature as long as bestCLHeightDiff == 0 (CheckCbTxBestChainlock, src/evo/specialtxman.cpp only verifies the signature when IsValid() is true). A deliberately hostile miner could mine a work block with an invalid nonzero signature, making Core derive the quorum modifier from the block hash while this library keeps the signature form. The reconstructed rotation member sets then diverge and every quorum whose quarter lands on that work block fails aggregate-signature validation.

Requires a hostile miner, no funds at risk, and the degradation paths added in #934 keep it from wedging the feed, but the member-set divergence stands until the fallback condition matches Core.

Why it was not fixed in #934

new_quorum_modifier_type compiles under the crate's default features (secp-recovery, bincode), while BLS deserialization (TryFrom<&BLSSignature> for blsful::Signature<Bls12381G2Impl>) exists only behind the bls feature in bls_sig_utils.rs. Gating the validity check would make the modifier derivation differ between feature sets, which is a worse hazard than the gap itself. Fixing this properly means moving or restructuring the feature boundary so a G2-validity check is available wherever the modifier is derived.

Suggested fix

Introduce a validity predicate on BLSSignature that is available unconditionally (or restructure the feature gates so the modifier derivation always has one), implement the fallback as work_block_height >= v20_activation_height && signature deserializes to a valid non-identity G2 point, and extend the existing selection test in quorum_modifier_type.rs with invalid-but-nonzero cases (compressed-infinity prefix, garbage bytes).

Found during the merge-readiness review of #934.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions