Skip to content

Commit

Permalink
fix: [Interchain Security] Fix leak in unbonding hooks (#12849)
Browse files Browse the repository at this point in the history
* remove leak for UBDEs and REDEs

* remove leak for val unbondings
  • Loading branch information
mpoke authored and julienrbrt committed Mar 23, 2023
1 parent 0877e55 commit c299773
Show file tree
Hide file tree
Showing 6 changed files with 893 additions and 773 deletions.
3 changes: 2 additions & 1 deletion docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -6910,7 +6910,8 @@ multiplied by exchange rate.
| `unbonding_time` | [google.protobuf.Timestamp](#google.protobuf.Timestamp) | | unbonding_time defines, if unbonding, the min time for the validator to complete unbonding. |
| `commission` | [Commission](#cosmos.staking.v1beta1.Commission) | | commission defines the commission parameters. |
| `min_self_delegation` | [string](#string) | | min_self_delegation is the validator's self declared minimum self delegation. |
| `unbonding_on_hold_ref_count` | [int64](#int64) | | Strictly positive if this validator's unbonding has been stopped by external modules |
| `unbonding_on_hold_ref_count` | [int64](#int64) | | strictly positive if this validator's unbonding has been stopped by external modules |
| `unbonding_ids` | [uint64](#uint64) | repeated | list of unbonding ids, each uniquely identifing an unbonding of this validator |



Expand Down
5 changes: 4 additions & 1 deletion proto/cosmos/staking/v1beta1/staking.proto
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,11 @@ message Validator {
(gogoproto.nullable) = false
];

// Strictly positive if this validator's unbonding has been stopped by external modules
// strictly positive if this validator's unbonding has been stopped by external modules
int64 unbonding_on_hold_ref_count = 12;

// list of unbonding ids, each uniquely identifing an unbonding of this validator
repeated uint64 unbonding_ids = 13;
}

// BondStatus is the status of a validator.
Expand Down
2 changes: 2 additions & 0 deletions x/staking/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,7 @@ func (k Keeper) CompleteUnbonding(ctx sdk.Context, delAddr sdk.AccAddress, valAd
// Proceed with unbonding
ubd.RemoveEntry(int64(i))
i--
k.DeleteUnbondingIndex(ctx, entry.UnbondingId)

// track undelegation only when remaining or truncated shares are non-zero
if !entry.Balance.IsZero() {
Expand Down Expand Up @@ -973,6 +974,7 @@ func (k Keeper) CompleteRedelegation(
if entry.IsMature(ctxTime) && !entry.OnHold() {
red.RemoveEntry(int64(i))
i--
k.DeleteUnbondingIndex(ctx, entry.UnbondingId)

if !entry.InitialBalance.IsZero() {
balances = balances.Add(sdk.NewCoin(bondDenom, entry.InitialBalance))
Expand Down
5 changes: 4 additions & 1 deletion x/staking/keeper/val_state_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,12 +333,16 @@ func (k Keeper) BeginUnbondingValidator(ctx sdk.Context, validator types.Validat
panic(fmt.Sprintf("should not already be unbonded or unbonding, validator: %v\n", validator))
}

id := k.IncrementUnbondingId(ctx)

validator = validator.UpdateStatus(types.Unbonding)

// set the unbonding completion time and completion height appropriately
validator.UnbondingTime = ctx.BlockHeader().Time.Add(params.UnbondingTime)
validator.UnbondingHeight = ctx.BlockHeader().Height

validator.UnbondingIds = append(validator.UnbondingIds, id)

// save the now unbonded validator record and power index
k.SetValidator(ctx, validator)
k.SetValidatorByPowerIndex(ctx, validator)
Expand All @@ -353,7 +357,6 @@ func (k Keeper) BeginUnbondingValidator(ctx sdk.Context, validator types.Validat
}
k.AfterValidatorBeginUnbonding(ctx, consAddr, validator.GetOperator())

id := k.IncrementUnbondingId(ctx)
k.SetValidatorByUnbondingIndex(ctx, validator, id)

k.AfterUnbondingInitiated(ctx, id)
Expand Down
6 changes: 6 additions & 0 deletions x/staking/keeper/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,9 +437,15 @@ func (k Keeper) UnbondAllMatureValidators(ctx sdk.Context) {
panic("unexpected validator in unbonding queue; status was not unbonding")
}
if val.UnbondingOnHoldRefCount == 0 {
for _, id := range val.UnbondingIds {
k.DeleteUnbondingIndex(ctx, id)
}
val = k.UnbondingToUnbonded(ctx, val)
if val.GetDelegatorShares().IsZero() {
k.RemoveValidator(ctx, val.GetOperator())
} else {
// remove unbonding ids
val.UnbondingIds = []uint64{}
}
// remove validator from queue
k.DeleteValidatorQueue(ctx, val)
Expand Down
Loading

0 comments on commit c299773

Please sign in to comment.