diff --git a/core/parsigex/parsigex.go b/core/parsigex/parsigex.go index 502bcfc6a..9f4ab053f 100644 --- a/core/parsigex/parsigex.go +++ b/core/parsigex/parsigex.go @@ -40,7 +40,7 @@ func Protocols() []protocol.ID { } func NewParSigEx(p2pNode host.Host, sendFunc p2p.SendFunc, peerIdx int, peers []peer.ID, - verifyFunc func(context.Context, core.Duty, core.PubKey, core.ParSignedData) error, + verifyFunc func(context.Context, peer.ID, core.Duty, core.PubKey, core.ParSignedData) error, gaterFunc core.DutyGaterFunc, p2pOpts ...p2p.SendRecvOption, ) *ParSigEx { parSigEx := &ParSigEx{ @@ -72,12 +72,12 @@ type ParSigEx struct { sendFunc p2p.SendFunc peerIdx int peers []peer.ID - verifyFunc func(context.Context, core.Duty, core.PubKey, core.ParSignedData) error + verifyFunc func(context.Context, peer.ID, core.Duty, core.PubKey, core.ParSignedData) error gaterFunc core.DutyGaterFunc subs []func(context.Context, core.Duty, core.ParSignedDataSet) error } -func (m *ParSigEx) handle(ctx context.Context, _ peer.ID, req proto.Message) (proto.Message, bool, error) { +func (m *ParSigEx) handle(ctx context.Context, sender peer.ID, req proto.Message) (proto.Message, bool, error) { pb, ok := req.(*pbv1.ParSigExMsg) if !ok { return nil, false, errors.New("invalid request type") @@ -110,7 +110,7 @@ func (m *ParSigEx) handle(ctx context.Context, _ peer.ID, req proto.Message) (pr verifyStart := time.Now() for pubkey, data := range set { - if err = m.verifyFunc(ctx, duty, pubkey, data); err != nil { + if err = m.verifyFunc(ctx, sender, duty, pubkey, data); err != nil { return nil, false, errors.Wrap(err, "invalid partial signature") } } @@ -165,8 +165,10 @@ func (m *ParSigEx) Subscribe(fn func(context.Context, core.Duty, core.ParSignedD } // NewEth2Verifier returns a partial signature verification function for core workflow eth2 signatures. -func NewEth2Verifier(eth2Cl eth2wrap.Client, pubSharesByKey map[core.PubKey]map[int]tbls.PublicKey) (func(context.Context, core.Duty, core.PubKey, core.ParSignedData) error, error) { - return func(ctx context.Context, duty core.Duty, pubkey core.PubKey, data core.ParSignedData) error { +// Core workflow partial signatures are verified cryptographically against the pubshare for the +// claimed share index, so the authenticated sender is not needed here. +func NewEth2Verifier(eth2Cl eth2wrap.Client, pubSharesByKey map[core.PubKey]map[int]tbls.PublicKey) (func(context.Context, peer.ID, core.Duty, core.PubKey, core.ParSignedData) error, error) { + return func(ctx context.Context, _ peer.ID, duty core.Duty, pubkey core.PubKey, data core.ParSignedData) error { pubshares, ok := pubSharesByKey[pubkey] if !ok { return errors.New("unknown pubkey, not part of cluster lock") diff --git a/core/parsigex/parsigex_test.go b/core/parsigex/parsigex_test.go index 41a0a7b16..413815003 100644 --- a/core/parsigex/parsigex_test.go +++ b/core/parsigex/parsigex_test.go @@ -73,7 +73,7 @@ func TestParSigEx(t *testing.T) { } } - verifyFunc := func(context.Context, core.Duty, core.PubKey, core.ParSignedData) error { + verifyFunc := func(context.Context, peer.ID, core.Duty, core.PubKey, core.ParSignedData) error { return nil } @@ -171,7 +171,7 @@ func TestParSigExVerifier(t *testing.T) { att.Deneb.Signature = sign(sigData[:]) data, err := core.NewPartialVersionedAttestation(att, shareIdx) require.NoError(t, err) - require.NoError(t, verifyFunc(ctx, core.NewAttesterDuty(slot), pubkey, data)) + require.NoError(t, verifyFunc(ctx, "", core.NewAttesterDuty(slot), pubkey, data)) }) t.Run("Verify proposal", func(t *testing.T) { @@ -186,7 +186,7 @@ func TestParSigExVerifier(t *testing.T) { data, err := core.NewPartialVersionedSignedProposal(proposal, shareIdx) require.NoError(t, err) - require.NoError(t, verifyFunc(ctx, core.NewProposerDuty(slot), pubkey, data)) + require.NoError(t, verifyFunc(ctx, "", core.NewProposerDuty(slot), pubkey, data)) }) t.Run("Verify blinded proposal", func(t *testing.T) { @@ -204,7 +204,7 @@ func TestParSigExVerifier(t *testing.T) { data, err := core.NewPartialVersionedSignedBlindedProposal(ð2apiBlinded, shareIdx) require.NoError(t, err) - require.NoError(t, verifyFunc(ctx, core.NewProposerDuty(slot), pubkey, data)) + require.NoError(t, verifyFunc(ctx, "", core.NewProposerDuty(slot), pubkey, data)) }) t.Run("Verify Randao", func(t *testing.T) { @@ -216,7 +216,7 @@ func TestParSigExVerifier(t *testing.T) { randao := core.NewPartialSignedRandao(epoch, sign(sigData[:]), shareIdx) - require.NoError(t, verifyFunc(ctx, core.NewRandaoDuty(slot), pubkey, randao)) + require.NoError(t, verifyFunc(ctx, "", core.NewRandaoDuty(slot), pubkey, randao)) }) t.Run("Verify Voluntary Exit", func(t *testing.T) { @@ -232,7 +232,7 @@ func TestParSigExVerifier(t *testing.T) { require.NoError(t, err) - require.NoError(t, verifyFunc(ctx, core.NewVoluntaryExit(slot), pubkey, data)) + require.NoError(t, verifyFunc(ctx, "", core.NewVoluntaryExit(slot), pubkey, data)) }) t.Run("Verify validator registration", func(t *testing.T) { @@ -249,7 +249,7 @@ func TestParSigExVerifier(t *testing.T) { data, err := core.NewPartialVersionedSignedValidatorRegistration(®.VersionedSignedValidatorRegistration, shareIdx) require.NoError(t, err) - require.NoError(t, verifyFunc(ctx, core.NewBuilderRegistrationDuty(slot), pubkey, data)) + require.NoError(t, verifyFunc(ctx, "", core.NewBuilderRegistrationDuty(slot), pubkey, data)) }) t.Run("Verify beacon committee selection", func(t *testing.T) { @@ -263,7 +263,7 @@ func TestParSigExVerifier(t *testing.T) { selection.SelectionProof = sign(sigData[:]) data := core.NewPartialSignedBeaconCommitteeSelection(selection, shareIdx) - require.NoError(t, verifyFunc(ctx, core.NewPrepareAggregatorDuty(slot), pubkey, data)) + require.NoError(t, verifyFunc(ctx, "", core.NewPrepareAggregatorDuty(slot), pubkey, data)) }) t.Run("Verify aggregate and proof", func(t *testing.T) { @@ -286,7 +286,7 @@ func TestParSigExVerifier(t *testing.T) { agg.Deneb.Signature = sign(sigData[:]) data := core.NewPartialVersionedSignedAggregateAndProof(agg, shareIdx) - require.NoError(t, verifyFunc(ctx, core.NewAggregatorDuty(slot), pubkey, data)) + require.NoError(t, verifyFunc(ctx, "", core.NewAggregatorDuty(slot), pubkey, data)) }) t.Run("verify sync committee message", func(t *testing.T) { @@ -299,11 +299,11 @@ func TestParSigExVerifier(t *testing.T) { msg.Signature = sign(sigData[:]) data := core.NewPartialSignedSyncMessage(msg, shareIdx) - require.NoError(t, verifyFunc(ctx, core.NewSyncMessageDuty(slot), pubkey, data)) + require.NoError(t, verifyFunc(ctx, "", core.NewSyncMessageDuty(slot), pubkey, data)) // Invalid sync committee message. data = core.NewPartialSignedRandao(epoch, testutil.RandomEth2Signature(), shareIdx) - err = verifyFunc(ctx, core.NewSyncMessageDuty(slot), pubkey, data) + err = verifyFunc(ctx, "", core.NewSyncMessageDuty(slot), pubkey, data) require.Error(t, err) require.ErrorContains(t, err, "invalid signature") }) @@ -326,7 +326,7 @@ func TestParSigExVerifier(t *testing.T) { parSigData := core.NewPartialSignedSyncCommitteeSelection(selection, shareIdx) - require.NoError(t, verifyFunc(ctx, core.NewPrepareSyncContributionDuty(slot), pubkey, parSigData)) + require.NoError(t, verifyFunc(ctx, "", core.NewPrepareSyncContributionDuty(slot), pubkey, parSigData)) }) t.Run("verify sync committee contribution and proof", func(t *testing.T) { @@ -343,7 +343,7 @@ func TestParSigExVerifier(t *testing.T) { parSigData := core.NewPartialSignedSyncContributionAndProof(proof, shareIdx) - require.NoError(t, verifyFunc(ctx, core.NewPrepareSyncContributionDuty(slot), pubkey, parSigData)) + require.NoError(t, verifyFunc(ctx, "", core.NewPrepareSyncContributionDuty(slot), pubkey, parSigData)) }) } diff --git a/dkg/dkg.go b/dkg/dkg.go index 6e15c735e..20ac6441a 100644 --- a/dkg/dkg.go +++ b/dkg/dkg.go @@ -241,24 +241,27 @@ func Run(ctx context.Context, conf Config) (err error) { return errors.Wrap(err, "get peer IDs") } - ex := newExchanger(p2pNode, nodeIdx.PeerIdx, peerIDs, []sigType{ - sigLock, - sigDepositData, - sigValidatorRegistration, - }, conf.Timeout) - - // Register libp2p handlers peerMap := make(map[peer.ID]cluster.NodeIdx) for _, p := range peers { - nodeIdx, err := def.NodeIdx(p.ID) + idx, err := def.NodeIdx(p.ID) if err != nil { return err } - peerMap[p.ID] = nodeIdx + peerMap[p.ID] = idx } + ex, err := newExchanger(p2pNode, nodeIdx.PeerIdx, peerIDs, peerMap, []sigType{ + sigLock, + sigDepositData, + sigValidatorRegistration, + }, conf.Timeout) + if err != nil { + return err + } + + // Register libp2p handlers caster := bcast.New(p2pNode, peerIDs, key) // register bcast callbacks for frostp2p diff --git a/dkg/exchanger.go b/dkg/exchanger.go index 04ca2f865..a8571a7d1 100644 --- a/dkg/exchanger.go +++ b/dkg/exchanger.go @@ -14,6 +14,7 @@ import ( "github.com/obolnetwork/charon/app/errors" "github.com/obolnetwork/charon/app/z" + "github.com/obolnetwork/charon/cluster" "github.com/obolnetwork/charon/core" "github.com/obolnetwork/charon/core/parsigdb" "github.com/obolnetwork/charon/core/parsigex" @@ -67,10 +68,25 @@ type exchanger struct { dutyGaterFunc func(duty core.Duty) bool } -func newExchanger(p2pNode host.Host, peerIdx int, peers []peer.ID, sigTypes []sigType, timeout time.Duration) *exchanger { - // Partial signature roots not known yet, so skip verification in parsigex, rather verify before we aggregate. - noopVerifier := func(context.Context, core.Duty, core.PubKey, core.ParSignedData) error { - return nil +func newExchanger(p2pNode host.Host, peerIdx int, peers []peer.ID, peerMap map[peer.ID]cluster.NodeIdx, sigTypes []sigType, timeout time.Duration) (*exchanger, error) { + if peerIdx < 0 || peerIdx >= len(peers) { + return nil, errors.New("peer index out of range", z.Int("peer_idx", peerIdx), z.Int("num_peers", len(peers))) + } + + // Every peer must have a valid share index in peerMap, otherwise its partial signatures would be + // rejected as unknown and the exchange would silently time out. Fail fast on a misconfigured map. + for _, p := range peers { + if nodeIdx, ok := peerMap[p]; !ok || nodeIdx.ShareIdx <= 0 { + return nil, errors.New("peer map missing valid share index for peer", z.Str("peer", p.String())) + } + } + + // Partial signature roots are not known during the exchange, so cryptographic verification is + // deferred until aggregation. Each received partial signature is still bound to its authenticated + // sender: a peer may only contribute partial signatures under its own assigned share index, + // consistent with the sender check in nodesigs.go. + verifyShareIdx := func(_ context.Context, sender peer.ID, _ core.Duty, _ core.PubKey, data core.ParSignedData) error { + return verifyPeerShareIdx(peerMap, sender, data) } st := make(map[sigType]bool) @@ -94,7 +110,7 @@ func newExchanger(p2pNode host.Host, peerIdx int, peers []peer.ID, sigTypes []si ex := &exchanger{ // threshold is len(peers) to wait until we get all the partial sigs from all the peers per DV sigdb: parsigdb.NewMemDB(len(peers), noopDeadliner{}, parsigdb.NewMemDBMetadata(0, time.Now())), // metadata timestamps are used for metrics, irrelevant for DKG - sigex: parsigex.NewParSigEx(p2pNode, p2p.Send, peerIdx, peers, noopVerifier, dutyGaterFunc, p2p.WithSendTimeout(timeout), p2p.WithReceiveTimeout(timeout)), + sigex: parsigex.NewParSigEx(p2pNode, p2p.Send, peerIdx, peers, verifyShareIdx, dutyGaterFunc, p2p.WithSendTimeout(timeout), p2p.WithReceiveTimeout(timeout)), sigTypes: st, sigData: dataByPubkey{ store: sigTypeStore{}, @@ -108,7 +124,25 @@ func newExchanger(p2pNode host.Host, peerIdx int, peers []peer.ID, sigTypes []si ex.sigdb.SubscribeThreshold(ex.pushPsigs) ex.sigex.Subscribe(ex.sigdb.StoreExternal) - return ex + return ex, nil +} + +// verifyPeerShareIdx checks that a received partial signature originates from a known peer and uses +// that peer's assigned share index. peerMap maps each participating peer to its node index, so the +// check remains correct when share indices are not contiguous with peer positions (for example when +// operators have been removed and the remaining ones keep their original share indices). +func verifyPeerShareIdx(peerMap map[peer.ID]cluster.NodeIdx, sender peer.ID, data core.ParSignedData) error { + nodeIdx, ok := peerMap[sender] + if !ok { + return errors.New("partial signature from unknown peer", z.Str("peer", sender.String())) + } + + if data.ShareIdx <= 0 || data.ShareIdx != nodeIdx.ShareIdx { + return errors.New("partial signature share index does not match sender peer", + z.Str("peer", sender.String()), z.Int("share_idx", data.ShareIdx), z.Int("expected_share_idx", nodeIdx.ShareIdx)) + } + + return nil } // exchange exchanges partial signatures of lockhash/deposit-data among dkg participants and returns all the partial diff --git a/dkg/exchanger_internal_test.go b/dkg/exchanger_internal_test.go index b2ce77fcb..a68f330f2 100644 --- a/dkg/exchanger_internal_test.go +++ b/dkg/exchanger_internal_test.go @@ -14,10 +14,22 @@ import ( "github.com/libp2p/go-libp2p/core/peerstore" "github.com/stretchr/testify/require" + "github.com/obolnetwork/charon/cluster" "github.com/obolnetwork/charon/core" "github.com/obolnetwork/charon/testutil" ) +// positionalPeerMap builds a peer-to-node-index map where each peer's share index is its position +// plus one, matching the default cluster layout. +func positionalPeerMap(peers []peer.ID) map[peer.ID]cluster.NodeIdx { + peerMap := make(map[peer.ID]cluster.NodeIdx, len(peers)) + for i, p := range peers { + peerMap[p] = cluster.NodeIdx{PeerIdx: i, ShareIdx: i + 1} + } + + return peerMap +} + // TODO(dhruv): add tests for negative scenarios (take inspiration from core/qbft/qbft_internal_test). func TestExchanger(t *testing.T) { ctx := context.Background() @@ -94,7 +106,9 @@ func TestExchanger(t *testing.T) { } for i := range nodes { - ex := newExchanger(hosts[i], i, peers, expectedSigTypes, 8*time.Second) + ex, err := newExchanger(hosts[i], i, peers, positionalPeerMap(peers), expectedSigTypes, 8*time.Second) + require.NoError(t, err) + exchangers = append(exchangers, ex) } @@ -189,7 +203,8 @@ func TestExchangerPushPsigsNeverBlocks(t *testing.T) { h := testutil.CreateHost(t, testutil.AvailableAddr(t)) peers := []peer.ID{h.ID()} - ex := newExchanger(h, 0, peers, []sigType{sigLock}, time.Second) + ex, err := newExchanger(h, 0, peers, positionalPeerMap(peers), []sigType{sigLock}, time.Second) + require.NoError(t, err) duty := core.NewSignatureDuty(uint64(sigLock)) @@ -224,3 +239,164 @@ func TestExchangerPushPsigsNeverBlocks(t *testing.T) { t.Fatal("pushPsigs deadlocked") } } + +// TestExchangerRejectsMismatchedShareIndex verifies the sender->shareIdx binding in the lock-hash +// exchange: a peer may only contribute partial signatures under its own share index. +// +// The exchange defers cryptographic verification until aggregation, so partial signatures are +// accepted at receive time without checking their signature. newExchanger therefore binds each +// received partial signature to its authenticated sender (shareIdx == peerIdx+1), consistent with +// the sender check in nodesigs.go. +// +// This drives the real /charon/parsigex/2.0.0 receive path: one node broadcasts partial signatures +// for a pubkey outside the cluster under every share index. Every share index other than that +// node's own is rejected, so the outside pubkey never reaches the threshold, while the honest +// exchange for the real validator completes with a full set of shares. +func TestExchangerRejectsMismatchedShareIndex(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + // threshold = len(peers), so reaching it for a single pubkey needs this many distinct share + // indices, which one sender can no longer supply on its own. + const nodes = 4 + + var ( + peers []peer.ID + hosts []host.Host + hostsInfo []peer.AddrInfo + ) + + for range nodes { + h := testutil.CreateHost(t, testutil.AvailableAddr(t)) + hostsInfo = append(hostsInfo, peer.AddrInfo{ID: h.ID(), Addrs: h.Addrs()}) + peers = append(peers, h.ID()) + hosts = append(hosts, h) + } + + // Connect every host to its peers so exchange broadcasts reach live handlers. + for i := range nodes { + for j := range nodes { + if i == j { + continue + } + + hosts[i].Peerstore().AddAddrs(hostsInfo[j].ID, hostsInfo[j].Addrs, peerstore.PermanentAddrTTL) + } + } + + exchangers := make([]*exchanger, nodes) + for i := range nodes { + ex, err := newExchanger(hosts[i], i, peers, positionalPeerMap(peers), []sigType{sigLock}, 8*time.Second) + require.NoError(t, err) + + exchangers[i] = ex + } + + realPk := testutil.RandomCorePubKey(t) // The cluster's genuine distributed validator. + outsidePk := testutil.RandomCorePubKey(t) // A pubkey that is not part of the cluster. + + // otherNode (peer index 1) has share index 2 as its own. It broadcasts partial signatures for + // outsidePk under every share index over the authenticated parsigex protocol. Every peer's + // sender->shareIdx binding must reject every share index other than otherNode's own, so outsidePk + // can never reach the threshold anywhere. + const otherNode = 1 + + sigLockDuty := core.NewSignatureDuty(uint64(sigLock)) + + for shareIdx := 1; shareIdx <= nodes; shareIdx++ { + set := core.ParSignedDataSet{ + outsidePk: core.NewPartialSignature(testutil.RandomCoreSignature(), shareIdx), + } + require.NoError(t, exchangers[otherNode].sigex.Broadcast(ctx, sigLockDuty, set)) + } + + // Every node runs the honest lock-hash exchange for the single real validator, each contributing + // only its own share. The real validator reaches the threshold legitimately and the exchange + // resolves with real data. + var wg sync.WaitGroup + + results := make([]map[core.PubKey][]core.ParSignedData, nodes) + errs := make([]error, nodes) + + for i := range nodes { + wg.Add(1) + + go func(node int) { + defer wg.Done() + + set := core.ParSignedDataSet{ + realPk: core.NewPartialSignature(testutil.RandomCoreSignature(), node+1), + } + results[node], errs[node] = exchangers[node].exchange(ctx, sigLock, set) + }(i) + } + + wg.Wait() + + // The outside pubkey never reached the threshold, so no exchange result contains it; the real + // validator is present with a full set of shares. + for i := range nodes { + require.NoErrorf(t, errs[i], "node %d exchange failed", i) + require.Containsf(t, results[i], realPk, "node %d missing the real validator", i) + require.Lenf(t, results[i][realPk], nodes, "node %d has an unexpected number of shares", i) + require.NotContainsf(t, results[i], outsidePk, "node %d accepted the outside pubkey", i) + } +} + +// TestVerifyPeerShareIdx covers the sender->shareIdx binding, including a non-contiguous layout where +// a peer's assigned share index does not equal its position (as when earlier operators are removed). +func TestVerifyPeerShareIdx(t *testing.T) { + const ( + self = peer.ID("self") + other = peer.ID("other") + unknown = peer.ID("unknown") + ) + + // "other" is the second peer but keeps share index 4, e.g. after operators with lower indices + // have been removed. + peerMap := map[peer.ID]cluster.NodeIdx{ + self: {PeerIdx: 0, ShareIdx: 1}, + other: {PeerIdx: 1, ShareIdx: 4}, + } + + tests := []struct { + name string + sender peer.ID + shareIdx int + wantErr string + }{ + {name: "own share index accepted", sender: self, shareIdx: 1}, + {name: "assigned non-contiguous share index accepted", sender: other, shareIdx: 4}, + {name: "mismatched share index rejected", sender: other, shareIdx: 2, wantErr: "share index does not match"}, + {name: "another peer's share index rejected", sender: self, shareIdx: 4, wantErr: "share index does not match"}, + {name: "non-positive share index rejected", sender: self, shareIdx: 0, wantErr: "share index does not match"}, + {name: "unknown sender rejected", sender: unknown, shareIdx: 1, wantErr: "unknown peer"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data := core.NewPartialSignature(testutil.RandomCoreSignature(), tt.shareIdx) + + err := verifyPeerShareIdx(peerMap, tt.sender, data) + if tt.wantErr == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tt.wantErr) + } + }) + } +} + +// TestNewExchangerRejectsIncompletePeerMap ensures construction fails fast when a peer has no valid +// share index, rather than silently rejecting its partial signatures and timing out. +func TestNewExchangerRejectsIncompletePeerMap(t *testing.T) { + h := testutil.CreateHost(t, testutil.AvailableAddr(t)) + peers := []peer.ID{h.ID(), peer.ID("missing")} + + peerMap := map[peer.ID]cluster.NodeIdx{ + h.ID(): {PeerIdx: 0, ShareIdx: 1}, + } + + _, err := newExchanger(h, 0, peers, peerMap, []sigType{sigLock}, time.Second) + require.ErrorContains(t, err, "missing valid share index") +} diff --git a/dkg/protocol_addoperators.go b/dkg/protocol_addoperators.go index 9028e96b8..3f8340d04 100644 --- a/dkg/protocol_addoperators.go +++ b/dkg/protocol_addoperators.go @@ -85,7 +85,12 @@ func (p *addOperatorsProtocol) PostInit(ctx context.Context, pctx *ProtocolConte p.allENRs = append(p.allENRs, p.newENRs...) - pctx.SigExchanger = newExchanger(pctx.ThisNode, pctx.ThisNodeIdx.PeerIdx, pctx.PeerIDs, []sigType{sigLock}, pctx.Config.Timeout) + sigEx, err := newExchanger(pctx.ThisNode, pctx.ThisNodeIdx.PeerIdx, pctx.PeerIDs, pctx.PeerMap, []sigType{sigLock}, pctx.Config.Timeout) + if err != nil { + return err + } + + pctx.SigExchanger = sigEx pctx.Caster = bcast.New(pctx.ThisNode, pctx.PeerIDs, pctx.ENRPrivateKey) pctx.NodeSigCaster = newNodeSigBcast(pctx.Peers, pctx.ThisNodeIdx, pctx.Caster) diff --git a/dkg/protocol_removeoperators.go b/dkg/protocol_removeoperators.go index 572f5078e..5d082cd1f 100644 --- a/dkg/protocol_removeoperators.go +++ b/dkg/protocol_removeoperators.go @@ -119,6 +119,12 @@ func (p *removeOperatorsProtocol) PostInit(ctx context.Context, pctx *ProtocolCo oldPeerIDs := make([]peer.ID, 0) newPeerIDs := make([]peer.ID, 0) + // exchangerPeerMap binds each remaining node to its original share index for the lock-hash + // exchange. Removed operators are excluded even when they still participate in the reshare, since + // they do not contribute lock-hash signatures; keeping them out prevents their authenticated peer + // ID from being used to submit one. + exchangerPeerMap := make(map[peer.ID]cluster.NodeIdx) + for i, op := range pctx.Lock.Operators { isOld := slices.Contains(p.oldENRs, op.ENR) if isOld { @@ -126,6 +132,9 @@ func (p *removeOperatorsProtocol) PostInit(ctx context.Context, pctx *ProtocolCo } else { newPeerIDs = append(newPeerIDs, allPeers[i].ID) p.operators = append(p.operators, op.ENR) + // ShareIdx is the 1-indexed original operator position, matching buildPeerMap and the + // index each remaining node signs with. + exchangerPeerMap[allPeers[i].ID] = cluster.NodeIdx{PeerIdx: i, ShareIdx: i + 1} } if pctx.ThisPeerID == allPeers[i].ID && isOld { @@ -146,12 +155,21 @@ func (p *removeOperatorsProtocol) PostInit(ctx context.Context, pctx *ProtocolCo if !p.oldNode { // SigExchanger is only created for nodes remaining in the cluster, because old nodes do not participate in signing. + // Remaining nodes keep their original share indices, so exchangerPeerMap (keyed by peer ID, + // with each node's original share index in the value) binds each partial signature to its + // sender's assigned share index. nodeIdx := slices.Index(newPeerIDs, pctx.ThisPeerID) pctx.ThisNodeIdx = cluster.NodeIdx{ PeerIdx: nodeIdx, ShareIdx: peerMap[pctx.ThisPeerID].ShareIdx, } - pctx.SigExchanger = newExchanger(pctx.ThisNode, nodeIdx, newPeerIDs, []sigType{sigLock}, pctx.Config.Timeout) + + sigEx, err := newExchanger(pctx.ThisNode, nodeIdx, newPeerIDs, exchangerPeerMap, []sigType{sigLock}, pctx.Config.Timeout) + if err != nil { + return err + } + + pctx.SigExchanger = sigEx } reshareConfig := pedersen.NewReshareConfig(len(pctx.Lock.Validators), p.newThreshold, nil, oldPeerIDs) diff --git a/dkg/protocol_replaceoperator.go b/dkg/protocol_replaceoperator.go index f9e82f976..ca0cfca26 100644 --- a/dkg/protocol_replaceoperator.go +++ b/dkg/protocol_replaceoperator.go @@ -103,7 +103,12 @@ func (p *replaceOperatorProtocol) GetPeers(lock *cluster.Lock) ([]p2p.Peer, erro } func (p *replaceOperatorProtocol) PostInit(ctx context.Context, pctx *ProtocolContext) error { - pctx.SigExchanger = newExchanger(pctx.ThisNode, pctx.ThisNodeIdx.PeerIdx, pctx.PeerIDs, []sigType{sigLock}, pctx.Config.Timeout) + sigEx, err := newExchanger(pctx.ThisNode, pctx.ThisNodeIdx.PeerIdx, pctx.PeerIDs, pctx.PeerMap, []sigType{sigLock}, pctx.Config.Timeout) + if err != nil { + return err + } + + pctx.SigExchanger = sigEx pctx.Caster = bcast.New(pctx.ThisNode, pctx.PeerIDs, pctx.ENRPrivateKey) pctx.NodeSigCaster = newNodeSigBcast(pctx.Peers, pctx.ThisNodeIdx, pctx.Caster) diff --git a/dkg/protocol_reshare.go b/dkg/protocol_reshare.go index 01a0c124b..0e6b14b09 100644 --- a/dkg/protocol_reshare.go +++ b/dkg/protocol_reshare.go @@ -49,7 +49,12 @@ func (*reshareProtocol) GetPeers(lock *cluster.Lock) ([]p2p.Peer, error) { } func (p *reshareProtocol) PostInit(ctx context.Context, pctx *ProtocolContext) error { - pctx.SigExchanger = newExchanger(pctx.ThisNode, pctx.ThisNodeIdx.PeerIdx, pctx.PeerIDs, []sigType{sigLock}, pctx.Config.Timeout) + sigEx, err := newExchanger(pctx.ThisNode, pctx.ThisNodeIdx.PeerIdx, pctx.PeerIDs, pctx.PeerMap, []sigType{sigLock}, pctx.Config.Timeout) + if err != nil { + return err + } + + pctx.SigExchanger = sigEx pctx.Caster = bcast.New(pctx.ThisNode, pctx.PeerIDs, pctx.ENRPrivateKey) pctx.NodeSigCaster = newNodeSigBcast(pctx.Peers, pctx.ThisNodeIdx, pctx.Caster) diff --git a/dkg/protocolsteps_internal_test.go b/dkg/protocolsteps_internal_test.go index 6571d0f76..85867f196 100644 --- a/dkg/protocolsteps_internal_test.go +++ b/dkg/protocolsteps_internal_test.go @@ -142,7 +142,8 @@ func TestUpdateLockProtocolStep(t *testing.T) { shares := valKeysToSharesNode0(t, valKeys, lock.Validators) host := testutil.CreateHost(t, testutil.AvailableAddr(t)) - sigex := newExchanger(host, 0, []peer.ID{host.ID()}, []sigType{sigLock}, 10*time.Second) + sigex, err := newExchanger(host, 0, []peer.ID{host.ID()}, positionalPeerMap([]peer.ID{host.ID()}), []sigType{sigLock}, 10*time.Second) + require.NoError(t, err) pctx := &ProtocolContext{ Lock: &lock, @@ -151,7 +152,7 @@ func TestUpdateLockProtocolStep(t *testing.T) { SigExchanger: sigex, ThisNodeIdx: cluster.NodeIdx{PeerIdx: 0, ShareIdx: 1}, } - err := step.Run(t.Context(), pctx) + err = step.Run(t.Context(), pctx) require.NoError(t, err) require.Equal(t, 4, pctx.Lock.Threshold)