Conversation
…te (IIP-52) Scaffolding for the BLS signature aggregation work tracked in IIP-52. No behavior change yet: EnableBLSAggregation is gated on IsToBeEnabled, and the BLS keys plumbed into rollDPoSCtx are not yet used to sign or verify endorsements. - blockchain/config.go: add Chain.BLSProducerPrivKey (comma-separated hex) and BLSProducerPrivateKeys(). Empty value falls back to deriving each BLS key from the corresponding ECDSA producer key via crypto.GenerateBLS12381PrivateKey. - consensus/scheme/rolldpos: Builder.SetBLSPriKey; NewRollDPoSCtx accepts []*crypto.BLS12381PrivateKey aligned 1:1 with producer ECDSA keys; rollDPoSCtx stores them on blsPriKeys for the upcoming signing path. - consensus/consensus.go: wire SetBLSPriKey(cfg.Chain.BLSProducerPrivateKeys()). - action/protocol/context.go: FeatureCtx.EnableBLSAggregation gated on g.IsToBeEnabled(height); flips to a named hardfork height later. - go.mod: bump iotex-proto to envestcc/iotex-proto bls-aggregate (52e72a6) for the BlockFooter aggregated_signature / signer_bitmap fields and the BLSEndorsement message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switches consensus vote signing to BLS12-381 post-fork, reusing the existing Endorsement type and dispatching on signature length (65B secp256k1 vs 96B BLS). Receiver verification and endorsement-manager quorum integration land in a follow-up PR; with the feature gate parked at IsToBeEnabled this commit is dead code in production. - endorsement.EndorseBLS / VerifyBLSEndorsement: thin helpers that produce / verify a regular *Endorsement whose signature field carries a BLS sig. Endorser remains the delegate's secp256k1 producer key so the existing Endorser().Address() path still resolves the iotex address; receivers look up the BLS verifying key from candidate state by that address. - ConsensusConfig.BLSAggregationEnabled(height): feature gate wired off Genesis.ToBeEnabledBlockHeight. Will be re-pointed at a named hardfork height once the full Phase-2 stack lands. - rollDPoSCtx.newEndorsement / endorseBlockProposal: post-fork, sign PROPOSAL, LOCK and COMMIT votes plus the proposer's wrapping endorsement with BLS (skipping delegates without a configured BLS key). Block header signing remains on the ECDSA producer key — that signature ties the block to chain identity and is unrelated to the consensus vote layer. - The proposer's producerKey (ECDSA + BLS + address) is threaded through Proposal / mintNewBlock / endorseBlockProposal so the branch can pick the right key without a separate lookup. - iotex-proto bump to envestcc/iotex-proto@e4439ef (PR iotexproject#169): clarifies Endorsement.signature semantics (pre-fork 65B secp256k1, post-fork 96B BLS, distinguished by length). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stacks on top of the BLS sender PR. Wires up the receiver side so BLS- signed consensus endorsements are accepted, verified against pubkeys resolved from candidate state, and counted toward quorum. With the feature gate parked at IsToBeEnabled this is dead code in production; intended for local iteration until the sender PRs land. - BLSPubKeysByEpochFunc callback type + Builder.SetBLSPubKeysByEpochFunc wired through to NewRollDPoSCtx. - consensus.go provides the implementation: reads the epoch's delegate list from candidate state and extracts each candidate's registered BLSPubKey, returning a map keyed by operator iotex address. - roundCalculator caches the BLS pubkey index per round, decoded as *crypto.BLS12381PublicKey. UpdateRound carries it across height transitions inside an epoch and re-fetches on epoch boundaries. - roundCtx.BLSPubKey(addr) accessor; roundCtx.verifyEndorsement(doc, en) dispatches on signature length (65B secp256k1 vs 96B BLS). - rollDPoSCtx.VerifyEndorsement(height, doc, en) is the public entry point; same length-aware dispatch plus pre/post-fork gating — pre-fork rejects 96B sigs, post-fork rejects 65B sigs. - HandleConsensusMsg replaces the unconditional ECDSA verify with the new VerifyEndorsement; CheckBlockProposer's proofOfLock replay path flows through round.AddVoteEndorsement which now dispatches on signature length internally, so BLS endorsements in proof-of-lock are verified transparently.
- endorsement/bls_endorsement_test.go: round-trip EndorseBLS / VerifyBLSEndorsement, plus negative cases (wrong pubkey, tampered document, tampered signature, nil inputs). Uses deterministic in-test keys (no identityset dep from this package). - consensus/scheme/rolldpos/bls_verify_test.go: covers the two-layer dispatch — roundCtx.verifyEndorsement (signature-length branch, BLS pubkey lookup miss, mismatched pubkey) and rollDPoSCtx.VerifyEndorsement (length gating with the BLS aggregation feature flag in both directions). Plus sender tests that newEndorsement emits the expected signature scheme based on the feature gate. 11 new tests; everything in the affected packages still passes (51 tests total across endorsement/ and consensus/scheme/rolldpos/).
- Bundle delegate address with BLS pubkey into a single 'delegate' struct; roundCtx.delegates becomes []delegate, dropping the parallel blsPubKeys map. roundCalc.delegatesAt replaces blsPubKeysFor and merges both callbacks into one slice — single source of truth for the address/pubkey alignment. - rollDPoSCtx.VerifyEndorsement takes a single *EndorsedConsensusMessage instead of (height, doc, en); the message already carries all three. - Shrink VerifyEndorsement lock scope: snapshot round + feature flag under RLock, release before doing the (potentially slow) signature verification. Safe because *roundCtx is replaced, never mutated in place. Test helpers + the two consumers (HandleConsensusMsg, roundCtx_test) updated. All targeted tests pass.
Per follow-up review on PR iotexproject#4843: remove the separate BLSPubKeysByEpochFunc callback. NodesSelectionByEpochFunc now returns []*Delegate (exported), where each Delegate pairs the operator address with its decoded BLS12-381 public key. consensus.go builds these from candidate state in one pass; roundCalculator stores them directly and no longer needs a parallel lookup/merge step. - Export delegate -> Delegate{Address, BLSPubKey}. - NodesSelectionByEpochFunc: ([]string) -> ([]*Delegate). - Drop BLSPubKeysByEpochFunc type, Builder.SetBLSPubKeysByEpochFunc, the NewRollDPoSCtx param, and roundCalculator.delegatesAt / blsPubKeysFor. - roundCalculator.Delegates returns []*Delegate; Proposers extracts addresses; IsDelegate scans by address. - consensus.go decodes each candidate's BLS pubkey once per epoch in delegatesByEpochFunc; proposersByEpochFunc reuses it. Net -68 lines. Build + vet clean; targeted tests pass.
Per PR iotexproject#4843 review: UpdateRound was reaching into round.delegates directly because Delegates() returned []string while the field is []*Delegate. Make the accessor return []*Delegate so UpdateRound (and any future caller) can use round.Delegates() consistently. - roundCtx.Delegates() now returns []*Delegate. - endorsementManager.Log's (unused) delegates param retyped to []*Delegate. - The one genuine []string consumer (ConsensusMetrics.LatestDelegates, an external metrics field) extracts addresses at the call site.
Phase 2 deliverable for IIP-52: proposer aggregates the per-block COMMIT BLS signatures into a single 96-byte sig + a signer bitmap, stored in BlockFooter.aggregated_signature and BlockFooter.signer_bitmap. Verifiers reconstruct the signer set from the bitmap, look up each BLS pubkey from the round's delegate index, and FastAggregateVerify the aggregate against the shared COMMIT-vote hash. - blockchain/block/footer.go: new fields aggregatedSignature, signerBitmap; proto round-trip; IsAggregated / AggregatedSignature / SignerBitmap accessors. - blockchain/block/block.go: new Block.FinalizeWithAggregate; the one-shot contract is preserved via a commitTime witness so either path errors on second call. - consensus/scheme/rolldpos/aggregate.go: aggregateCommitEndorsements builds the aggregate sig + bitmap from a slice of BLS COMMIT endorsements indexed against the round's delegates; bitmapSigners is the inverse for the verifier. - consensus/scheme/rolldpos/rolldposctx.go: at commit time, branch on BLSAggregationEnabled and call FinalizeWithAggregate post-fork. - consensus/scheme/rolldpos/rolldpos.go: ValidateBlockFooter routes aggregated footers through validateAggregatedFooter — bitmap → delegates → BLS pubkeys → BLSAggregateSignature.Verify, with a separate 2/3 majority check. - action/protocol/staking/protocol.go: ActiveCandidates filters out candidates without a registered BLS pubkey once aggregation is enabled, so the aggregate signer set is well-defined. - endorsement/endorsement.go: expose SigningHash so the verifier can reconstruct the COMMIT-vote hash from blk.CommitTime() outside an Endorsement struct. All signers sign the same hash (deterministic ts from round start + TTL sum), which is what FastAggregateVerify requires. 8 new unit tests cover the aggregate round-trip, partial signer sets, rejection of non-BLS endorsements / unknown endorsers, and bitmap edge cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Foundational PR (Y1) for the BLS Producer Identity follow-up to IIP-52. Decouples Header from the secp256k1-only public key type so that the existing methods can transparently handle BLS-signed headers post-fork. No behavior change for pre-fork blocks. - Header.pubkey (crypto.PublicKey) -> Header.producerPubkey ([]byte) raw storage. The signature scheme is implied by len(blockSig): 65 bytes secp256k1 (pre-fork), 96 bytes BLS12-381 (post-fork). - Header.PublicKey() returns nil for non-secp256k1 producer pubkeys; new Header.ProducerPubKey() []byte exposes the raw bytes regardless of scheme. Callers that need a typed PublicKey for an address derivation should switch to ProducerAddress() or do a state lookup post-fork. - Header.VerifySignature() dispatches on len(blockSig); BLS path uses crypto.BLS12381PublicKeyFromBytes + Verify against HashHeaderCore. - Header.ProducerAddress() dispatches on len(blockSig). Pre-fork returns the io1... iotex address (existing behavior). Post-fork returns the hex encoding of the 48-byte BLS pubkey, which is the canonical post-fork operator identifier (per the IIP draft). Return type stays string; the format flips across the fork boundary. Builder/testing setters write producerPubkey directly. blockindexer.go is left as-is: its blk.PublicKey().Address() call returns nil for BLS-signed headers and errors with "failed to get pubkey", which is the safe fail-loud behavior until the per-block state lookup path (populating BlockCtx.Producer with the candidate's Operator address) lands in a later PR. 5 new unit tests cover the BLS dispatch paths: positive round-trip, proto round-trip, wrong-scheme rejection, and empty-pubkey rejection. 27 existing block-package tests continue to pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR iotexproject#4851 review feedback (envestcc): storing the producer pubkey as []byte sacrifices type information. Pre-Y1 it was crypto.PublicKey, which BLS can't satisfy because the interface bundles ECDSA-shaped identity methods (Address, Hash, EcdsaPublicKey) that have no meaningful BLS analogue — forcing BLS through Address() would invite silent truncation in hash.BytesToHash160 and common.BytesToAddress consumers. Compromise: a new narrow interface block.Verifier {Bytes; Verify} that both crypto.PublicKey (secp256k1) and *crypto.BLS12381PublicKey satisfy today without modification. Header.pubkey moves from []byte to Verifier. - VerifySignature: h.pubkey.Verify(digest, sig) — typed, no length switch - ProducerAddress: type-switch on the stored Verifier instead of dispatch on len(blockSig) — intent is explicit ("I'm a BLS key, hex-encode me") - LoadFromBlockHeaderProto: length-based dispatch lives only here, at the wire→typed boundary; downstream code sees the typed Verifier - PublicKey() accessor: type-assert to crypto.PublicKey; returns nil for BLS-signed headers (existing behavior, but no longer re-parses the bytes on every call) - ProducerPubKey(): pubkey.Bytes() with defensive copy - Identity-derivation (Address/Hash) is deliberately absent from Verifier — BLS has no iotex address; the rationale is captured in verifier.go and the BLS Producer Identity IIP draft Identity-shaped accessors that were length-dispatching are now type-dispatching; tests updated to write the typed key into the field rather than raw bytes. Added a decode-side rejection test for malformed pubkey lengths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Foundation for the Y4 consumer migration. Adds two pieces of plumbing that Y4b will consume: 1. BlockCtx.ProducerPubKey []byte — raw producer pubkey bytes populated at BlockCtx assembly. Pre-fork this is the secp256k1 pubkey (33 / 65 B); post-fork it's the BLS12-381 pubkey (48 B). Consumers that match against state.Candidate.BLSPubKey use this rather than Producer.String(), since iotex-address derivation is undefined for a BLS pubkey. 2. CandidateCenter.GetByBLSPubKey + CandidateStateManager method — linear scan over candidates returning the one whose registered BLSPubKey matches. Mirrors the existing GetByName / GetByOwner / GetByOperator pattern. Used by Y4b consumers (reward attribution, EVM fee recipient, productivity tracking) to resolve a candidate from a BLS-signed header's ProducerPubKey. blockchain.go's three BlockCtx-assembly sites (Validate, contextWithBlock used by MintNewBlock + commitBlock) now populate ProducerPubKey: - Validate: blk.Header.ProducerPubKey() — the bytes carried on the header by Y1's length-dispatch. - MintNewBlock: producerPrivateKey.PublicKey().Bytes() — the producer signs with their own key, no header to consult yet. - commitBlock: blk.Header.ProducerPubKey() — same as Validate. No behaviour change for any existing consumer: Producer is still the iotex-address-shaped field they read today, ProducerPubKey is a NEW field that no one consumes yet. Y4b migrates EVM Coinbase to use a state-looked-up Reward address, reward.go to match by ProducerPubKey, ValidateBlockFooter to use roundCtx.IsProducer, etc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
envestcc
added a commit
to envestcc/iotex-core
that referenced
this pull request
Jun 16, 2026
Addresses two review threads on PR iotexproject#4854: ## Comments 1 + 5: drop the `except` parameter Both envestcc and CoderZhi flagged that ContainsBLSPubKey(pubkey, except) bool pushed the "is this me?" decision into a generic helper that doesn't belong there. Replaces ContainsBLSPubKey(pubkey, except) bool with the broader GetByBLSPubKey(pubkey) *Candidate: callers receive the (possibly nil) holder and compare identifiers themselves. The three register / update handler call sites now read if holder := csm.GetByBLSPubKey(act.BLSPubKey()); holder != nil && holder.GetIdentifier().String() != c.GetIdentifier().String() { return ErrCandidateConflict } which is more explicit than the prior `except`-hiding API. It also unifies with the same GetByBLSPubKey method added in iotexproject#4857 (Y4a) so the two PRs don't introduce parallel BLS-pubkey-lookup APIs. ## Comments 2 + 4: strict nil-candidateID contract Both BLSPopSigningRoot and the surrounding Sign / Verify helpers used to silently accept a nil candidateID by skipping the candidate-binding write. That degrades the scheme to domain+pubkey-only — exactly the shape an attacker reaching for a cross-candidate replay would hope for. The three entry points now refuse: - BLSPopSigningRoot returns nil - SignBLSPop returns error("nil candidate ID; PoP must bind to a candidate identity") - VerifyBLSPop rejects before any cryptographic work TestBLSPop_RejectNilCandidateID locks the contract in place. ## Not in this commit Comment 3 ("blsPubKey can be removed from the signing root") — deferred. Will reply on the thread; the short answer is that the IRTF BLS draft defines canonical PoP as sign(sk, pk) and dropping blsPubKey from the digest opens up same-message aggregation when owner-uniqueness is ever relaxed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 16, 2026
Merged
guo
pushed a commit
to envestcc/iotex-core
that referenced
this pull request
Aug 20, 2026
Addresses two review threads on PR iotexproject#4854: ## Comments 1 + 5: drop the `except` parameter Both envestcc and CoderZhi flagged that ContainsBLSPubKey(pubkey, except) bool pushed the "is this me?" decision into a generic helper that doesn't belong there. Replaces ContainsBLSPubKey(pubkey, except) bool with the broader GetByBLSPubKey(pubkey) *Candidate: callers receive the (possibly nil) holder and compare identifiers themselves. The three register / update handler call sites now read if holder := csm.GetByBLSPubKey(act.BLSPubKey()); holder != nil && holder.GetIdentifier().String() != c.GetIdentifier().String() { return ErrCandidateConflict } which is more explicit than the prior `except`-hiding API. It also unifies with the same GetByBLSPubKey method added in iotexproject#4857 (Y4a) so the two PRs don't introduce parallel BLS-pubkey-lookup APIs. ## Comments 2 + 4: strict nil-candidateID contract Both BLSPopSigningRoot and the surrounding Sign / Verify helpers used to silently accept a nil candidateID by skipping the candidate-binding write. That degrades the scheme to domain+pubkey-only — exactly the shape an attacker reaching for a cross-candidate replay would hope for. The three entry points now refuse: - BLSPopSigningRoot returns nil - SignBLSPop returns error("nil candidate ID; PoP must bind to a candidate identity") - VerifyBLSPop rejects before any cryptographic work TestBLSPop_RejectNilCandidateID locks the contract in place. ## Not in this commit Comment 3 ("blsPubKey can be removed from the signing root") — deferred. Will reply on the thread; the short answer is that the IRTF BLS draft defines canonical PoP as sign(sk, pk) and dropping blsPubKey from the digest opens up same-message aggregation when owner-uniqueness is ever relaxed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
envestcc
added a commit
that referenced
this pull request
Aug 25, 2026
…RITICAL, pre-IIP-52) (#4854) * fix(staking): require BLS proof-of-possession at candidate register/update CRITICAL security fix for the BLS rogue-key aggregate-forgery attack against IIP-52's same-message FastAggregateVerify path. IIP-52 verifies the COMMIT-vote BLS aggregate signature against the sum of registered candidate BLS public keys: blstAggregateSignature.FastAggregateVerify(true, Σ pk_i, msg, dst) with dst = "BLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_POP_". The _POP_ suffix names the IRTF "Proof of Possession" ciphersuite, which is only sound when each registered pubkey has been accompanied at registration time by a signature proving knowledge of the corresponding secret key. Today registration validates only key format + subgroup membership: crypto.BLS12381PublicKeyFromBytes(blsPubKey) No PoP is required. A registered candidate can publish pk_rogue = g^x − Σ(other delegates' pubkeys) computed from public information, supply a single signature σ = sign(x, msg) and a bitmap that claims every delegate signed, and FastAggregateVerify collapses Σ pk_i + pk_rogue to g^x, accepting σ. The bitmap satisfies isMajorityCount, so one delegate forges a 2/3+ quorum certificate and the consensus safety property is broken. The window is latent: IIP-52's ToBeEnabledBlockHeight = math.MaxUint64 so this code path is not live on any chain yet. It must be fixed before activation. - Add a 96-byte blsPop field to CandidateBasicInfo proto (iotex-proto PR pushed; go.mod replace updated to envestcc/iotex-proto@7a486f6) - New action/protocol/staking/bls_pop.go defines the PoP signing root: BLSPopSigningRoot(blsPubKey, ownerAddress) = SHA-256("IOTEX_BLS_POP_v1" || blsPubKey || ownerAddress.Bytes()) Binding all three values blocks the three replay variants: (a) attacker without the secret cannot sign over the canonical message at all — closes rogue key; (b) PoP for owner A does not validate for owner B — closes candidate-substitution replay; (c) "IOTEX_BLS_POP_v1" domain prefix is disjoint from any other BLS DST iotex uses, so a PoP cannot be replayed as a consensus vote or vice versa, and a future fork can rotate via "v2". - New FeatureCtx.EnforceBLSPoP shares IsToBeEnabled height with EnableBLSAggregation today but is a semantically distinct switch so the two can diverge if a hotfix or fork-height adjustment is needed. - action.CandidateRegister and action.CandidateUpdate carry blsPop through their constructors, Proto/LoadProto, and accessors. NewCandidateRegisterWithBLS and NewCandidateUpdateWithBLS now accept a blsPop parameter; pre-fork callers may pass nil. - handleCandidateRegister, handleCandidateUpdate, and handleCandidateUpdateByOperator call VerifyBLSPop before writing Candidate.BLSPubKey when EnforceBLSPoP is active. Failure returns ReceiptStatus_ErrUnauthorizedOperator. - TestBLSPop_RogueKeyAttackBlocked is the regression guard: it models the attacker trying to register a pubkey for which they do not have the secret, demonstrates the only PoP they can produce (signed under their own secret) does NOT validate against the target pubkey, and confirms the control case (a legitimate registrant can produce a valid PoP). - ioctl/SDK wiring: stake2register.go and stake2update.go currently pass nil for blsPop with TODO markers. These need a flag to ingest a BLS private key and derive the PoP. Pre-fork the empty PoP is accepted; post-fork the same call will fail with the new check, so the tooling MUST be updated before the fork. - Migration: existing pre-fork candidate records have no PoP. Before the fork activates, all active candidates must submit a candidateUpdate carrying a valid blsPop; the alternative is a fork-block state migration that drops BLS pubkeys without accompanying PoPs from ActiveCandidates. IIP draft will document the chosen approach. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(staking): reject duplicate BLS pubkey at candidate register/update Companion to the BLS proof-of-possession fix. Even with PoP in place, nothing in the current handler stops two different candidates from registering with the same blsPubKey: the existing ContainsName / ContainsOwner / ContainsOperator checks have no BLS equivalent. Two delegates sharing one BLS pubkey breaks IIP-52's quorum-counting model. The signer bitmap counts both delegates as having voted, but FastAggregateVerify aggregates pubkeys as a set — one contribution per distinct pubkey. The committee size as seen by aggregation is N, the committee size as seen by the bitmap is N+1, and the second delegate's stake-weight effectively "votes for free." This is a quieter cousin of the rogue-key attack: the aggregation math still verifies; only the accounting is off. Adds CandidateCenter.ContainsBLSPubKey(blsPubKey, except) and surfaces it through CandidateStateManager. Implementation is a linear scan over candidates — registration / update are sparse calls and the active delegate set is bounded; the saved O(1) lookup is not worth maintaining a fourth index map across the change/base commit flow. Hooks into all three handler paths under the EnforceBLSPoP gate: - handleCandidateRegister: reject if blsPubKey is held by any candidate other than the incumbent (when re-registering against an existing owner-without-selfstake record). - handleCandidateUpdate: reject if blsPubKey is held by any candidate other than the one being updated. A candidate keeping its own pubkey across updates is allowed (except = c.GetIdentifier()). - handleCandidateUpdateByOperator: same as update-by-owner. Failures return ReceiptStatus_ErrCandidateConflict (matches existing collision semantics for name / operator). Gated by EnforceBLSPoP so pre-fork blocks replay unchanged. Test: TestCandidateCenter_ContainsBLSPubKey covers nil / empty / no-match / match-with-nil-except / match-against-self (allowed) / match-against- other (rejected). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(staking): bind update-path BLS PoP to identifier, not current owner PoP at candidate update was binding to c.Owner — the *current* owner. For post-Xingu candidates this drifts after CandidateTransferOwnership: the new owner has to know they're the new owner at signing time, and a PoP signed under the old owner during a concurrent transfer is rejected purely on a value mismatch that has nothing to do with key possession. c.GetIdentifier() is the stable handle for this purpose: - Post-Xingu non-collision (the common case): Identifier is set once at register time to the original owner address (generateCandidateID fast-paths to owner when it's free). So a PoP signed at register binding to owner == identifier, and a PoP signed at update binding to c.GetIdentifier() lines up with that same value, even if owner has since transferred. - Post-Xingu collision (edge case): Identifier is a hash-derived address. Register still binds to act.OwnerAddress() (signer can't predict the hash), updates use c.GetIdentifier(). The register-time PoP doesn't get re-verified later, so this asymmetry is harmless; inside the candidate's lifetime, all update PoPs use the same hash consistently. - Pre-Xingu: c.Identifier is nil and c.GetIdentifier() falls back to c.Owner — behavior identical to the prior code. Renamed the function parameter from ownerAddress to candidateID throughout (BLSPopSigningRoot, SignBLSPop, VerifyBLSPop) and updated the docstring to spell out what to pass at register vs update. The on-the-wire bytes are unchanged — only the conceptual binding shifts. Tests: renamed TestBLSPop_RejectWrongOwner to RejectWrongCandidateID; added TestBLSPop_StableAcrossOwnershipTransfer locking in the property that a PoP signed under the original owner still verifies under the same identifier after the candidate is transferred to a new owner. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(action): web3 ABI methods for BLS register/update with PoP PoP coverage in the Go action layer (envestcc/iotex-core#4854) wasn't visible from the web3 path: candidateRegisterWithBLS / candidateUpdateWithBLS ABI entries have no blsPop slot, so any tx routed through eth_sendRawTransaction and decoded via NewCandidateRegister/UpdateFromABIBinary silently dropped the field. Post-fork that would have produced txs the handler always rejects. Adds two new V2 ABI methods rather than mutating the existing selectors: - candidateRegisterWithBLSAndPoP (8 fields + blsPop + data) - candidateUpdateWithBLSAndPoP (4 fields + blsPubKey + blsPop) Why V2 instead of extending the existing methods: changing a parameter list changes the 4-byte function selector, breaking any tooling that hardcoded the old ABI. The legacy WithBLS entries stay working pre-fork (their handler path is unchanged), and post-fork they reject naturally for lacking PoP — coexistence with no selector churn. EthData routing picks the entry by data carried on the action: - WithBLS && len(blsPop) > 0 → V2 selector, calldata includes PoP - WithBLS → legacy selector, no PoP slot - otherwise → non-BLS legacy candidateRegister FromABIBinary recognises the V2 selector and decodes blsPop. Tests: extended TestCandidateRegisterABIEncodeAndDecode with a "with public key and PoP" subcase, and added TestCandidateUpdate / "ABI encode with PoP" — both confirm the codec round-trips a non-empty PoP through Pack + Unpack, locking in the property whose absence was the bug. Out of scope for this commit: e2e coverage that exercises the web3 path through to a post-fork handler (proves the new selector actually flows blsPop into Candidate.BLSPubKey assignment). Will follow once the EnforceBLSPoP gate gets switched on in an integration scenario. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(action): mirror PoP ABI methods in the .sol interface + sync warning The previous commit added candidateRegisterWithBLSAndPoP / candidateUpdateWithBLSAndPoP entries to native_staking_contract_abi.json but left native_staking_contract_interface.sol — the human-readable source-of-truth that clients / docs consume — out of sync. Adds the matching Solidity declarations and a header comment in both the .sol file and native_staking_contract_abi.go pointing out the non-obvious sharp edge: there is no Makefile target that regenerates the JSON from the .sol, so the two files must be kept in sync by hand on every ABI change. Out-of-sync edits silently misencode the web3 path — clients embed one function signature while the node decodes another. No runtime behaviour change; the JSON has been authoritative all along. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(staking): GetByBLSPubKey + strict nil-candidateID PoP contract Addresses two review threads on PR #4854: ## Comments 1 + 5: drop the `except` parameter Both envestcc and CoderZhi flagged that ContainsBLSPubKey(pubkey, except) bool pushed the "is this me?" decision into a generic helper that doesn't belong there. Replaces ContainsBLSPubKey(pubkey, except) bool with the broader GetByBLSPubKey(pubkey) *Candidate: callers receive the (possibly nil) holder and compare identifiers themselves. The three register / update handler call sites now read if holder := csm.GetByBLSPubKey(act.BLSPubKey()); holder != nil && holder.GetIdentifier().String() != c.GetIdentifier().String() { return ErrCandidateConflict } which is more explicit than the prior `except`-hiding API. It also unifies with the same GetByBLSPubKey method added in #4857 (Y4a) so the two PRs don't introduce parallel BLS-pubkey-lookup APIs. ## Comments 2 + 4: strict nil-candidateID contract Both BLSPopSigningRoot and the surrounding Sign / Verify helpers used to silently accept a nil candidateID by skipping the candidate-binding write. That degrades the scheme to domain+pubkey-only — exactly the shape an attacker reaching for a cross-candidate replay would hope for. The three entry points now refuse: - BLSPopSigningRoot returns nil - SignBLSPop returns error("nil candidate ID; PoP must bind to a candidate identity") - VerifyBLSPop rejects before any cryptographic work TestBLSPop_RejectNilCandidateID locks the contract in place. ## Not in this commit Comment 3 ("blsPubKey can be removed from the signing root") — deferred. Will reply on the thread; the short answer is that the IRTF BLS draft defines canonical PoP as sign(sk, pk) and dropping blsPubKey from the digest opens up same-message aggregation when owner-uniqueness is ever relaxed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(staking): drop blsPubKey from PoP signing root Addresses Comment 3 on PR #4854 (CoderZhi). The previous signing root included blsPubKey: H(blsPopDomain || blsPubKey || candidateID) per the IRTF BLS canonical PoP form. After discussion, switching to: H(blsPopDomain || candidateID) The pairing verifier Verify(PK, msg, sig) already commits PK into the signature equation, so the basic rogue-key registration ("register pk_rogue without owning its discrete log") is blocked by basic PoP correctness without needing pubkey-in-message. Same-message aggregation — the classical attack that pubkey-in-message defends against — requires two distinct honest signers to sign the same signing root; the protocol-enforced uniqueness of candidate identifiers rules that out. Cross-candidate replay is still blocked by the candidateID binding, and cross-domain replay by blsPopDomain. The simpler signing root keeps the on-the-wire calldata independent of pubkey-length/encoding choices and removes an apparent redundancy between the signed message and the verifier's pk argument. SignBLSPop no longer derives pk from sk; the only caller-provided binding is candidateID. The bls_pop bytes themselves are unchanged in length and DST. Updated TestBLSPop_RogueKeyAttackBlocked and TestBLSPop_RejectNilCandidateID to use the new BLSPopSigningRoot(candidateID) signature. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(ioctl): BLS PoP UX for stake2 register/update + account bls-sign-pop Wires the three-option model agreed on PR #4854: how does a delegate actually supply the BLS proof-of-possession the post-fork handler now requires? ## stake2 register / update BREAKING positional change: BLS_PUBKEY moves out of the positional arg list. Pre-fork callers passing it positionally must migrate; the post-fork handler requires PoP anyway so this command was always going to need re-tooling. Three-option matrix lives in blspop_helper.go and applies to both register (mandatory BLS) and update (optional BLS, default = don't touch): Option 1 — auto-derive from signer (default for register): No --bls-* flags. The tool decrypts the signer keystore, derives a BLS key from the ECDSA scalar (same algorithm as `ioctl account blskey` — crypto.GenerateBLS12381PrivateKey(ecdsa_sk.Bytes())), computes the PoP, and prompts the user to confirm. The prompt is explicit about ECDSA↔BLS coupling so delegates aren't surprised when an ECDSA leak also exposes their producer identity. --yes suppresses for CI. Option 2 — --bls-priv-key HEX: Standalone BLS key supplied directly. Tool derives the pubkey and signs the PoP. No prompt. Option 3 — --bls-pubkey HEX + --bls-pop HEX: User supplies both. Tool VerifyBLSPop's locally against the candidateID before submission so a malformed PoP fails fast without burning gas. For register the candidateID is the owner address from positional args; for update it must be passed via --candidate-id. Option 0 (update only): no BLS flags at all → BLS untouched. Going from "leave BLS alone" to "rotate to derived key" requires explicit --bls-from-signer so a name/operator update can never silently rotate the producer key. Mutual-exclusion + completeness rules are centralised in blsPoPFlags.classifyForRegister / classifyForUpdate and locked in by TestBlsPoPFlags_ClassifyForRegister / ClassifyForUpdate. --bls-keystore is a placeholder; the iotex BLS keystore format isn't specified yet. Tool returns an explicit error pointing at --bls-priv-key. ## account bls-sign-pop New offline PoP signer for air-gapped workflows. Accepts either --bls-priv-key HEX or --signer ADDRESS (ECDSA keystore → derive), plus --candidate-id ADDRESS. Writes the 96-byte PoP hex to stdout (informational fields go to stderr so shell redirection captures only the PoP). The delegate signs on the cold machine, transfers the hex over USB / QR, and runs `ioctl stake2 register --bls-pubkey ... --bls-pop ...` on a hot machine — the BLS private key never leaves cold storage. Lives in the account package next to the existing `account blskey` so BLS key operations are co-located rather than scattered. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(staking): handler + e2e coverage for BLS PoP gate PR #4854 had no handler-level integration coverage of the EnforceBLSPoP gate — only the underlying SignBLSPop / VerifyBLSPop unit tests in bls_pop_test.go — and no e2etest of the post-fork register path at all. Reviewers couldn't confirm the three handler sites actually fail-closed under the gate without running the full chain manually. ## Handler integration (action/protocol/staking/handlers_blspop_test.go) Three new test functions, 11 subcases: - TestHandleCandidateRegister_PoPGate - gate on + valid PoP → ReceiptStatus_Success, c.BLSPubKey persisted - gate on + empty PoP → ErrUnauthorizedOperator - gate on + PoP signed under wrong key → ErrUnauthorizedOperator - gate off + empty PoP → Success (pre-fork behaviour preserved) - TestHandleCandidateRegister_BLSPubKeyUniqueness - second candidate registering the same BLS pubkey with a valid PoP under its own owner → ErrCandidateConflict; the GetByBLSPubKey uniqueness check protects IIP-52's quorum-counting invariant against the "shared keypair" silent break - TestHandleCandidateUpdate_PoPGate - gate on + valid PoP under c.GetIdentifier() → rotation succeeds, new BLSPubKey in state - gate on + empty PoP → ErrUnauthorizedOperator - gate on + PoP signed under wrong candidateID → ErrUnauthorizedOperator - gate off + empty PoP → rotation succeeds (pre-fork compat) The fixture flips EnforceBLSPoP via genesis.ToBeEnabledBlockHeight (0 / math.MaxUint64) and forces XinguBlockHeight = 0 so the BLS registration codepath is reachable at the test block. ## e2etest (e2etest/native_staking_test.go) TestCandidateBLSPoP exercises the full chain pipeline — encode → submit → mint → handler → receipt → state — with four post-fork subcases plus a pre-fork backward-compat anchor: 1. Pre-fork register without PoP succeeds 2. Post-fork register with valid PoP succeeds, BlsPubKey persists 3. Post-fork register without PoP returns ErrUnauthorizedOperator 4. Post-fork update rotates BLS pubkey with valid PoP 5. Post-fork update without PoP returns ErrUnauthorizedOperator and leaves the previously rotated pubkey untouched in state Genesis wires ToBeEnabledBlockHeight == XinguBlockHeight so the BLS register path and the PoP gate activate together — the same shape the planned fork rollout will use. Pre-existing TestProtocol_FetchBucketAndValidate flake (master and this branch both flap ~1/3) is unrelated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(staking): make the candidate BLS key optional on this fork Xingu made blsPubKey mandatory on candidate register and update. Nothing consumes it until IIP-52 aggregation activates, so it becomes optional again on the same fork that turns on EnforceBLSPoP, behind a new OptionalCandidateBLSPublicKey gate. Register now has three eras rather than two: pre-Xingu no BLS fields, amount in the legacy field Xingu..fork BLS mandatory, amount in value post-fork BLS optional A registration that supplies a key still uses the value-carrying ABI. One that omits it has no such method to call -- candidateRegisterWithBLS* both reject an empty blsPubKey at decode time -- so it goes back through the legacy candidateRegister entry and its amount lands in the legacy field. Amount() already keys off WithBLS(), so both conventions coexist without further plumbing. Update needs no such split: candidateUpdate carries no amount. An update that omits the key leaves any previously registered one untouched; the handler's write is already inside `if act.WithBLS()`. Validation only rejects a PoP arriving without a key, which no ABI method can produce but is cheap to exclude. The mirror case, a key with no PoP, stays with the handlers: VerifyBLSPop already rejects it as ErrUnauthorizedOperator on the receipt, and checking it here as well would move that rejection out of the block entirely and stop charging gas for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * deps: use the tagged iotex-proto v0.6.12 instead of a fork This branch pinned iotex-proto to envestcc/iotex-proto bls-pop (7a486f6a453d) because blsPop had nowhere else to live. It has since been merged upstream (iotexproject/iotex-proto#178) and released in v0.6.12, so the replace directive can go. Beyond tidiness this was a real blocker: the fork branches from a commit that predates v0.6.10, so it lacks the IIP-59 voter reward messages. Any branch carrying both this PR and #4953 could satisfy only one of them at a time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(ioctl): allow registering a candidate without a BLS key The key is optional from this fork on, but stake2 register could only auto-derive one: blsModeNone existed for update only, so there was no way to register without publishing a key. Adds --no-bls, mutually exclusive with the other BLS flags. Auto-derive stays the default, so existing invocations are unchanged and opting out is explicit. A keyless registration is built with NewCandidateRegister rather than NewCandidateRegisterWithBLS. The stake rides in value only when a key is present; the WithBLS shape with an empty key would leave value set and amount nil, and Amount() -- which keys off WithBLS() -- would return nil, panicking validation on the first Cmp. The constructor refuses an empty key for the same reason, so this is belt and braces, and the test pins both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(staking): reject BLS PoP before the fork activates Blocker 1 of the consensus review of #4854: a node that understands blsPop accepts blocks that a node without it rejects, at any pre-fork height an attacker picks. Two independent routes. ABI: an old node does not know selectors 0x370c13df / 0x80980508. Every New*FromABIBinary in the builder.go fallthrough chain returns errDecodeFailure, txContainer Unfold fails and the block is rejected. A new node decodes the call, skips VerifyBLSPop because EnforceBLSPoP is off, and executes it -- writing BLSPubKey to state. Native protobuf: an old node's iotex-proto has no blsPop field (it landed in v0.6.12). LoadProto copies known fields into the Go struct and Proto() rebuilds the message from those, so the field does not survive the round trip; envelopeHash differs and signature verification fails there while it passes here. One rule closes both. Decoding either V2 selector implies a non-empty blsPop -- both reject an empty one at decode time -- so "the V2 call decoded" and "blsPop is non-empty" are the same statement. Rejecting a non-empty blsPop while the gate is off therefore rejects exactly the divergent actions, on both paths. The check sits in validation, not in the handler: a handler error writes a failure receipt, which is itself a divergence from the old node's outright block rejection, while a validation error aborts workingSet.process before any receipt exists. It sits outside the era switch because that switch tests WithBLS(), which reads the public key and never the PoP. Also nests blsPop under the pubkey in CandidateUpdate.Proto(), matching CandidateRegister.Proto() and CandidateUpdate.LoadProto(). Tests pin the two properties the rule depends on: the V2 selectors reject an empty PoP, and an action without a PoP serialises exactly as it did before the field existed, so it hashes identically across the upgrade. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(staking): make the duplicate BLS key check order-independent Blocker 2 of the consensus review of #4854. GetByBLSPubKey named a holder by returning the first match from All(), which walks candBase.identifierMap -- a Go map, whose iteration order Go randomises per process. Every caller then compared that holder against itself to choose between Success and ErrCandidateConflict. Two candidates can already share a BLS pubkey: nothing forbids it before the uniqueness rule activates, and it can be arranged deliberately ahead of the fork. For such a pair, a node that saw holder A and a node that saw holder B wrote different receipt statuses for the same block, and therefore different receipt roots. The added handler test reproduces it on the pre-fix tree: 60 identical rounds split 52 Success / 8 ErrCandidateConflict. Replaced with HasBLSPubKeyOtherThan(pubkey, self), which asks about the whole set instead of naming one member. For holders {A, B} the answer is true for A, for B and for any third party alike, so it cannot depend on which one is seen first. This also matches ContainsName and ContainsOperator, the collision checks either side of it at all three call sites, which are already existence predicates. Behaviour changes only for an existing duplicate pair, where a holder re-asserting the shared key is now always rejected rather than accepted half the time. That is the safe direction: carrying a duplicate forward is exactly what breaks IIP-52's quorum counting, since FastAggregateVerify sums the pubkey set as a set while the signer bitmap counts both delegates. GetByBLSPubKey is removed rather than left deterministic: it is new in this PR, so nothing outside depends on it, and a lookup that names one of several holders has no correct use here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why split Y4 into Y4a + Y4b
Y4's full scope touches 10+ call sites of `blkCtx.Producer` / `blk.PublicKey().Address()` across rewarding, EVM, slasher, consensus, blockchain validation, API, and indexer. Landing them all in one PR makes review hard and risks regression in unrelated paths.
Y4a establishes the data plumbing (new BlockCtx field + state lookup helper) without touching any consumer. Y4b then migrates each consumer behind the activation gate.
What
1. `BlockCtx.ProducerPubKey []byte`
Raw producer pubkey bytes populated at BlockCtx assembly. Pre-fork = secp256k1 (33/65 B), post-fork = BLS12-381 (48 B). Consumers that need to match against `state.Candidate.BLSPubKey` use this rather than `Producer.String()`, since iotex-address derivation is undefined for a BLS pubkey.
2. `CandidateCenter.GetByBLSPubKey` + manager method
Linear scan returning the candidate whose registered `BLSPubKey` matches the given bytes. Mirrors the existing `GetByName` / `GetByOwner` / `GetByOperator` pattern. Linear is fine — registration / update are sparse and the active candidate set is bounded; not worth maintaining another index map across the change / base commit flow.
3. blockchain.go BlockCtx assembly
The three BlockCtx-construction sites now populate `ProducerPubKey`:
`contextWithBlock` helper signature is extended with `producerPubKey []byte`. The two callers already had access to the right source.
What's NOT in this PR (Y4b territory)
Test plan
🤖 Generated with Claude Code