diff --git a/universalClient/tss/coordinator/coordinator.go b/universalClient/tss/coordinator/coordinator.go index 55627b11..ed72c6e9 100644 --- a/universalClient/tss/coordinator/coordinator.go +++ b/universalClient/tss/coordinator/coordinator.go @@ -33,6 +33,7 @@ import ( type PushCoreClient interface { GetLatestBlock(ctx context.Context) (uint64, error) GetCurrentKey(ctx context.Context) (*utsstypes.TssKey, error) + GetKeyByID(ctx context.Context, keyID string) (*utsstypes.TssKey, error) GetAllUniversalValidators(ctx context.Context) ([]*types.UniversalValidator, error) } @@ -472,11 +473,13 @@ func (c *Coordinator) processConfirmedEvents(ctx context.Context) error { // For SIGN/FUND_MIGRATE: pick a random threshold subset (>2/3 of eligible) rather than all eligible. // A threshold subset suffices for signing and is more resilient when some nodes are offline. // For all other protocols (keygen, keyrefresh, quorum_change), all eligible must participate. - var participants []*types.UniversalValidator - if event.Type == store.EventTypeSignOutbound || event.Type == store.EventTypeSignFundMigrate { - participants = getSignParticipants(allValidators) - } else { - participants = getEligibleForProtocol(event.Type, allValidators) + participants, err := c.SelectParticipants(ctx, event, allValidators) + if err != nil { + c.logger.Error().Err(err). + Str("event_id", event.EventID). + Str("type", event.Type). + Msg("cannot select participants for event") + continue } if participants == nil { c.logger.Debug().Str("event_id", event.EventID).Str("type", event.Type).Msg("unknown protocol type") @@ -976,7 +979,7 @@ func getSignParticipants(allValidators []*types.UniversalValidator) []*types.Uni eligible := getSignEligible(allValidators) // Use utils function to select random threshold subset - return selectRandomThreshold(eligible) + return selectRandomThreshold(eligible, CalculateThreshold(len(eligible))) } // getInFlightSignCountPerChain returns per-chain in-flight SIGN count. @@ -1156,3 +1159,112 @@ func (c *Coordinator) assignFundMigrateNonce(ctx context.Context, event store.Ev return builder.GetNextNonce(ctx, oldTSSAddr, true) } + +// SelectParticipants picks who takes part in an event. +// +// For SIGN a random threshold subset (>2/3 of eligible) suffices and is more +// resilient when some nodes are offline. For all other protocols (keygen, +// keyrefresh, quorum change) every eligible validator must participate. +func (c *Coordinator) SelectParticipants( + ctx context.Context, + event store.Event, + allValidators []*types.UniversalValidator, +) ([]*types.UniversalValidator, error) { + switch event.Type { + case store.EventTypeSignOutbound: + return getSignParticipants(allValidators), nil + case store.EventTypeSignFundMigrate: + // Signed with the old key's shares, so the signers must be drawn from + // the validators that hold them rather than from whoever is eligible + // now. A newcomer selected here has no such share and never ACKs, so + // the session stalls waiting for a party that cannot take part. + return c.fundMigrateParticipants(ctx, event, allValidators) + default: + return getEligibleForProtocol(event.Type, allValidators), nil + } +} + +// FundMigrateEligible returns the validators that may sign a fund migration, +// and how many of them are required. +// +// Used by the coordinator to select signers and by every participant to +// validate the selection it receives. Both derive the answer from the same +// chain state, so a set the coordinator can legitimately pick is a set the +// participants accept. +func (c *Coordinator) FundMigrateEligible( + ctx context.Context, + event store.Event, +) ([]*types.UniversalValidator, int, error) { + return c.fundMigrateEligible(ctx, event, c.validatorsSnapshot()) +} + +// fundMigrateParticipants selects signers for a fund migration from the +// validators that hold the old key's shares. +func (c *Coordinator) fundMigrateParticipants( + ctx context.Context, + event store.Event, + allValidators []*types.UniversalValidator, +) ([]*types.UniversalValidator, error) { + holders, required, err := c.fundMigrateEligible(ctx, event, allValidators) + if err != nil { + return nil, err + } + return selectRandomThreshold(holders, required), nil +} + +// fundMigrateEligible resolves the eligible signers and the required count for +// a fund migration. +// +// The signature is produced with the old keyshare, so eligibility is decided by +// the historical shareholder set recorded on chain, not by who is a validator +// today. The required count is the old key's threshold for the same reason: it +// is the quorum that key was created under. +// +// Fails rather than returning a set that is already too small. Too few +// surviving shareholders means no subset can sign, and proceeding anyway would +// stall the session on an ACK that is never coming instead of reporting why. +// +// Nothing here can rebuild a lost quorum: an old key of N tolerates only +// N-threshold(N) departures, so migration must follow keygen promptly. +func (c *Coordinator) fundMigrateEligible( + ctx context.Context, + event store.Event, + allValidators []*types.UniversalValidator, +) ([]*types.UniversalValidator, int, error) { + var migrationData utsstypes.FundMigrationInitiatedEventData + if err := json.Unmarshal(event.EventData, &migrationData); err != nil { + return nil, 0, fmt.Errorf("parse fund migration data: %w", err) + } + if migrationData.OldKeyID == "" { + return nil, 0, fmt.Errorf("fund migration event carries no old key id") + } + + oldKey, err := c.pushCore.GetKeyByID(ctx, migrationData.OldKeyID) + if err != nil { + return nil, 0, fmt.Errorf("fetch old key %s: %w", migrationData.OldKeyID, err) + } + if oldKey == nil || len(oldKey.Participants) == 0 { + return nil, 0, fmt.Errorf("old key %s records no participants", migrationData.OldKeyID) + } + + shareholders := make(map[string]bool, len(oldKey.Participants)) + for _, p := range oldKey.Participants { + shareholders[p] = true + } + + var holders []*types.UniversalValidator + for _, v := range getSignEligible(allValidators) { + if v.IdentifyInfo != nil && shareholders[v.IdentifyInfo.CoreValidatorAddress] { + holders = append(holders, v) + } + } + + required := CalculateThreshold(len(oldKey.Participants)) + if len(holders) < required { + return nil, 0, fmt.Errorf( + "key %s needs %d of its %d shareholders to sign, only %d are still eligible", + migrationData.OldKeyID, required, len(oldKey.Participants), len(holders)) + } + + return holders, required, nil +} diff --git a/universalClient/tss/coordinator/coordinator_test.go b/universalClient/tss/coordinator/coordinator_test.go index 72daca8e..df8ce9aa 100644 --- a/universalClient/tss/coordinator/coordinator_test.go +++ b/universalClient/tss/coordinator/coordinator_test.go @@ -379,21 +379,21 @@ func TestSelectRandomThreshold(t *testing.T) { t.Run("returns exactly threshold count", func(t *testing.T) { // threshold(5) = 4 - assert.Len(t, selectRandomThreshold(makeN(5)), 4) + assert.Len(t, selectRandomThreshold(makeN(5), CalculateThreshold(5)), 4) }) t.Run("returns all when count equals threshold", func(t *testing.T) { // threshold(2) = 2 → returns all 2 - assert.Len(t, selectRandomThreshold(makeN(2)), 2) + assert.Len(t, selectRandomThreshold(makeN(2), CalculateThreshold(2)), 2) }) t.Run("returns all when count is below threshold", func(t *testing.T) { // threshold(1) = 1 → returns all 1 - assert.Len(t, selectRandomThreshold(makeN(1)), 1) + assert.Len(t, selectRandomThreshold(makeN(1), CalculateThreshold(1)), 1) }) t.Run("returns nil for empty list", func(t *testing.T) { - assert.Nil(t, selectRandomThreshold(nil)) + assert.Nil(t, selectRandomThreshold(nil, 3)) }) } @@ -1123,6 +1123,10 @@ type stalenessMockPushCore struct { block uint64 validators []*types.UniversalValidator failGetAll bool + + // Old key history, consulted when selecting fund migration signers. + keysByID map[string]*utsstypes.TssKey + keyErr error } func (m *stalenessMockPushCore) GetLatestBlock(_ context.Context) (uint64, error) { @@ -1133,6 +1137,13 @@ func (m *stalenessMockPushCore) GetCurrentKey(_ context.Context) (*utsstypes.Tss return &utsstypes.TssKey{KeyId: "test-key"}, nil } +func (m *stalenessMockPushCore) GetKeyByID(_ context.Context, keyID string) (*utsstypes.TssKey, error) { + if m.keyErr != nil { + return nil, m.keyErr + } + return m.keysByID[keyID], nil +} + func (m *stalenessMockPushCore) GetAllUniversalValidators(_ context.Context) ([]*types.UniversalValidator, error) { if m.failGetAll { return nil, fmt.Errorf("simulated GetAllUniversalValidators RPC failure") diff --git a/universalClient/tss/coordinator/fund_migrate_participants_test.go b/universalClient/tss/coordinator/fund_migrate_participants_test.go new file mode 100644 index 00000000..8bd91961 --- /dev/null +++ b/universalClient/tss/coordinator/fund_migrate_participants_test.go @@ -0,0 +1,276 @@ +package coordinator + +import ( + "context" + "encoding/json" + "fmt" + "testing" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pushchain/push-chain-node/universalClient/store" + utsstypes "github.com/pushchain/push-chain-node/x/utss/types" + "github.com/pushchain/push-chain-node/x/uvalidator/types" +) + +func activeValidator(addr string) *types.UniversalValidator { + return &types.UniversalValidator{ + IdentifyInfo: &types.IdentityInfo{CoreValidatorAddress: addr}, + LifecycleInfo: &types.LifecycleInfo{CurrentStatus: types.UVStatus_UV_STATUS_ACTIVE}, + } +} + +func validatorWithStatus(addr string, status types.UVStatus) *types.UniversalValidator { + return &types.UniversalValidator{ + IdentifyInfo: &types.IdentityInfo{CoreValidatorAddress: addr}, + LifecycleInfo: &types.LifecycleInfo{CurrentStatus: status}, + } +} + +func validatorSet(addrs ...string) []*types.UniversalValidator { + set := make([]*types.UniversalValidator, 0, len(addrs)) + for _, a := range addrs { + set = append(set, activeValidator(a)) + } + return set +} + +func addressesOf(vs []*types.UniversalValidator) []string { + addrs := make([]string, 0, len(vs)) + for _, v := range vs { + addrs = append(addrs, v.IdentifyInfo.CoreValidatorAddress) + } + return addrs +} + +func fundMigrateEvent(t *testing.T, oldKeyID string) store.Event { + t.Helper() + data, err := json.Marshal(utsstypes.FundMigrationInitiatedEventData{OldKeyID: oldKeyID}) + require.NoError(t, err) + return store.Event{ + EventID: "fm-1", + Type: store.EventTypeSignFundMigrate, + EventData: data, + } +} + +func coordinatorWithKeys(keys map[string]*utsstypes.TssKey) *Coordinator { + return &Coordinator{ + pushCore: &stalenessMockPushCore{keysByID: keys}, + logger: zerolog.Nop(), + } +} + +// The finding's scenario: the old key has three shareholders, the validator set +// has since grown to ten. Selecting from the current set draws newcomers who +// hold no share of that key. +func TestFundMigrateParticipants_DrawsOnlyFromOldKeyShareholders(t *testing.T) { + keys := map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3"}}, + } + c := coordinatorWithKeys(keys) + + all := validatorSet("v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9", "v10") + + // Selection is randomised, so repeat to catch a newcomer slipping in. + for i := 0; i < 200; i++ { + got, err := c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, "old-key"), all) + require.NoError(t, err) + + // Old key threshold is 3 of 3, not 7 of 10. + require.Len(t, got, 3) + assert.ElementsMatch(t, []string{"v1", "v2", "v3"}, addressesOf(got)) + } +} + +// A subset of shareholders large enough to sign, alongside a much larger +// current set. Every signer must still be a shareholder. +func TestFundMigrateParticipants_UsesOldKeyThreshold(t *testing.T) { + keys := map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3", "v4", "v5", "v6"}}, + } + c := coordinatorWithKeys(keys) + + all := validatorSet("v1", "v2", "v3", "v4", "v5", "v6", "n1", "n2", "n3", "n4", "n5") + + shareholders := map[string]bool{"v1": true, "v2": true, "v3": true, "v4": true, "v5": true, "v6": true} + for i := 0; i < 200; i++ { + got, err := c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, "old-key"), all) + require.NoError(t, err) + + // CalculateThreshold(6) is 5, and it is the old key's size that decides. + require.Len(t, got, CalculateThreshold(6)) + for _, addr := range addressesOf(got) { + assert.True(t, shareholders[addr], "selected %s which holds no share of the old key", addr) + } + } +} + +// Fail closed rather than hand back a set that cannot reach the old key's +// threshold. A short set would stall the session on an ACK that never arrives. +func TestFundMigrateParticipants_FailsWhenTooFewShareholdersRemain(t *testing.T) { + keys := map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3", "v4", "v5", "v6"}}, + } + c := coordinatorWithKeys(keys) + + // Only 4 of the 6 shareholders remain, one short of the threshold of 5, + // while the current set is comfortably large. + all := validatorSet("v1", "v2", "v3", "v4", "n1", "n2", "n3", "n4", "n5", "n6") + + got, err := c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, "old-key"), all) + require.Error(t, err) + assert.Nil(t, got) + assert.Contains(t, err.Error(), "only 4 are still eligible") +} + +// Pending leave keeps signing; anything else is not a usable signer even when +// it holds a share. +func TestFundMigrateParticipants_ExcludesIneligibleShareholders(t *testing.T) { + keys := map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3"}}, + } + c := coordinatorWithKeys(keys) + + all := []*types.UniversalValidator{ + validatorWithStatus("v1", types.UVStatus_UV_STATUS_ACTIVE), + validatorWithStatus("v2", types.UVStatus_UV_STATUS_PENDING_LEAVE), + validatorWithStatus("v3", types.UVStatus_UV_STATUS_ACTIVE), + } + + got, err := c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, "old-key"), all) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"v1", "v2", "v3"}, addressesOf(got)) + + // The same set with one shareholder no longer signing is one short. + all[1] = validatorWithStatus("v2", types.UVStatus_UV_STATUS_INACTIVE) + got, err = c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, "old-key"), all) + require.Error(t, err) + assert.Nil(t, got) +} + +func TestFundMigrateParticipants_RejectsUnusableEventData(t *testing.T) { + c := coordinatorWithKeys(map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3"}}, + }) + all := validatorSet("v1", "v2", "v3") + + t.Run("malformed event data", func(t *testing.T) { + event := store.Event{EventID: "fm-1", Type: store.EventTypeSignFundMigrate, EventData: []byte("not json")} + _, err := c.fundMigrateParticipants(context.Background(), event, all) + require.Error(t, err) + assert.Contains(t, err.Error(), "parse fund migration data") + }) + + t.Run("no old key id", func(t *testing.T) { + _, err := c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, ""), all) + require.Error(t, err) + assert.Contains(t, err.Error(), "no old key id") + }) + + t.Run("unknown old key", func(t *testing.T) { + _, err := c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, "missing-key"), all) + require.Error(t, err) + assert.Contains(t, err.Error(), "records no participants") + }) + + t.Run("key with empty participants", func(t *testing.T) { + c := coordinatorWithKeys(map[string]*utsstypes.TssKey{"old-key": {KeyId: "old-key"}}) + _, err := c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, "old-key"), all) + require.Error(t, err) + assert.Contains(t, err.Error(), "records no participants") + }) + + t.Run("lookup failure", func(t *testing.T) { + c := &Coordinator{ + pushCore: &stalenessMockPushCore{keyErr: fmt.Errorf("rpc down")}, + logger: zerolog.Nop(), + } + _, err := c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, "old-key"), all) + require.Error(t, err) + assert.Contains(t, err.Error(), "fetch old key") + }) +} + +// A shareholder that has since dropped its identity record must not be counted +// towards the threshold, since it cannot be addressed as a party. +func TestFundMigrateParticipants_SkipsValidatorWithoutIdentity(t *testing.T) { + c := coordinatorWithKeys(map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3"}}, + }) + + all := []*types.UniversalValidator{ + activeValidator("v1"), + {LifecycleInfo: &types.LifecycleInfo{CurrentStatus: types.UVStatus_UV_STATUS_ACTIVE}}, + activeValidator("v3"), + } + + _, err := c.fundMigrateParticipants(context.Background(), fundMigrateEvent(t, "old-key"), all) + require.Error(t, err) + assert.Contains(t, err.Error(), "only 2 are still eligible") +} + +// The routing itself: a fund migration must not be selected the way an +// outbound is, which is the defect this change fixes. +func TestSelectParticipants_RoutesFundMigrateToShareholders(t *testing.T) { + c := coordinatorWithKeys(map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3"}}, + }) + + all := validatorSet("v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9", "v10") + + t.Run("fund migrate is confined to the old key", func(t *testing.T) { + for i := 0; i < 100; i++ { + got, err := c.SelectParticipants(context.Background(), fundMigrateEvent(t, "old-key"), all) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"v1", "v2", "v3"}, addressesOf(got)) + } + }) + + t.Run("outbound still uses the current set", func(t *testing.T) { + event := store.Event{EventID: "ob-1", Type: store.EventTypeSignOutbound} + got, err := c.SelectParticipants(context.Background(), event, all) + require.NoError(t, err) + assert.Len(t, got, CalculateThreshold(len(all))) + }) + + t.Run("fund migrate reports rather than returning a short set", func(t *testing.T) { + _, err := c.SelectParticipants(context.Background(), fundMigrateEvent(t, "gone"), all) + require.Error(t, err) + }) + + t.Run("other protocols take every eligible validator", func(t *testing.T) { + event := store.Event{EventID: "kg-1", Type: store.EventTypeKeygen} + got, err := c.SelectParticipants(context.Background(), event, all) + require.NoError(t, err) + assert.Len(t, got, len(all)) + }) +} + +// The caller-supplied threshold is what keeps the count tied to the old key +// rather than to the surviving holders. +func TestSelectRandomThreshold_ExplicitCount(t *testing.T) { + all := validatorSet("v1", "v2", "v3", "v4", "v5") + + assert.Nil(t, selectRandomThreshold(nil, 3)) + assert.Nil(t, selectRandomThreshold(all, 0)) + assert.Nil(t, selectRandomThreshold(all, -1)) + assert.Len(t, selectRandomThreshold(all, 5), 5) + assert.Len(t, selectRandomThreshold(all, 9), 5) + + // Picks vary across calls and never repeat a validator within one pick. + seen := map[string]bool{} + for i := 0; i < 200; i++ { + got := selectRandomThreshold(all, 3) + require.Len(t, got, 3) + unique := map[string]bool{} + for _, addr := range addressesOf(got) { + assert.False(t, unique[addr], "duplicate %s in one selection", addr) + unique[addr] = true + seen[addr] = true + } + } + assert.Len(t, seen, 5, "selection never reached some validators") +} diff --git a/universalClient/tss/coordinator/utils.go b/universalClient/tss/coordinator/utils.go index b64371ba..562138af 100644 --- a/universalClient/tss/coordinator/utils.go +++ b/universalClient/tss/coordinator/utils.go @@ -65,28 +65,25 @@ func deriveKeyIDBytes(keyID string) []byte { return sum[:] } -// selectRandomThreshold selects a random subset of at least threshold count from eligible validators. -// Returns a shuffled copy of at least threshold validators (or all if fewer than threshold). -func selectRandomThreshold(eligible []*types.UniversalValidator) []*types.UniversalValidator { - if len(eligible) == 0 { +// selectRandomThreshold selects a random threshold count of eligible validators. +// Returns a shuffled copy of threshold validators (or all if fewer than threshold). +// The caller supplies the threshold: for fund migration it belongs to the old key, +// not to the set of validators still holding its shares. +func selectRandomThreshold(eligible []*types.UniversalValidator, threshold int) []*types.UniversalValidator { + if len(eligible) == 0 || threshold <= 0 { return nil } - // Calculate minimum required: >2/3 (same as threshold calculation) - minRequired := CalculateThreshold(len(eligible)) - - // If we have fewer than minRequired, return all - if len(eligible) <= minRequired { + // If we have fewer than threshold, return all + if len(eligible) <= threshold { return eligible } - // Randomly select at least minRequired participants - // Shuffle and take first minRequired shuffled := make([]*types.UniversalValidator, len(eligible)) copy(shuffled, eligible) rand.Shuffle(len(shuffled), func(i, j int) { shuffled[i], shuffled[j] = shuffled[j], shuffled[i] }) - return shuffled[:minRequired] + return shuffled[:threshold] } diff --git a/universalClient/tss/sessionmanager/fund_migrate_e2e_test.go b/universalClient/tss/sessionmanager/fund_migrate_e2e_test.go new file mode 100644 index 00000000..685295d7 --- /dev/null +++ b/universalClient/tss/sessionmanager/fund_migrate_e2e_test.go @@ -0,0 +1,263 @@ +package sessionmanager + +import ( + "context" + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pushchain/push-chain-node/universalClient/store" + "github.com/pushchain/push-chain-node/universalClient/tss/coordinator" + utsstypes "github.com/pushchain/push-chain-node/x/utss/types" + "github.com/pushchain/push-chain-node/x/uvalidator/types" +) + +func fundMigrateStoreEvent(t *testing.T, oldKeyID string) *store.Event { + t.Helper() + data, err := json.Marshal(utsstypes.FundMigrationInitiatedEventData{OldKeyID: oldKeyID}) + require.NoError(t, err) + return &store.Event{ + EventID: "fm-e2e", + Type: store.EventTypeSignFundMigrate, + EventData: data, + } +} + +func activeValidators(addrs ...string) []*types.UniversalValidator { + set := make([]*types.UniversalValidator, 0, len(addrs)) + for _, a := range addrs { + set = append(set, makeActiveValidator(a)) + } + return set +} + +func partyIDs(vs []*types.UniversalValidator) []string { + ids := make([]string, 0, len(vs)) + for _, v := range vs { + ids = append(ids, v.IdentifyInfo.CoreValidatorAddress) + } + return ids +} + +// End to end across both components: the coordinator selects the participants, +// then a participant validates the setup message it receives. +// +// The two sides derive the answer independently, so a change to one that the +// other does not mirror leaves a selection the coordinator can legitimately +// make and every participant rejects. Neither side's own tests catch that. +func TestFundMigrate_CoordinatorSelectionPassesParticipantValidation(t *testing.T) { + ctx := context.Background() + + // The finding's scenario: the old key has 3 shareholders and the validator + // set has since grown to 10. + oldKey := &utsstypes.TssKey{KeyId: "old-key", Participants: []string{"v1", "v2", "v3"}} + + sm, coord, _, _, _, _ := setupTestSessionManager(t) + setCoordinatorPushCore(coord, &mockPushCore{ + keysByID: map[string]*utsstypes.TssKey{"old-key": oldKey}, + }) + setCoordinatorValidators(coord, activeValidators( + "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9", "v10")) + + event := fundMigrateStoreEvent(t, "old-key") + + // Selection is randomised, so repeat rather than trusting one draw. + for i := 0; i < 100; i++ { + selected, err := coord.SelectParticipants(ctx, *event, coord.Validators()) + require.NoError(t, err, "coordinator could not select signers") + + ids := partyIDs(selected) + assert.ElementsMatch(t, []string{"v1", "v2", "v3"}, ids, + "coordinator selected a validator that holds no share of the old key") + + require.NoError(t, sm.validateParticipants(ctx, ids, event), + "participant rejected a selection the coordinator legitimately made") + } +} + +// The same round trip for an outbound, which must keep using the current +// validator set on both sides. +func TestSignOutbound_CoordinatorSelectionPassesParticipantValidation(t *testing.T) { + ctx := context.Background() + + sm, coord, _, _, _, _ := setupTestSessionManager(t) + all := activeValidators("v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9", "v10") + setCoordinatorValidators(coord, all) + + event := &store.Event{EventID: "ob-e2e", Type: store.EventTypeSignOutbound} + + for i := 0; i < 100; i++ { + selected, err := coord.SelectParticipants(ctx, *event, coord.Validators()) + require.NoError(t, err) + + ids := partyIDs(selected) + require.Len(t, ids, coordinator.CalculateThreshold(len(all))) + require.NoError(t, sm.validateParticipants(ctx, ids, event)) + } +} + +// Validation must be tied to the old key, not merely lenient. A set that meets +// the count but contains a validator holding no share is still rejected. +func TestFundMigrate_ValidationRejectsNonShareholders(t *testing.T) { + ctx := context.Background() + + sm, coord, _, _, _, _ := setupTestSessionManager(t) + setCoordinatorPushCore(coord, &mockPushCore{ + keysByID: map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3"}}, + }, + }) + setCoordinatorValidators(coord, activeValidators( + "v1", "v2", "v3", "v4", "v5", "v6", "v7", "v8", "v9", "v10")) + + event := fundMigrateStoreEvent(t, "old-key") + + t.Run("newcomer in an otherwise valid set", func(t *testing.T) { + err := sm.validateParticipants(ctx, []string{"v1", "v2", "v10"}, event) + require.Error(t, err) + assert.Contains(t, err.Error(), "v10") + }) + + t.Run("all newcomers, count satisfied", func(t *testing.T) { + err := sm.validateParticipants(ctx, []string{"v8", "v9", "v10"}, event) + require.Error(t, err) + }) + + t.Run("below the old key threshold", func(t *testing.T) { + err := sm.validateParticipants(ctx, []string{"v1", "v2"}, event) + require.Error(t, err) + assert.Contains(t, err.Error(), "below required threshold 3") + }) + + t.Run("exactly the shareholders is accepted", func(t *testing.T) { + require.NoError(t, sm.validateParticipants(ctx, []string{"v1", "v2", "v3"}, event)) + }) +} + +// A larger old key, so the accepted count is a strict subset of shareholders +// rather than all of them, and the current set is not what sizes it. +func TestFundMigrate_ValidationUsesOldKeyThresholdNotCurrentSet(t *testing.T) { + ctx := context.Background() + + sm, coord, _, _, _, _ := setupTestSessionManager(t) + setCoordinatorPushCore(coord, &mockPushCore{ + keysByID: map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3", "v4", "v5", "v6"}}, + }, + }) + // 6 shareholders among 12 validators. Old key threshold is 5, the current + // set's would be 9, which no set of shareholders could ever satisfy. + setCoordinatorValidators(coord, activeValidators( + "v1", "v2", "v3", "v4", "v5", "v6", "n1", "n2", "n3", "n4", "n5", "n6")) + + event := fundMigrateStoreEvent(t, "old-key") + + require.Equal(t, 5, coordinator.CalculateThreshold(6)) + require.Equal(t, 9, coordinator.CalculateThreshold(12)) + + t.Run("old key threshold is accepted", func(t *testing.T) { + require.NoError(t, sm.validateParticipants(ctx, []string{"v1", "v2", "v3", "v4", "v5"}, event)) + }) + + t.Run("all shareholders is accepted", func(t *testing.T) { + require.NoError(t, sm.validateParticipants(ctx, []string{"v1", "v2", "v3", "v4", "v5", "v6"}, event)) + }) + + t.Run("one below the old key threshold is rejected", func(t *testing.T) { + err := sm.validateParticipants(ctx, []string{"v1", "v2", "v3", "v4"}, event) + require.Error(t, err) + assert.Contains(t, err.Error(), "below required threshold 5") + }) +} + +// Some shareholders are gone but enough remain to sign. The threshold must +// still be the old key's, not one derived from the survivors: deriving it from +// the survivors lowers the bar every time a shareholder drops out, so a +// coordinator could open a session below the quorum the key was created under. +func TestFundMigrate_ValidationThresholdDoesNotShrinkWithSurvivors(t *testing.T) { + ctx := context.Background() + + sm, coord, _, _, _, _ := setupTestSessionManager(t) + setCoordinatorPushCore(coord, &mockPushCore{ + keysByID: map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3", "v4", "v5", "v6"}}, + }, + }) + // v6 is gone, so 5 of the 6 shareholders survive. The old key still requires + // 5, while a threshold over the survivors would be only 4. + setCoordinatorValidators(coord, activeValidators("v1", "v2", "v3", "v4", "v5", "n1", "n2", "n3")) + + event := fundMigrateStoreEvent(t, "old-key") + + require.Equal(t, 5, coordinator.CalculateThreshold(6), "old key threshold") + require.Equal(t, 4, coordinator.CalculateThreshold(5), "threshold over survivors") + + t.Run("four survivors is below the old key threshold", func(t *testing.T) { + err := sm.validateParticipants(ctx, []string{"v1", "v2", "v3", "v4"}, event) + require.Error(t, err) + assert.Contains(t, err.Error(), "below required threshold 5") + }) + + t.Run("all five survivors is accepted", func(t *testing.T) { + require.NoError(t, sm.validateParticipants(ctx, []string{"v1", "v2", "v3", "v4", "v5"}, event)) + }) + + t.Run("the coordinator selects exactly those five", func(t *testing.T) { + selected, err := coord.SelectParticipants(ctx, *event, coord.Validators()) + require.NoError(t, err) + ids := partyIDs(selected) + assert.ElementsMatch(t, []string{"v1", "v2", "v3", "v4", "v5"}, ids) + require.NoError(t, sm.validateParticipants(ctx, ids, event)) + }) +} + +// Too few shareholders left to sign at all. Both sides must refuse, and the +// coordinator must not dispatch a set it knows cannot reach quorum. +func TestFundMigrate_BothSidesFailClosedWhenShareholdersGone(t *testing.T) { + ctx := context.Background() + + sm, coord, _, _, _, _ := setupTestSessionManager(t) + setCoordinatorPushCore(coord, &mockPushCore{ + keysByID: map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3", "v4", "v5", "v6"}}, + }, + }) + // Only 4 of the 6 shareholders remain, one short of the threshold of 5. + setCoordinatorValidators(coord, activeValidators("v1", "v2", "v3", "v4", "n1", "n2", "n3", "n4")) + + event := fundMigrateStoreEvent(t, "old-key") + + _, err := coord.SelectParticipants(ctx, *event, coord.Validators()) + require.Error(t, err, "coordinator dispatched a set that cannot reach quorum") + + err = sm.validateParticipants(ctx, []string{"v1", "v2", "v3", "v4"}, event) + require.Error(t, err) +} + +// Validation must not fall open when the old key cannot be resolved. +func TestFundMigrate_ValidationFailsClosedOnUnresolvableKey(t *testing.T) { + ctx := context.Background() + + sm, coord, _, _, _, _ := setupTestSessionManager(t) + setCoordinatorValidators(coord, activeValidators("v1", "v2", "v3", "v4", "v5")) + + // The default mock returns a key with no participants for any id. + setCoordinatorPushCore(coord, &mockPushCore{}) + + err := sm.validateParticipants(ctx, []string{"v1", "v2", "v3", "v4"}, fundMigrateStoreEvent(t, "old-key")) + require.Error(t, err) + assert.Contains(t, err.Error(), "resolve fund migration signers") + + t.Run("malformed event data", func(t *testing.T) { + event := &store.Event{ + EventID: "fm-bad", + Type: store.EventTypeSignFundMigrate, + EventData: []byte("not json"), + } + err := sm.validateParticipants(ctx, []string{"v1", "v2", "v3", "v4"}, event) + require.Error(t, err) + assert.Contains(t, err.Error(), "resolve fund migration signers") + }) +} diff --git a/universalClient/tss/sessionmanager/sessionmanager.go b/universalClient/tss/sessionmanager/sessionmanager.go index 47db05ec..b6bc53d6 100644 --- a/universalClient/tss/sessionmanager/sessionmanager.go +++ b/universalClient/tss/sessionmanager/sessionmanager.go @@ -25,6 +25,7 @@ import ( "github.com/pushchain/push-chain-node/universalClient/tss/keyshare" uexecutortypes "github.com/pushchain/push-chain-node/x/uexecutor/types" utsstypes "github.com/pushchain/push-chain-node/x/utss/types" + uvalidatortypes "github.com/pushchain/push-chain-node/x/uvalidator/types" ) // SendFunc is a function type for sending messages to participants. @@ -179,7 +180,7 @@ func (sm *SessionManager) handleSetupMessage(ctx context.Context, senderPeerID s } // 5. Validate participants list matches event protocol requirements - if err := sm.validateParticipants(msg.Participants, event); err != nil { + if err := sm.validateParticipants(ctx, msg.Participants, event); err != nil { return fmt.Errorf("participants validation failed: %w", err) } @@ -752,9 +753,24 @@ func (sm *SessionManager) createSession(ctx context.Context, event *store.Event, // validateParticipants validates that participants match protocol requirements. // For keygen/keyrefresh: participants must match exactly with eligible participants (same elements). // For sign: participants must be a valid >2/3 subset of eligible participants. -func (sm *SessionManager) validateParticipants(participants []string, event *store.Event) error { - // Get eligible validators for this protocol - eligible := sm.coordinator.GetEligibleUV(string(event.Type)) +func (sm *SessionManager) validateParticipants(ctx context.Context, participants []string, event *store.Event) error { + // Get eligible validators for this protocol. + // + // Fund migration is signed with the old key's shares, so both who may take + // part and how many are required come from that key rather than from the + // current validator set. Resolved through the coordinator so the check here + // mirrors the selection exactly. + var eligible []*uvalidatortypes.UniversalValidator + var fundMigrateRequired int + if event.Type == store.EventTypeSignFundMigrate { + var err error + eligible, fundMigrateRequired, err = sm.coordinator.FundMigrateEligible(ctx, *event) + if err != nil { + return fmt.Errorf("resolve fund migration signers: %w", err) + } + } else { + eligible = sm.coordinator.GetEligibleUV(string(event.Type)) + } if len(eligible) == 0 { return fmt.Errorf("no eligible validators for protocol") } @@ -793,8 +809,8 @@ func (sm *SessionManager) validateParticipants(participants []string, event *sto } } - case store.EventTypeSignOutbound, store.EventTypeSignFundMigrate: - // For SIGN and FUND_MIGRATE the coordinator picks a random threshold subset (>2/3 of eligible) + case store.EventTypeSignOutbound: + // For SIGN the coordinator picks a random threshold subset (>2/3 of eligible) // rather than all eligible validators. Accept any subset as long as it meets the threshold // minimum; all participants are already verified eligible by the eligibleSet check above. threshold := coordinator.CalculateThreshold(len(eligibleList)) @@ -803,6 +819,15 @@ func (sm *SessionManager) validateParticipants(participants []string, event *sto event.Type, len(participants), threshold, len(eligibleList)) } + case store.EventTypeSignFundMigrate: + // The old key's threshold, not the current set's. Sizing this from the + // live validator set would reject a legitimate selection whenever the + // set has grown since that key was created. + if len(participants) < fundMigrateRequired { + return fmt.Errorf("%s participants count %d is below required threshold %d (shareholders still eligible: %d)", + event.Type, len(participants), fundMigrateRequired, len(eligibleList)) + } + default: return fmt.Errorf("unknown protocol type: %s", event.Type) } diff --git a/universalClient/tss/sessionmanager/sessionmanager_test.go b/universalClient/tss/sessionmanager/sessionmanager_test.go index 73f0dd2a..cd1ae782 100644 --- a/universalClient/tss/sessionmanager/sessionmanager_test.go +++ b/universalClient/tss/sessionmanager/sessionmanager_test.go @@ -53,6 +53,9 @@ func containsAny(s string, substrings []string) bool { // block height (0 by default) so coordinator-at-block math is deterministic. type mockPushCore struct { block uint64 + + // Old key history, consulted when validating fund migration signers. + keysByID map[string]*utsstypes.TssKey } func (m *mockPushCore) GetLatestBlock(_ context.Context) (uint64, error) { @@ -63,6 +66,13 @@ func (m *mockPushCore) GetCurrentKey(_ context.Context) (*utsstypes.TssKey, erro return &utsstypes.TssKey{KeyId: "test-key"}, nil } +func (m *mockPushCore) GetKeyByID(_ context.Context, keyID string) (*utsstypes.TssKey, error) { + if key, ok := m.keysByID[keyID]; ok { + return key, nil + } + return &utsstypes.TssKey{KeyId: keyID}, nil +} + func (m *mockPushCore) GetAllUniversalValidators(_ context.Context) ([]*types.UniversalValidator, error) { return nil, nil } @@ -367,6 +377,11 @@ func setCoordinatorValidators(coord *coordinator.Coordinator, validators []*type if field.IsValid() { *(*[]*types.UniversalValidator)(unsafe.Pointer(field.UnsafeAddr())) = validators } + // Keep the cache fresh, otherwise a slow test trips the staleness halt and + // the snapshot comes back empty. + if refresh := coordValue.FieldByName("lastValidatorsRefreshAt"); refresh.IsValid() { + *(*time.Time)(unsafe.Pointer(refresh.UnsafeAddr())) = time.Now() + } } func makeActiveValidator(addr string) *types.UniversalValidator { @@ -398,54 +413,69 @@ func TestValidateParticipants(t *testing.T) { t.Run("SIGN: threshold subset is valid", func(t *testing.T) { // 3 of 4 eligible satisfies threshold(4)=3 - assert.NoError(t, sm.validateParticipants([]string{"v1", "v2", "v3"}, signEvent)) + assert.NoError(t, sm.validateParticipants(context.Background(), []string{"v1", "v2", "v3"}, signEvent)) }) t.Run("SIGN: all eligible is also valid (threshold is a minimum)", func(t *testing.T) { - assert.NoError(t, sm.validateParticipants([]string{"v1", "v2", "v3", "v4"}, signEvent)) + assert.NoError(t, sm.validateParticipants(context.Background(), []string{"v1", "v2", "v3", "v4"}, signEvent)) }) t.Run("SIGN: below threshold is rejected", func(t *testing.T) { // 2 < threshold(4)=3 - err := sm.validateParticipants([]string{"v1", "v2"}, signEvent) + err := sm.validateParticipants(context.Background(), []string{"v1", "v2"}, signEvent) require.Error(t, err) assert.Contains(t, err.Error(), "threshold") }) t.Run("SIGN: non-eligible participant is rejected", func(t *testing.T) { - err := sm.validateParticipants([]string{"v1", "v2", "unknown"}, signEvent) + err := sm.validateParticipants(context.Background(), []string{"v1", "v2", "unknown"}, signEvent) require.Error(t, err) assert.Contains(t, err.Error(), "not eligible") }) - // --- SIGN_FUND_MIGRATE: same threshold rules as SIGN_OUTBOUND --- + // --- SIGN_FUND_MIGRATE: rules come from the old key, not the current set --- - fmEvent := &store.Event{EventID: "fm-1", Type: store.EventTypeSignFundMigrate} + // The old key's shareholders are v1..v4, matching the current set here, so + // the threshold is the same 3 as for SIGN_OUTBOUND above. The two diverge + // once the sets differ, covered in fund_migrate_e2e_test.go. + setCoordinatorPushCore(coord, &mockPushCore{ + keysByID: map[string]*utsstypes.TssKey{ + "old-key": {KeyId: "old-key", Participants: []string{"v1", "v2", "v3", "v4"}}, + }, + }) + fmEvent := fundMigrateStoreEvent(t, "old-key") t.Run("SIGN_FUND_MIGRATE: threshold subset is valid", func(t *testing.T) { - assert.NoError(t, sm.validateParticipants([]string{"v1", "v2", "v3"}, fmEvent)) + assert.NoError(t, sm.validateParticipants(context.Background(), []string{"v1", "v2", "v3"}, fmEvent)) }) t.Run("SIGN_FUND_MIGRATE: below threshold is rejected", func(t *testing.T) { - err := sm.validateParticipants([]string{"v1", "v2"}, fmEvent) + err := sm.validateParticipants(context.Background(), []string{"v1", "v2"}, fmEvent) require.Error(t, err) assert.Contains(t, err.Error(), "threshold") }) + t.Run("SIGN_FUND_MIGRATE: event without an old key id is rejected", func(t *testing.T) { + bare := &store.Event{EventID: "fm-bare", Type: store.EventTypeSignFundMigrate} + err := sm.validateParticipants(context.Background(), []string{"v1", "v2", "v3"}, bare) + require.Error(t, err) + assert.Contains(t, err.Error(), "resolve fund migration signers") + }) + // --- KEYGEN: exact-match rules (all eligible must participate) --- t.Run("KEYGEN: all eligible is valid", func(t *testing.T) { - assert.NoError(t, sm.validateParticipants([]string{"v1", "v2", "v3", "v4"}, keygenEvent)) + assert.NoError(t, sm.validateParticipants(context.Background(), []string{"v1", "v2", "v3", "v4"}, keygenEvent)) }) t.Run("KEYGEN: missing participant is rejected", func(t *testing.T) { - err := sm.validateParticipants([]string{"v1", "v2", "v3"}, keygenEvent) // v4 missing + err := sm.validateParticipants(context.Background(), []string{"v1", "v2", "v3"}, keygenEvent) // v4 missing require.Error(t, err) assert.Contains(t, err.Error(), "does not match eligible count") }) t.Run("KEYGEN: non-eligible participant is rejected", func(t *testing.T) { - err := sm.validateParticipants([]string{"v1", "v2", "v3", "v4", "unknown"}, keygenEvent) + err := sm.validateParticipants(context.Background(), []string{"v1", "v2", "v3", "v4", "unknown"}, keygenEvent) require.Error(t, err) assert.Contains(t, err.Error(), "not eligible") })