LaconicNetwork/cosmos-sdk
10
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix: call AfterUnbondingInitiated for new unbonding entries only (#16043)

Browse Source
This commit is contained in:
MSalopek 2023-05-11 11:46:48 +02:00 committed by GitHub
parent 69642f6176
commit b28c50fdd2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 137 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -208,7 +208,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (cli) [#15299](https://github.com/cosmos/cosmos-sdk/pull/15299) remove `--amino` flag from `sign` and `multi-sign` commands. Amino `StdTx` has been deprecated for a while. Amino JSON signing still works as expected.
### Bug Fixes
* (x/staking) [#16043](https://github.com/cosmos/cosmos-sdk/pull/16043) Call `AfterUnbondingInitiated` hook for new unbonding entries only and fix `UnbondingDelegation` entries handling
* (types) [#16010](https://github.com/cosmos/cosmos-sdk/pull/16010) Let `module.CoreAppModuleBasicAdaptor` fallback to legacy genesis handling.
* (x/group) [#16017](https://github.com/cosmos/cosmos-sdk/pull/16017) Correctly apply account number in group v2 migration.
* (types) [#15691](https://github.com/cosmos/cosmos-sdk/pull/15691) Make `Coin.Validate()` check that `.Amount` is not nil.

16
x/staking/keeper/delegation.go
View File

@ -335,21 +335,25 @@ func (k Keeper) SetUnbondingDelegationEntry(
) types.UnbondingDelegation {
ubd, found := k.GetUnbondingDelegation(ctx, delegatorAddr, validatorAddr)
id := k.IncrementUnbondingID(ctx)
isNewUbdEntry := true
if found {
ubd.AddEntry(creationHeight, minTime, balance, id)
isNewUbdEntry = ubd.AddEntry(creationHeight, minTime, balance, id)
} else {
ubd = types.NewUnbondingDelegation(delegatorAddr, validatorAddr, creationHeight, minTime, balance, id)
}
k.SetUnbondingDelegation(ctx, ubd)
// Add to the UBDByUnbondingOp index to look up the UBD by the UBDE ID
k.SetUnbondingDelegationByUnbondingID(ctx, ubd, id)
// only call the hook for new entries since
// calls to AfterUnbondingInitiated are not idempotent
if isNewUbdEntry {
// Add to the UBDByUnbondingOp index to look up the UBD by the UBDE ID
k.SetUnbondingDelegationByUnbondingID(ctx, ubd, id)
if err := k.Hooks().AfterUnbondingInitiated(ctx, id); err != nil {
k.Logger(ctx).Error("failed to call after unbonding initiated hook", "error", err)
if err := k.Hooks().AfterUnbondingInitiated(ctx, id); err != nil {
k.Logger(ctx).Error("failed to call after unbonding initiated hook", "error", err)
}
}
return ubd
}

120
x/staking/keeper/delegation_test.go
View File

@ -1063,3 +1063,123 @@ func (s *KeeperTestSuite) TestRedelegateFromUnbondedValidator() {
red, found := keeper.GetRedelegation(ctx, addrDels[0], addrVals[0], addrVals[1])
require.False(found, "%v", red)
}
func (s *KeeperTestSuite) TestUnbondingDelegationAddEntry() {
require := s.Require()
delAddrs, valAddrs := createValAddrs(1)
for _, addr := range delAddrs {
s.accountKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes()
s.accountKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes()
}
delAddr := delAddrs[0]
valAddr := valAddrs[0]
creationHeight := int64(10)
ubd := stakingtypes.NewUnbondingDelegation(
delAddr,
valAddr,
creationHeight,
time.Unix(0, 0).UTC(),
math.NewInt(10),
0,
)
var initialEntries []stakingtypes.UnbondingDelegationEntry
initialEntries = append(initialEntries, ubd.Entries...)
require.Len(initialEntries, 1)
isNew := ubd.AddEntry(creationHeight, time.Unix(0, 0).UTC(), math.NewInt(5), 1)
require.False(isNew)
require.Len(ubd.Entries, 1) // entry was merged
require.NotEqual(initialEntries, ubd.Entries)
require.Equal(creationHeight, ubd.Entries[0].CreationHeight)
require.Equal(initialEntries[0].UnbondingId, ubd.Entries[0].UnbondingId) // unbondingID remains unchanged
require.Equal(ubd.Entries[0].Balance, math.NewInt(15)) // 10 from previous + 5 from merged
newCreationHeight := int64(11)
isNew = ubd.AddEntry(newCreationHeight, time.Unix(1, 0).UTC(), math.NewInt(5), 2)
require.True(isNew)
require.Len(ubd.Entries, 2) // entry was appended
require.NotEqual(initialEntries, ubd.Entries)
require.Equal(creationHeight, ubd.Entries[0].CreationHeight)
require.Equal(newCreationHeight, ubd.Entries[1].CreationHeight)
require.Equal(ubd.Entries[0].Balance, math.NewInt(15))
require.Equal(ubd.Entries[1].Balance, math.NewInt(5))
require.NotEqual(ubd.Entries[0].UnbondingId, ubd.Entries[1].UnbondingId) // appended entry has a new unbondingID
}
func (s *KeeperTestSuite) TestSetUnbondingDelegationEntry() {
ctx, keeper := s.ctx, s.stakingKeeper
require := s.Require()
delAddrs, valAddrs := createValAddrs(1)
for _, addr := range delAddrs {
s.accountKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes()
s.accountKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes()
}
delAddr := delAddrs[0]
valAddr := valAddrs[0]
creationHeight := int64(0)
ubd := stakingtypes.NewUnbondingDelegation(
delAddr,
valAddr,
creationHeight,
time.Unix(0, 0).UTC(),
math.NewInt(5),
0,
)
// set and retrieve a record
keeper.SetUnbondingDelegation(ctx, ubd)
resUnbond, found := keeper.GetUnbondingDelegation(ctx, delAddr, valAddr)
require.True(found)
require.Equal(ubd, resUnbond)
initialEntries := ubd.Entries
require.Len(initialEntries, 1)
require.Equal(initialEntries[0].Balance, math.NewInt(5))
require.Equal(initialEntries[0].UnbondingId, uint64(0)) // initial unbondingID
// set unbonding delegation entry for existing creationHeight
// entries are expected to be merged
keeper.SetUnbondingDelegationEntry(
ctx,
delAddr,
valAddr,
creationHeight,
time.Unix(0, 0).UTC(),
math.NewInt(5),
)
resUnbonding, found := keeper.GetUnbondingDelegation(ctx, delAddr, valAddr)
require.True(found)
require.Len(resUnbonding.Entries, 1)
require.NotEqual(initialEntries, resUnbonding.Entries)
require.Equal(creationHeight, resUnbonding.Entries[0].CreationHeight)
require.Equal(initialEntries[0].UnbondingId, resUnbonding.Entries[0].UnbondingId) // initial unbondingID remains unchanged
require.Equal(resUnbonding.Entries[0].Balance, math.NewInt(10)) // 5 from previous entry + 5 from merged entry
// set unbonding delegation entry for newCreationHeight
// new entry is expected to be appended to the existing entries
newCreationHeight := int64(1)
keeper.SetUnbondingDelegationEntry(
ctx,
delAddr,
valAddr,
newCreationHeight,
time.Unix(1, 0).UTC(),
math.NewInt(10),
)
resUnbonding, found = keeper.GetUnbondingDelegation(ctx, delAddr, valAddr)
require.True(found)
require.Len(resUnbonding.Entries, 2)
require.NotEqual(initialEntries, resUnbonding.Entries)
require.NotEqual(resUnbonding.Entries[0], resUnbonding.Entries[1])
require.Equal(creationHeight, resUnbonding.Entries[0].CreationHeight)
require.Equal(newCreationHeight, resUnbonding.Entries[1].CreationHeight)
// unbondingID is incremented on every call to SetUnbondingDelegationEntry
// unbondingID == 1 was skipped because the entry was merged with the existing entry with unbondingID == 0
// unbondingID comes from a global counter -> gaps in unbondingIDs are OK as long as every unbondingID is unique
require.Equal(uint64(2), resUnbonding.Entries[1].UnbondingId)
}

11
x/staking/types/delegation.go
View File

@ -127,7 +127,7 @@ func NewUnbondingDelegation(
}
// AddEntry - append entry to the unbonding delegation
func (ubd *UnbondingDelegation) AddEntry(creationHeight int64, minTime time.Time, balance math.Int, unbondingID uint64) {
func (ubd *UnbondingDelegation) AddEntry(creationHeight int64, minTime time.Time, balance math.Int, unbondingID uint64) bool {
// Check the entries exists with creation_height and complete_time
entryIndex := -1
for index, ubdEntry := range ubd.Entries {
@ -144,11 +144,12 @@ func (ubd *UnbondingDelegation) AddEntry(creationHeight int64, minTime time.Time
// update the entry
ubd.Entries[entryIndex] = ubdEntry
} else {
// append the new unbond delegation entry
entry := NewUnbondingDelegationEntry(creationHeight, minTime, balance, unbondingID)
ubd.Entries = append(ubd.Entries, entry)
return false
}
// append the new unbond delegation entry
entry := NewUnbondingDelegationEntry(creationHeight, minTime, balance, unbondingID)
ubd.Entries = append(ubd.Entries, entry)
return true
}
// RemoveEntry - remove entry at index i to the unbonding delegation