From 2c732eaead41c245be142d6b9700825b4b34751b Mon Sep 17 00:00:00 2001 From: TimmyExogenous <144751317+TimmyExogenous@users.noreply.github.com> Date: Tue, 15 Oct 2024 10:59:18 +0800 Subject: [PATCH] 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 --- x/delegation/keeper/abci.go | 67 ++++++++++++++----------------------- x/operator/keeper/abci.go | 20 +++++++---- x/operator/keeper/slash.go | 7 ++-- 3 files changed, 44 insertions(+), 50 deletions(-) diff --git a/x/delegation/keeper/abci.go b/x/delegation/keeper/abci.go index 8a44e7496..47ca02748 100644 --- 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 index 189272672..b5a16b4ba 100644 --- 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 index f2c17abef..1f1c6a58d 100644 --- 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