Merge pull request #188 from lavanet/CNS-29-few-more-changes

CNS-29: fix minor issues around relay payment and relay payment test
lavanet · Dec 22, 2022 · dfd9ec4 · dfd9ec4
2 parents 0fbcd15 + 0dead9d
commit dfd9ec4
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 35 deletions.
diff --git a/x/pairing/keeper/msg_server_relay_payment.go b/x/pairing/keeper/msg_server_relay_payment.go
@@ -340,7 +340,11 @@ func (k msgServer) dealWithUnresponsiveProviders(ctx sdk.Context, unresponsiveDa
 			} else if totalPaymentRequests*providerPaymentMultiplier < len(providerPaymentStorage.UnresponsivenessComplaints) {
 				// unstake provider
 				utils.LogLavaEvent(ctx, logger, "jailing_event", map[string]string{"provider_address": sdkUnresponsiveProviderAddress.String(), "chain_id": chainID}, "Unresponsive provider was unstaked from the chain due to unresponsiveness")
-				k.unSafeUnstakeProviderEntry(ctx, epochstoragetypes.ProviderKey, chainID, indexInStakeStorage, existingEntry)
+				err = k.unSafeUnstakeProviderEntry(ctx, epochstoragetypes.ProviderKey, chainID, indexInStakeStorage, existingEntry)
+				if err != nil {
+					utils.LavaFormatError("unable to unstake provider entry (unsafe method)", err, &map[string]string{"chainID": chainID, "indexInStakeStorage": strconv.FormatUint(indexInStakeStorage, 10), "existingEntry": existingEntry.GetStake().String()})
+					continue
+				}
 			}
 		}
 		// set the final provider payment storage state including the complaints
@@ -367,11 +371,11 @@ func (k msgServer) getTotalPaymentsForPreviousEpochs(ctx sdk.Context, numberOfEp
 	return totalPaymentRequests, nil
 }
 
-func (k msgServer) unSafeUnstakeProviderEntry(ctx sdk.Context, providerKey string, chainID string, indexInStakeStorage uint64, existingEntry epochstoragetypes.StakeEntry) {
+func (k msgServer) unSafeUnstakeProviderEntry(ctx sdk.Context, providerKey string, chainID string, indexInStakeStorage uint64, existingEntry epochstoragetypes.StakeEntry) error {
 	err := k.epochStorageKeeper.RemoveStakeEntryCurrent(ctx, epochstoragetypes.ProviderKey, chainID, indexInStakeStorage)
 	if err != nil {
-		utils.LavaError(ctx, k.Logger(ctx), "relay_payment_unstake", map[string]string{"existingEntry": fmt.Sprintf("%+v", existingEntry)}, "tried to unstake unsafe but didnt find entry")
-	} else {
-		k.epochStorageKeeper.AppendUnstakeEntry(ctx, epochstoragetypes.ProviderKey, existingEntry)
+		return utils.LavaError(ctx, k.Logger(ctx), "relay_payment_unstake", map[string]string{"existingEntry": fmt.Sprintf("%+v", existingEntry)}, "tried to unstake unsafe but didnt find entry")
 	}
+	k.epochStorageKeeper.AppendUnstakeEntry(ctx, epochstoragetypes.ProviderKey, existingEntry)
+	return nil
 }
diff --git a/x/pairing/keeper/msg_server_relay_payment_gov_test.go b/x/pairing/keeper/msg_server_relay_payment_gov_test.go
@@ -998,9 +998,6 @@ func verifyRelayPaymentObjects(t *testing.T, ts *testStruct, relayRequest *pairi
 	}
 	require.NotEmpty(t, providerPaymentStorageFromEpochPayments.GetIndex())
 	require.Equal(t, uint64(relayRequest.GetBlockHeight()), providerPaymentStorageFromEpochPayments.GetEpoch())
-	usedCUFromproviderPaymentStorageFromEpochPayments, err := ts.keepers.Pairing.GetTotalUsedCUForConsumerPerEpoch(sdk.UnwrapSDKContext(ts.ctx), ts.clients[0].address.String(), providerPaymentStorageFromEpochPayments.UniquePaymentStorageClientProvider, ts.providers[0].address.String())
-	require.Nil(t, err)
-	require.Equal(t, relayRequest.GetCuSum(), usedCUFromproviderPaymentStorageFromEpochPayments)
 
 	// Get the UniquePaymentStorageClientProvider key
 	uniquePaymentStorageClientProviderKey := ts.keepers.Pairing.EncodeUniquePaymentKey(sdk.UnwrapSDKContext(ts.ctx), ts.clients[0].address, ts.providers[0].address, strconv.FormatUint(relayRequest.SessionId, 16), ts.spec.Name)
@@ -1014,6 +1011,7 @@ func verifyRelayPaymentObjects(t *testing.T, ts *testStruct, relayRequest *pairi
 	}
 	require.NotEmpty(t, uniquePaymentStorageClientProviderFromProviderPaymentStorage.GetIndex())
 	require.Equal(t, uint64(relayRequest.GetBlockHeight()), uniquePaymentStorageClientProviderFromProviderPaymentStorage.GetBlock())
+	require.Equal(t, relayRequest.GetCuSum(), uniquePaymentStorageClientProviderFromProviderPaymentStorage.GetUsedCU())
 
 	// when checking CU, the client may be trying to use a relay request with more CU than his MaxCU (determined by StakeThreshold)
 	clientStakeEntry, err := ts.keepers.Epochstorage.GetStakeEntryForClientEpoch(sdk.UnwrapSDKContext(ts.ctx), relayRequest.GetChainID(), ts.clients[0].address, uint64(relayRequest.GetBlockHeight()))
@@ -1041,4 +1039,5 @@ func verifyRelayPaymentObjects(t *testing.T, ts *testStruct, relayRequest *pairi
 	} else {
 		require.Equal(t, relayRequest.GetCuSum(), uniquePaymentStorageClientProvider.GetUsedCU())
 	}
+	require.Equal(t, relayRequest.GetCuSum(), uniquePaymentStorageClientProvider.GetUsedCU())
 }
diff --git a/x/pairing/keeper/msg_server_relay_payment_test.go b/x/pairing/keeper/msg_server_relay_payment_test.go
@@ -85,61 +85,73 @@ func (ts *testStruct) getProvider(addr string) *account {
 	return nil
 }
 
-// Test that a provider payment is valid if he asked it within the chain's memory, and check the payment object
+// Test that a provider payment is valid if he asked it within the chain's memory, and check the relay payment object (RPO)
 func TestRelayPaymentMemoryTransferAfterEpochChange(t *testing.T) {
 
 	// setup testnet with mock spec, a staked client and a staked provider
 	ts := setupForPaymentTest(t)
 
-	// Get epochsToSave - the number of epochs chain can save to its memory
-	epochsToSave, err := ts.keepers.Epochstorage.EpochsToSave(sdk.UnwrapSDKContext(ts.ctx), uint64(sdk.UnwrapSDKContext(ts.ctx).BlockHeight()))
-	require.Nil(t, err)
+	// Advance epoch to apply client and provider stake
+	ts.ctx = testkeeper.AdvanceEpoch(ts.ctx, ts.keepers)
+	firstEpoch := ts.keepers.Epochstorage.GetEpochStart(sdk.UnwrapSDKContext(ts.ctx))
 
-	// Advance epochsToSave*2 epochs (i.e. in the next epoch, the first epoch will be forgotten)
-	for i := 0; i < int(2*epochsToSave); i++ {
-		ts.ctx = testkeeper.AdvanceEpoch(ts.ctx, ts.keepers)
-	}
+	// Get epochsToSave, and epochBlocks
+	epochsToSave, err := ts.keepers.Epochstorage.EpochsToSave(sdk.UnwrapSDKContext(ts.ctx), firstEpoch)
+	require.Nil(t, err)
+	epochBlocks, err := ts.keepers.Epochstorage.EpochBlocks(sdk.UnwrapSDKContext(ts.ctx), firstEpoch)
+	require.Nil(t, err)
 
-	// Get currentEpoch, lastBlockInEpoch, blockNumInEpoch
-	currentEpoch := ts.keepers.Epochstorage.GetEpochStart(sdk.UnwrapSDKContext(ts.ctx))
-	lastBlockInEpoch := ts.keepers.Epochstorage.EpochBlocksRaw(sdk.UnwrapSDKContext(ts.ctx)) - 1
-	blockNumInEpoch := ts.keepers.Epochstorage.EpochBlocksRaw(sdk.UnwrapSDKContext(ts.ctx))
+	// The last block offset in each epoch is epochBlocks-1
+	lastBlockInEpoch := epochBlocks - 1
 
 	// define tests - different epoch+blocks, valid tells if the payment request should work
 	tests := []struct {
-		name  string
-		epoch uint64
-		block uint64
-		valid bool
+		name            string
+		epochsToAdvance uint64 // number of epochs to advance in the test
+		blocksToAdvance uint64 // number of blocks to advance in the test
+		valid           bool
 	}{
-		{"PaymentCurrentEpoch", currentEpoch, 0, true},                                                                 // first block of current epoch
-		{"PaymentPreviousEpoch", currentEpoch - blockNumInEpoch, 0, true},                                              // first block of previous epoch
-		{"PaymentEndOfMemoryEpochLastBlock", currentEpoch - epochsToSave*blockNumInEpoch, lastBlockInEpoch, true},      // last block of end of memory epoch
-		{"PaymentEndOfMemoryEpochFirstBlock", currentEpoch - epochsToSave*blockNumInEpoch, 0, true},                    // first block of end of memory epoch
-		{"PaymentOutOfMemoryEpochLastBlock", currentEpoch - (epochsToSave+1)*blockNumInEpoch, lastBlockInEpoch, false}, // first block of out of memory epoch (end of memory+1)
-		{"PaymentOutOfMemoryEpochFirstBlock", currentEpoch - (epochsToSave+1)*blockNumInEpoch, 0, false},               // last block of out of memory epoch (end of memory+1)
+		{"PaymentFirstEpoch", 0, 0, true},                                // first block of current epoch
+		{"PaymentEndOfMemoryEpochFirstBlock", epochsToSave, 0, true},     // first block of end of memory epoch
+		{"PaymentEndOfMemoryEpochLastBlock", 0, lastBlockInEpoch, true},  // last block of end of memory epoch
+		{"PaymentOutOfMemoryEpochFirstBlock", 0, 1, false},               // first block of out of memory epoch (end of memory+1)
+		{"PaymentOutOfMemoryEpochLastBlock", 0, lastBlockInEpoch, false}, // last block of out of memory epoch (end of memory+1)
 	}
 
-	sessionCounter := 0
+	sessionCounter := uint64(0)
 	for _, tt := range tests {
 		sessionCounter += 1
 		t.Run(tt.name, func(t *testing.T) {
 
-			// Create relay request that was done in the test's epoch+block. Change session ID each iteration to avoid double spending error (provider asks reward for the same transaction twice)
+			// Advance epochs according to the epochsToAdvance
+			if tt.epochsToAdvance != 0 {
+				for i := 0; i < int(tt.epochsToAdvance); i++ {
+					ts.ctx = testkeeper.AdvanceEpoch(ts.ctx, ts.keepers)
+				}
+			}
+
+			// Advance blocks according to the blocksToAdvance
+			if tt.blocksToAdvance != 0 {
+				for i := 0; i < int(tt.blocksToAdvance); i++ {
+					ts.ctx = testkeeper.AdvanceBlock(ts.ctx, ts.keepers)
+				}
+			}
+
+			// Create relay request that was done in the first epoch. Change session ID each iteration to avoid double spending error (provider asks reward for the same transaction twice)
 			relayRequest := &types.RelayRequest{
 				Provider:        ts.providers[0].address.String(),
 				ApiUrl:          "",
 				Data:            []byte(ts.spec.Apis[0].Name),
-				SessionId:       uint64(sessionCounter),
+				SessionId:       sessionCounter,
 				ChainID:         ts.spec.Name,
 				CuSum:           ts.spec.Apis[0].ComputeUnits * 10,
-				BlockHeight:     int64(tt.epoch + tt.block),
+				BlockHeight:     int64(firstEpoch),
 				RelayNum:        0,
 				RequestBlock:    -1,
 				DataReliability: nil,
 			}
 
-			// Sign and send the payment requests for block 0 tx
+			// Sign and send the payment requests
 			sig, err := sigs.SignRelay(ts.clients[0].secretKey, *relayRequest)
 			relayRequest.Sig = sig
 			require.Nil(t, err)
@@ -149,8 +161,13 @@ func TestRelayPaymentMemoryTransferAfterEpochChange(t *testing.T) {
 			Relays = append(Relays, relayRequest)
 			relayPaymentMessage := types.MsgRelayPayment{Creator: ts.providers[0].address.String(), Relays: Relays}
 			payAndVerifyBalance(t, ts, relayPaymentMessage, tt.valid, ts.clients[0].address, ts.providers[0].address)
+
+			// Check the RPO exists (shouldn't exist after epochsToSave+1 passes)
+			verifyRelayPaymentObjects(t, ts, relayRequest, tt.valid)
+
 		})
 	}
+
 }
 
 func setupForPaymentTest(t *testing.T) *testStruct {