Skip to content

Commit

Permalink
Merge pull request #423 from lavanet/CNS-386-prevent-jail-on-insuffic…
Browse files Browse the repository at this point in the history
…ient-providers-count

CNS-386-do not jail if not enough providers exist
  • Loading branch information
Yaroms authored Apr 16, 2023
2 parents b6a1c02 + d77ca8a commit 7e084e8
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 46 deletions.
15 changes: 14 additions & 1 deletion x/pairing/keeper/unresponsive_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,21 @@ func (k Keeper) UnstakeUnresponsiveProviders(ctx sdk.Context, epochsNumToCheckCU
return nil
}

// TODO: when we use the policy providers number, this should be updated
minimumProvidersCount, err := k.ServicersToPairCount(ctx, uint64(ctx.BlockHeight()))
if err != nil {
return utils.LavaError(ctx, k.Logger(ctx), "param_reading", map[string]string{"err": err.Error()}, "couldn't read k.ServicersToPairCount(ctx, uint64(ctx.BlockHeight()))")
}

// Go over the staked provider entries (on all chains)
for _, providerStakeStorage := range providerStakeStorageList {
for _, providerStakeEntry := range providerStakeStorage.GetStakeEntries() {
providerStakeEntriesForChain := providerStakeStorage.GetStakeEntries()
existingProviders := uint64(len(providerStakeEntriesForChain))
for _, providerStakeEntry := range providerStakeEntriesForChain {
if existingProviders <= minimumProvidersCount {
// not enough providers, skip jailing any more providers
break
}
if minHistoryBlock < providerStakeEntry.StakeAppliedBlock {
// this staked provider has too short history (either since staking
// or since it was last unfrozen) - do not consider for jailing
Expand All @@ -69,6 +81,7 @@ func (k Keeper) UnstakeUnresponsiveProviders(ctx sdk.Context, epochsNumToCheckCU
// providerPaymentStorageKeyList is not empty -> provider should be punished
if len(providerPaymentStorageKeyList) != 0 {
err = k.punishUnresponsiveProvider(ctx, minPaymentBlock, providerPaymentStorageKeyList, providerStakeEntry.GetAddress(), providerStakeEntry.GetChain())
existingProviders--
if err != nil {
return utils.LavaError(ctx, k.Logger(ctx), "punish_unresponsive_provider", map[string]string{"err": err.Error()}, "couldn't punish unresponsive provider")
}
Expand Down
170 changes: 125 additions & 45 deletions x/pairing/keeper/unresponsive_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ func TestUnresponsivenessStressTest(t *testing.T) {
// Test to measure the time the check for unresponsiveness every epoch start takes
func TestUnstakingProviderForUnresponsiveness(t *testing.T) {
// setup test for unresponsiveness
testClientAmount := 4
testProviderAmount := 2
testClientAmount := 1
testProviderAmount := 10
ts := setupClientsAndProvidersForUnresponsiveness(t, testClientAmount, testProviderAmount)

// get recommendedEpochNumToCollectPayment
Expand All @@ -130,20 +130,26 @@ func TestUnstakingProviderForUnresponsiveness(t *testing.T) {
ts.ctx = testkeeper.AdvanceEpoch(ts.ctx, ts.keepers)
}

// find two providers in the pairing
pairingProviders, err := ts.keepers.Pairing.GetPairingForClient(sdk.UnwrapSDKContext(ts.ctx), ts.spec.Name, ts.clients[0].Addr)
require.NoError(t, err)
provider0_addr := sdk.MustAccAddressFromBech32(pairingProviders[0].Address)
provider1_addr := sdk.MustAccAddressFromBech32(pairingProviders[1].Address)

// get provider1's balance before the stake
staked_amount, _, _ := ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, ts.providers[1].Addr)
balanceProvideratBeforeStake := staked_amount.Stake.Amount.Int64() + ts.keepers.BankKeeper.GetBalance(sdk.UnwrapSDKContext(ts.ctx), ts.providers[1].Addr, epochstoragetypes.TokenDenom).Amount.Int64()
staked_amount, _, _ := ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, provider1_addr)
balanceProvideratBeforeStake := staked_amount.Stake.Amount.Int64() + ts.keepers.BankKeeper.GetBalance(sdk.UnwrapSDKContext(ts.ctx), provider1_addr, epochstoragetypes.TokenDenom).Amount.Int64()

// create unresponsive data that includes provider1 being unresponsive
unresponsiveProvidersData, err := json.Marshal([]string{ts.providers[1].Addr.String()})
unresponsiveProvidersData, err := json.Marshal([]string{provider1_addr.String()})
require.Nil(t, err)

// create relay requests for provider0 that contain complaints about provider1
relayEpoch := sdk.UnwrapSDKContext(ts.ctx).BlockHeight()
for clientIndex := 0; clientIndex < testClientAmount; clientIndex++ { // testing testClientAmount of complaints
var Relays []*types.RelaySession
relayRequest := &types.RelaySession{
Provider: ts.providers[0].Addr.String(),
Provider: provider0_addr.String(),
ContentHash: []byte(ts.spec.Apis[0].Name),
SessionId: uint64(0),
SpecId: ts.spec.Name,
Expand All @@ -159,7 +165,7 @@ func TestUnstakingProviderForUnresponsiveness(t *testing.T) {
Relays = append(Relays, relayRequest)

// send relay payment and check the funds did transfer normally
payAndVerifyBalance(t, ts, types.MsgRelayPayment{Creator: ts.providers[0].Addr.String(), Relays: Relays}, true, ts.clients[clientIndex].Addr, ts.providers[0].Addr)
payAndVerifyBalance(t, ts, types.MsgRelayPayment{Creator: provider0_addr.String(), Relays: Relays}, true, ts.clients[clientIndex].Addr, provider0_addr)
}

// advance enough epochs so the unresponsive provider will be punished
Expand All @@ -171,21 +177,21 @@ func TestUnstakingProviderForUnresponsiveness(t *testing.T) {
}

// test the unresponsive provider1 has been unstaked
_, unstakeStoragefound, _ := ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.providers[1].Addr)
_, unstakeStoragefound, _ := ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, provider1_addr)
require.True(t, unstakeStoragefound)
_, stakeStorageFound, _ := ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, ts.providers[1].Addr)
_, stakeStorageFound, _ := ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, provider1_addr)
require.False(t, stakeStorageFound)

// validate the complainers CU field in provider1's providerPaymentStorage has been reset after being punished (note we use the epoch from the relay because that is when it got reported)
providerPaymentStorageKey := ts.keepers.Pairing.GetProviderPaymentStorageKey(sdk.UnwrapSDKContext(ts.ctx), ts.spec.Name, uint64(relayEpoch), ts.providers[1].Addr)
providerPaymentStorageKey := ts.keepers.Pairing.GetProviderPaymentStorageKey(sdk.UnwrapSDKContext(ts.ctx), ts.spec.Name, uint64(relayEpoch), provider1_addr)
providerPaymentStorage, found := ts.keepers.Pairing.GetProviderPaymentStorage(sdk.UnwrapSDKContext(ts.ctx), providerPaymentStorageKey)
require.Equal(t, true, found)
require.Equal(t, uint64(0), providerPaymentStorage.GetComplainersTotalCu())

// test the responsive provider0 hasn't been unstaked
_, unstakeStoragefound, _ = ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.providers[0].Addr)
_, unstakeStoragefound, _ = ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, provider0_addr)
require.False(t, unstakeStoragefound)
_, stakeStorageFound, _ = ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, ts.providers[0].Addr)
_, stakeStorageFound, _ = ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, provider0_addr)
require.True(t, stakeStorageFound)

// advance enough epochs so the current block will be deleted (advance more than the chain's memory - blocksToSave)
Expand All @@ -201,22 +207,22 @@ func TestUnstakingProviderForUnresponsiveness(t *testing.T) {
}

// validate that the provider is no longer unstaked
_, unstakeStoragefound, _ = ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.providers[1].Addr)
_, unstakeStoragefound, _ = ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, provider1_addr)
require.False(t, unstakeStoragefound)

// also validate that the provider hasn't returned to the stake pool
_, stakeStorageFound, _ = ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, ts.providers[1].Addr)
_, stakeStorageFound, _ = ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, provider1_addr)
require.False(t, stakeStorageFound)

// validate that the provider's balance after the unstake is the same as before he staked
balanceProviderAfterUnstakeMoneyReturned := ts.keepers.BankKeeper.GetBalance(sdk.UnwrapSDKContext(ts.ctx), ts.providers[1].Addr, epochstoragetypes.TokenDenom).Amount.Int64()
balanceProviderAfterUnstakeMoneyReturned := ts.keepers.BankKeeper.GetBalance(sdk.UnwrapSDKContext(ts.ctx), provider1_addr, epochstoragetypes.TokenDenom).Amount.Int64()
require.Equal(t, balanceProvideratBeforeStake, balanceProviderAfterUnstakeMoneyReturned)
}

func TestUnstakingProviderForUnresponsivenessContinueComplainingAfterUnstake(t *testing.T) {
// setup test for unresponsiveness
testClientAmount := 4
testProviderAmount := 2
testClientAmount := 1
testProviderAmount := 3
ts := setupClientsAndProvidersForUnresponsiveness(t, testClientAmount, testProviderAmount)

// get recommendedEpochNumToCollectPayment
Expand All @@ -233,34 +239,38 @@ func TestUnstakingProviderForUnresponsivenessContinueComplainingAfterUnstake(t *
ts.ctx = testkeeper.AdvanceEpoch(ts.ctx, ts.keepers)
}

// find two providers in the pairing
pairingProviders, err := ts.keepers.Pairing.GetPairingForClient(sdk.UnwrapSDKContext(ts.ctx), ts.spec.Name, ts.clients[0].Addr)
require.NoError(t, err)
provider0_addr := sdk.MustAccAddressFromBech32(pairingProviders[0].Address)
provider1_addr := sdk.MustAccAddressFromBech32(pairingProviders[1].Address)
// create unresponsive data that includes provider1 being unresponsive
unresponsiveProvidersData, err := json.Marshal([]string{ts.providers[1].Addr.String()})
unresponsiveProvidersData, err := json.Marshal([]string{provider1_addr.String()})
require.Nil(t, err)

// create relay requests for provider0 that contain complaints about provider1
relayEpoch := sdk.UnwrapSDKContext(ts.ctx).BlockHeight()
for clientIndex := 0; clientIndex < testClientAmount; clientIndex++ { // testing testClientAmount of complaints
var Relays []*types.RelaySession
relayRequest := &types.RelaySession{
Provider: ts.providers[0].Addr.String(),
ContentHash: []byte(ts.spec.Apis[0].Name),
SessionId: uint64(0),
SpecId: ts.spec.Name,
CuSum: ts.spec.Apis[0].ComputeUnits * 10,
Epoch: relayEpoch,
RelayNum: 0,
UnresponsiveProviders: unresponsiveProvidersData, // create the complaint
}

sig, err := sigs.SignRelay(ts.clients[clientIndex].SK, *relayRequest)
relayRequest.Sig = sig
require.Nil(t, err)
Relays = append(Relays, relayRequest)

// send relay payment and check the funds did transfer normally
payAndVerifyBalance(t, ts, types.MsgRelayPayment{Creator: ts.providers[0].Addr.String(), Relays: Relays}, true, ts.clients[clientIndex].Addr, ts.providers[0].Addr)
var Relays []*types.RelaySession
relayRequest := &types.RelaySession{
Provider: provider0_addr.String(),
ContentHash: []byte(ts.spec.Apis[0].Name),
SessionId: uint64(0),
SpecId: ts.spec.Name,
CuSum: ts.spec.Apis[0].ComputeUnits * 10,
Epoch: relayEpoch,
RelayNum: 0,
UnresponsiveProviders: unresponsiveProvidersData, // create the complaint
}

sig, err := sigs.SignRelay(ts.clients[0].SK, *relayRequest)
relayRequest.Sig = sig
require.Nil(t, err)
Relays = append(Relays, relayRequest)

// send relay payment and check the funds did transfer normally
payAndVerifyBalance(t, ts, types.MsgRelayPayment{Creator: provider0_addr.String(), Relays: Relays}, true, ts.clients[0].Addr, provider0_addr)

// advance enough epochs so the unresponsive provider will be punished
if largerConst < recommendedEpochNumToCollectPayment {
largerConst = recommendedEpochNumToCollectPayment
Expand All @@ -270,13 +280,13 @@ func TestUnstakingProviderForUnresponsivenessContinueComplainingAfterUnstake(t *
}

// test the provider has been unstaked
_, unStakeStoragefound, _ := ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.providers[1].Addr)
_, unStakeStoragefound, _ := ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, provider1_addr)
require.True(t, unStakeStoragefound)
_, stakeStorageFound, _ := ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, ts.providers[1].Addr)
_, stakeStorageFound, _ := ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, provider1_addr)
require.False(t, stakeStorageFound)

// validate the complainers CU field in provider1's providerPaymentStorage has been reset after being punished (note we use the epoch from the relay because that is when it got reported)
providerPaymentStorageKey := ts.keepers.Pairing.GetProviderPaymentStorageKey(sdk.UnwrapSDKContext(ts.ctx), ts.spec.Name, uint64(relayEpoch), ts.providers[1].Addr)
providerPaymentStorageKey := ts.keepers.Pairing.GetProviderPaymentStorageKey(sdk.UnwrapSDKContext(ts.ctx), ts.spec.Name, uint64(relayEpoch), provider1_addr)
providerPaymentStorage, found := ts.keepers.Pairing.GetProviderPaymentStorage(sdk.UnwrapSDKContext(ts.ctx), providerPaymentStorageKey)
require.Equal(t, true, found)
require.Equal(t, uint64(0), providerPaymentStorage.GetComplainersTotalCu())
Expand All @@ -290,7 +300,7 @@ func TestUnstakingProviderForUnresponsivenessContinueComplainingAfterUnstake(t *
for clientIndex := 0; clientIndex < testClientAmount; clientIndex++ { // testing testClientAmount of complaints
var RelaysAfter []*types.RelaySession
relayRequest := &types.RelaySession{
Provider: ts.providers[0].Addr.String(),
Provider: provider0_addr.String(),
ContentHash: []byte(ts.spec.Apis[0].Name),
SessionId: uint64(2),
SpecId: ts.spec.Name,
Expand All @@ -305,13 +315,13 @@ func TestUnstakingProviderForUnresponsivenessContinueComplainingAfterUnstake(t *
RelaysAfter = append(RelaysAfter, relayRequest)

// send relay payment and check the funds did transfer normally
payAndVerifyBalance(t, ts, types.MsgRelayPayment{Creator: ts.providers[0].Addr.String(), Relays: RelaysAfter}, true, ts.clients[clientIndex].Addr, ts.providers[0].Addr)
payAndVerifyBalance(t, ts, types.MsgRelayPayment{Creator: provider0_addr.String(), Relays: RelaysAfter}, true, ts.clients[clientIndex].Addr, provider0_addr)
}

// test the provider is still unstaked
_, stakeStorageFound, _ = ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, ts.providers[1].Addr)
_, stakeStorageFound, _ = ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, provider1_addr)
require.False(t, stakeStorageFound)
_, unStakeStoragefound, _ = ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.providers[1].Addr)
_, unStakeStoragefound, _ = ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, provider1_addr)
require.True(t, unStakeStoragefound)

// get the current unstake storage
Expand All @@ -321,9 +331,79 @@ func TestUnstakingProviderForUnresponsivenessContinueComplainingAfterUnstake(t *
// validate the punished provider is not shown twice (or more) in the unstake storage
var numberOfAppearances int
for _, stored := range storage.StakeEntries {
if stored.Address == ts.providers[1].Addr.String() {
if stored.Address == provider1_addr.String() {
numberOfAppearances += 1
}
}
require.Equal(t, numberOfAppearances, 1)
}

func TestNotUnstakingProviderForUnresponsivenessWithMinProviders(t *testing.T) {
// setup test for unresponsiveness
testClientAmount := 1
testProviderAmount := 2
ts := setupClientsAndProvidersForUnresponsiveness(t, testClientAmount, testProviderAmount)

// get recommendedEpochNumToCollectPayment
recommendedEpochNumToCollectPayment := ts.keepers.Pairing.RecommendedEpochNumToCollectPayment(sdk.UnwrapSDKContext(ts.ctx))

// check which const is larger
largerConst := pairing.EPOCHS_NUM_TO_CHECK_CU_FOR_UNRESPONSIVE_PROVIDER
if largerConst < pairing.EPOCHS_NUM_TO_CHECK_FOR_COMPLAINERS {
largerConst = pairing.EPOCHS_NUM_TO_CHECK_FOR_COMPLAINERS
}

// advance enough epochs so we can check punishment due to unresponsiveness (if the epoch is too early, there's no punishment)
for i := uint64(0); i < uint64(largerConst)+recommendedEpochNumToCollectPayment; i++ {
ts.ctx = testkeeper.AdvanceEpoch(ts.ctx, ts.keepers)
}

// find two providers in the pairing
pairingProviders, err := ts.keepers.Pairing.GetPairingForClient(sdk.UnwrapSDKContext(ts.ctx), ts.spec.Name, ts.clients[0].Addr)
require.NoError(t, err)
provider0_addr := sdk.MustAccAddressFromBech32(pairingProviders[0].Address)
provider1_addr := sdk.MustAccAddressFromBech32(pairingProviders[1].Address)

// create unresponsive data that includes provider1 being unresponsive
unresponsiveProvidersData, err := json.Marshal([]string{provider1_addr.String()})
require.Nil(t, err)

// create relay requests for provider0 that contain complaints about provider1
relayEpoch := sdk.UnwrapSDKContext(ts.ctx).BlockHeight()
for clientIndex := 0; clientIndex < testClientAmount; clientIndex++ { // testing testClientAmount of complaints
var Relays []*types.RelaySession
relayRequest := &types.RelaySession{
Provider: provider0_addr.String(),
ContentHash: []byte(ts.spec.Apis[0].Name),
SessionId: uint64(0),
SpecId: ts.spec.Name,
CuSum: ts.spec.Apis[0].ComputeUnits*10 + uint64(clientIndex),
Epoch: relayEpoch,
RelayNum: 0,
UnresponsiveProviders: unresponsiveProvidersData, // create the complaint
}

sig, err := sigs.SignRelay(ts.clients[clientIndex].SK, *relayRequest)
relayRequest.Sig = sig
require.Nil(t, err)
Relays = append(Relays, relayRequest)

// send relay payment and check the funds did transfer normally
payAndVerifyBalance(t, ts, types.MsgRelayPayment{Creator: provider0_addr.String(), Relays: Relays}, true, ts.clients[clientIndex].Addr, provider0_addr)
}

// advance enough epochs so the unresponsive provider will be punished
if largerConst < recommendedEpochNumToCollectPayment {
largerConst = recommendedEpochNumToCollectPayment
}
for i := uint64(0); i < largerConst; i++ {
ts.ctx = testkeeper.AdvanceEpoch(ts.ctx, ts.keepers)
}

// test the unresponsive provider1 has been unstaked
_, unstakeStoragefound, _ := ts.keepers.Epochstorage.UnstakeEntryByAddress(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, provider1_addr)
require.False(t, unstakeStoragefound)
_, stakeStorageFound, _ := ts.keepers.Epochstorage.GetStakeEntryByAddressCurrent(sdk.UnwrapSDKContext(ts.ctx), epochstoragetypes.ProviderKey, ts.spec.Name, provider1_addr)
require.True(t, stakeStorageFound)

}

0 comments on commit 7e084e8

Please sign in to comment.