fix(abci): adjust the error handling in beginBlock and endBlock (#207)

* detail the return error for operator info * adjust the error handling in beginBlock and endBlock * rebase and fix the conflict
ExocoreNetwork · Oct 15, 2024 · 2c732ea · 2c732ea
1 parent 2b7a55c
commit 2c732ea
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 50 deletions.
diff --git a/x/delegation/keeper/abci.go b/x/delegation/keeper/abci.go
@@ -19,32 +19,33 @@ func (k *Keeper) EndBlock(
 		originalCtx, uint64(originalCtx.BlockHeight()),
 	)
 	if err != nil {
-		logger.Error("failed to get pending undelegation records", "error", err)
+		// When encountering an error while retrieving pending undelegation, skip the undelegation at the given height without causing the node to stop running.
+		logger.Error("Error in GetPendingUndelegationRecords during the delegation's EndBlock execution", "error", err)
+		return []abci.ValidatorUpdate{}
 	}
 	if len(records) == 0 {
-		logger.Info("no pending undelegation records")
 		return []abci.ValidatorUpdate{}
 	}
 	for i := range records {
 		record := records[i] // avoid implicit memory aliasing
-		ctx, writeCache := originalCtx.CacheContext()
+		cc, writeCache := originalCtx.CacheContext()
 		// we can use `Must` here because we stored this record ourselves.
 		operatorAccAddress := sdk.MustAccAddressFromBech32(record.OperatorAddr)
 		// TODO check if the operator has been slashed or frozen
 		recordID := types.GetUndelegationRecordKey(
 			record.BlockNumber, record.LzTxNonce, record.TxHash, record.OperatorAddr,
 		)
-		if k.GetUndelegationHoldCount(ctx, recordID) > 0 {
+		if k.GetUndelegationHoldCount(cc, recordID) > 0 {
 			// delete from all 3 states
-			if err := k.DeleteUndelegationRecord(ctx, record); err != nil {
+			if err := k.DeleteUndelegationRecord(cc, record); err != nil {
 				logger.Error("failed to delete undelegation record", "error", err)
 				continue
 			}
 			// add back to all 3 states, with the new block height
 			// #nosec G701
-			record.CompleteBlockNumber = uint64(ctx.BlockHeight()) + 1
+			record.CompleteBlockNumber = uint64(cc.BlockHeight()) + 1
 			if err := k.SetUndelegationRecords(
-				ctx, []types.UndelegationRecord{*record},
+				cc, []types.UndelegationRecord{*record},
 			); err != nil {
 				logger.Error("failed to set undelegation records", "error", err)
 				continue
@@ -58,13 +59,9 @@ func (k *Keeper) EndBlock(
 		deltaAmount := &types.DeltaDelegationAmounts{
 			WaitUndelegationAmount: recordAmountNeg,
 		}
-		if _, err := k.UpdateDelegationState(
-			ctx, record.StakerID, record.AssetID, record.OperatorAddr, deltaAmount,
-		); err != nil {
-			logger.Error(
-				"failed to update delegation state",
-				"error", err,
-			)
+		_, err = k.UpdateDelegationState(cc, record.StakerID, record.AssetID, record.OperatorAddr, deltaAmount)
+		if err != nil {
+			logger.Error("Error in UpdateDelegationState during the delegation's EndBlock execution", "error", err)
 			continue
 		}
 
@@ -88,7 +85,7 @@ func (k *Keeper) EndBlock(
 			}
 			stakerAddr := sdk.AccAddress(stakerAddrBytes)
 			if err := k.bankKeeper.UndelegateCoinsFromModuleToAccount(
-				ctx, types.DelegatedPoolName, stakerAddr,
+				cc, types.DelegatedPoolName, stakerAddr,
 				sdk.NewCoins(
 					sdk.NewCoin(assetstypes.ExocoreAssetDenom, record.ActualCompletedAmount),
 				),
@@ -100,41 +97,29 @@ func (k *Keeper) EndBlock(
 				continue
 			}
 		} else {
-			if err := k.assetsKeeper.UpdateStakerAssetState(
-				ctx, record.StakerID, record.AssetID,
-				assetstypes.DeltaStakerSingleAsset{
-					WithdrawableAmount:        record.ActualCompletedAmount,
-					PendingUndelegationAmount: recordAmountNeg,
-				},
-			); err != nil {
-				logger.Error(
-					"failed to update staker asset state",
-					"error", err,
-				)
+			err = k.assetsKeeper.UpdateStakerAssetState(cc, record.StakerID, record.AssetID, assetstypes.DeltaStakerSingleAsset{
+				WithdrawableAmount:        record.ActualCompletedAmount,
+				PendingUndelegationAmount: recordAmountNeg,
+			})
+			if err != nil {
+				logger.Error("Error in UpdateStakerAssetState during the delegation's EndBlock execution", "error", err)
 				continue
 			}
 		}
 
 		// update the operator state
-		if err := k.assetsKeeper.UpdateOperatorAssetState(
-			ctx, operatorAccAddress, record.AssetID,
-			assetstypes.DeltaOperatorSingleAsset{
-				PendingUndelegationAmount: recordAmountNeg,
-			},
-		); err != nil {
-			logger.Error(
-				"failed to update operator asset state",
-				"error", err,
-			)
+		err = k.assetsKeeper.UpdateOperatorAssetState(cc, operatorAccAddress, record.AssetID, assetstypes.DeltaOperatorSingleAsset{
+			PendingUndelegationAmount: recordAmountNeg,
+		})
+		if err != nil {
+			logger.Error("Error in UpdateOperatorAssetState during the delegation's EndBlock execution", "error", err)
 			continue
 		}
 
 		// delete the Undelegation records that have been complemented
-		if err := k.DeleteUndelegationRecord(ctx, record); err != nil {
-			logger.Error(
-				"failed to delete undelegation record",
-				"error", err,
-			)
+		err = k.DeleteUndelegationRecord(cc, record)
+		if err != nil {
+			logger.Error("Error in DeleteUndelegationRecord during the delegation's EndBlock execution", "error", err)
 			continue
 		}
 		// when calling `writeCache`, events are automatically emitted on the parent context

diff --git a/x/operator/keeper/abci.go b/x/operator/keeper/abci.go
@@ -16,11 +16,10 @@ func (k *Keeper) UpdateVotingPower(ctx sdk.Context, avsAddr string) error {
 	// get assets supported by the AVS
 	// the mock keeper returns all registered assets.
 	assets, err := k.avsKeeper.GetAVSSupportedAssets(ctx, avsAddr)
-	if err != nil {
-		return err
-	}
-	if assets == nil {
-		ctx.Logger().Info("UpdateVotingPower the assets list supported by AVS is nil")
+	// set the voting power to zero if an error is returned, which may prevent malicious behavior
+	// where errors are intentionally triggered to avoid updating the voting power.
+	if err != nil || assets == nil {
+		ctx.Logger().Info("UpdateVotingPower the assets list supported by AVS is nil or can't get the assets list", "error", err)
 		// clear the voting power regarding this AVS if there isn't any assets supported by it.
 		err = k.DeleteAllOperatorsUSDValueForAVS(ctx, avsAddr)
 		if err != nil {
@@ -53,8 +52,11 @@ func (k *Keeper) UpdateVotingPower(ctx sdk.Context, avsAddr string) error {
 	// check if self USD value is more than the minimum self delegation.
 	minimumSelfDelegation, err := k.avsKeeper.GetAVSMinimumSelfDelegation(ctx, avsAddr)
 	if err != nil {
+		// this error is handled earlier when calling `GetAVSSupportedAssets`,
+		// so we don't set the voting power to zero here.
 		return err
 	}
+
 	opFunc := func(operator string, optedUSDValues *operatortypes.OperatorOptedUSDValue) error {
 		// clear the old voting power for the operator
 		*optedUSDValues = operatortypes.OperatorOptedUSDValue{
@@ -74,17 +76,21 @@ func (k *Keeper) UpdateVotingPower(ctx sdk.Context, avsAddr string) error {
 		}
 		return nil
 	}
+
+	// using cache context to ensure the atomicity of the operation.
+	cc, writeFunc := ctx.CacheContext()
 	// iterate all operators of the AVS to update their voting power
 	// and calculate the voting power for AVS
-	err = k.IterateOperatorsForAVS(ctx, avsAddr, true, opFunc)
+	err = k.IterateOperatorsForAVS(cc, avsAddr, true, opFunc)
 	if err != nil {
 		return err
 	}
 	// set the voting power for AVS
-	err = k.SetAVSUSDValue(ctx, avsAddr, avsVotingPower)
+	err = k.SetAVSUSDValue(cc, avsAddr, avsVotingPower)
 	if err != nil {
 		return err
 	}
+	writeFunc()
 	return nil
 }
 

diff --git a/x/operator/keeper/slash.go b/x/operator/keeper/slash.go
@@ -157,10 +157,13 @@ func (k *Keeper) Slash(ctx sdk.Context, parameter *types.SlashInputInfo) error {
 	}
 
 	// slash assets according to the input information
-	executionInfo, err := k.SlashAssets(ctx, parameter)
+	// using cache context to ensure the atomicity of slash execution.
+	cc, writeFunc := ctx.CacheContext()
+	executionInfo, err := k.SlashAssets(cc, parameter)
 	if err != nil {
 		return err
 	}
+	writeFunc()
 	// store the slash information
 	height := ctx.BlockHeight()
 	slashInfo := types.OperatorSlashInfo{
@@ -202,7 +205,7 @@ func (k Keeper) SlashWithInfractionReason(
 	}
 	err := k.Slash(ctx, slashParam)
 	if err != nil {
-		k.Logger(ctx).Error(err.Error(), avsAddr)
+		k.Logger(ctx).Error("error when executing slash", "error", err, "avsAddr", avsAddr)
 		return sdkmath.NewInt(0)
 	}
 	// todo: The returned value should be the amount of burned Exo if we considering a slash from the reward