Skip to content

Commit

Permalink
[AUDIT FIX] - updated total delegations in confirm delegation
Browse files Browse the repository at this point in the history
  • Loading branch information
sampocs committed Jun 20, 2024
1 parent f1fe92c commit df38a31
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 38 deletions.
6 changes: 6 additions & 0 deletions x/staketia/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ func (k Keeper) ConfirmDelegation(ctx sdk.Context, recordId uint64, txHash strin
if err != nil {
return err
}
stakeibcHostZone, err := k.stakeibcKeeper.GetActiveHostZone(ctx, types.CelestiaChainId)
if err != nil {
return err
}

// verify delegation record is nonzero
if !delegationRecord.NativeAmount.IsPositive() {
Expand All @@ -107,7 +111,9 @@ func (k Keeper) ConfirmDelegation(ctx sdk.Context, recordId uint64, txHash strin

// increment delegation on Host Zone
hostZone.RemainingDelegatedBalance = hostZone.RemainingDelegatedBalance.Add(delegationRecord.NativeAmount)
stakeibcHostZone.TotalDelegations = stakeibcHostZone.TotalDelegations.Add(delegationRecord.NativeAmount)
k.SetHostZone(ctx, hostZone)
k.stakeibcKeeper.SetHostZone(ctx, stakeibcHostZone)

EmitSuccessfulConfirmDelegationEvent(ctx, recordId, delegationRecord.NativeAmount, txHash, sender)
return nil
Expand Down
19 changes: 17 additions & 2 deletions x/staketia/keeper/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
ibctesting "github.com/cosmos/ibc-go/v7/testing"

stakeibctypes "github.com/Stride-Labs/stride/v22/x/stakeibc/types"
"github.com/Stride-Labs/stride/v22/x/staketia/types"
)

Expand Down Expand Up @@ -151,10 +152,18 @@ func (s *KeeperTestSuite) SetupDelegationRecords() {
s.App.StaketiaKeeper.SetDelegationRecord(s.Ctx, delegationRecord)
}

// Set HostZone
// Set staketia hostZone
hostZone := s.initializeHostZone()
hostZone.RemainingDelegatedBalance = InitialDelegation
s.App.StaketiaKeeper.SetHostZone(s.Ctx, hostZone)

// Set stakeibc host zone with the same total delegation
stakeibcHostZone := stakeibctypes.HostZone{
ChainId: types.CelestiaChainId,
TotalDelegations: InitialDelegation,
Halted: false,
}
s.App.StakeibcKeeper.SetHostZone(s.Ctx, stakeibcHostZone)
}

func (s *KeeperTestSuite) VerifyDelegationRecords(verifyIdentical bool, archiveIds ...uint64) {
Expand Down Expand Up @@ -219,8 +228,14 @@ func (s *KeeperTestSuite) TestConfirmDelegation_Successful() {
s.Require().Equal(ValidTxHashNew, loadedDelegationRecord.TxHash, "delegation record should be updated with txHash")

// verify hostZone delegated balance is same as initial delegation + 6000
expectedDelegation := InitialDelegation.Int64() + 6000

hostZone := s.MustGetHostZone()
s.Require().Equal(InitialDelegation.Int64()+6000, hostZone.RemainingDelegatedBalance.Int64(), "hostZone delegated balance should have increased by 6000")
s.Require().Equal(expectedDelegation, hostZone.RemainingDelegatedBalance.Int64(), "staketia remaining delegated balance")

stakeibcHostZone, found := s.App.StakeibcKeeper.GetHostZone(s.Ctx, types.CelestiaChainId)
s.Require().True(found)
s.Require().Equal(expectedDelegation, stakeibcHostZone.TotalDelegations.Int64(), "stakeibc total delegations")
}

func (s *KeeperTestSuite) TestConfirmDelegation_DelegationZero() {
Expand Down
12 changes: 2 additions & 10 deletions x/staketia/keeper/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"

errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"

recordtypes "github.com/Stride-Labs/stride/v22/x/records/types"
Expand Down Expand Up @@ -48,15 +47,8 @@ func (k Keeper) UpdateStakeibcHostZone(ctx sdk.Context, legacyHostZone oldtypes.
stakeibcHostZone.MinInnerRedemptionRate = legacyHostZone.MinInnerRedemptionRate
stakeibcHostZone.MaxInnerRedemptionRate = legacyHostZone.MaxInnerRedemptionRate

// Set the total delegations to the sum of the staketia total, plus any delegation records
// This is so we don't have to trigger any stakeibc account changes when delegations are
// confirmed from staketia
// In practice, if timed right, there should be no delegation records
pendingDelegations := sdkmath.ZeroInt()
for _, delegationRecord := range k.GetAllActiveDelegationRecords(ctx) {
pendingDelegations = pendingDelegations.Add(delegationRecord.NativeAmount)
}
stakeibcHostZone.TotalDelegations = legacyHostZone.DelegatedBalance.Add(pendingDelegations)
// Set the total delegations to the sum of the staketia total
stakeibcHostZone.TotalDelegations = legacyHostZone.DelegatedBalance
k.stakeibcKeeper.SetHostZone(ctx, stakeibcHostZone)

return stakeibcHostZone, nil
Expand Down
32 changes: 6 additions & 26 deletions x/staketia/keeper/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,14 @@ import (
)

func (s *KeeperTestSuite) TestUpdateStakeibcHostZone() {
// Create deposit records with amounts 100 and 200 respectively
delegationRecords := []types.DelegationRecord{
{Id: 1, Status: types.TRANSFER_IN_PROGRESS, NativeAmount: sdk.NewInt(100)},
{Id: 2, Status: types.DELEGATION_QUEUE, NativeAmount: sdk.NewInt(200)},
}
for _, delegationRecord := range delegationRecords {
s.App.StaketiaKeeper.SetDelegationRecord(s.Ctx, delegationRecord)
}

// Create a host zone with a delegated balance of 1000
totalDelegations := sdk.NewInt(1_000)
redemptionRate := sdk.NewDec(2)
minInnerRedemptionRate := sdk.MustNewDecFromStr("1.9")
maxInnerRedemptionRate := sdk.MustNewDecFromStr("2.1")
legacyHostZone := oldtypes.HostZone{
RedemptionRate: redemptionRate,
DelegatedBalance: sdk.NewInt(1_000),
DelegatedBalance: totalDelegations,
MinInnerRedemptionRate: minInnerRedemptionRate,
MaxInnerRedemptionRate: maxInnerRedemptionRate,
}
Expand All @@ -39,16 +31,12 @@ func (s *KeeperTestSuite) TestUpdateStakeibcHostZone() {
s.App.StaketiaKeeper.SetLegacyHostZone(s.Ctx, legacyHostZone)
s.App.StakeibcKeeper.SetHostZone(s.Ctx, stakeibcHostZone)

// The expected stakeibc host zone should have total delegations
// equal to 1000 + 100 + 200 = 1300
expectedStakeibcTotalDelegations := sdkmath.NewInt(1_000 + 100 + 200)

// Call the update host zone function and confirm against expectations
actualStakeibcHostZone, err := s.App.StaketiaKeeper.UpdateStakeibcHostZone(s.Ctx, legacyHostZone)
s.Require().NoError(err, "no error expected when updating host zone")

s.Require().Equal(types.CelestiaChainId, actualStakeibcHostZone.ChainId, "chain ID")
s.Require().Equal(expectedStakeibcTotalDelegations, actualStakeibcHostZone.TotalDelegations, "total delegations")
s.Require().Equal(totalDelegations, actualStakeibcHostZone.TotalDelegations, "total delegations")
s.Require().Equal(redemptionRate, actualStakeibcHostZone.RedemptionRate, "redemption rate")
s.Require().Equal(minInnerRedemptionRate, actualStakeibcHostZone.MinInnerRedemptionRate, "min redemption rate")
s.Require().Equal(maxInnerRedemptionRate, actualStakeibcHostZone.MaxInnerRedemptionRate, "max redemption rate")
Expand Down Expand Up @@ -128,6 +116,7 @@ func (s *KeeperTestSuite) TestInitiateMigration() {
// Fund the staketia deposit and fee accounts
depositBalance := sdkmath.NewInt(1000)
feeBalance := sdkmath.NewInt(2000)
totalDelegations := sdk.NewInt(1000)
s.FundAccount(staketiaDepositAccount, sdk.NewCoin(HostIBCDenom, depositBalance))
s.FundModuleAccount(staketiaFeeModuleName, sdk.NewCoin(HostIBCDenom, feeBalance))

Expand All @@ -141,19 +130,10 @@ func (s *KeeperTestSuite) TestInitiateMigration() {
MinRedemptionRate: sdk.MustNewDecFromStr("0.90"),
MaxRedemptionRate: sdk.MustNewDecFromStr("1.5"),
RedemptionRate: sdk.MustNewDecFromStr("1.2"),
DelegatedBalance: sdk.NewInt(1000),
DelegatedBalance: totalDelegations,
}
s.App.StaketiaKeeper.SetLegacyHostZone(s.Ctx, legacyHostZone)

// Create a delegation record that will be used in the delegated balance migration
delegationRecord := types.DelegationRecord{
Id: 1,
Status: types.DELEGATION_QUEUE,
NativeAmount: sdk.NewInt(100),
}
s.App.StaketiaKeeper.SetDelegationRecord(s.Ctx, delegationRecord)
expectedTotalDelegations := legacyHostZone.DelegatedBalance.Add(delegationRecord.NativeAmount)

// Create epoch trackers and EURs which are needed for the stakeibc registration
s.App.StakeibcKeeper.SetEpochTracker(s.Ctx, stakeibctypes.EpochTracker{
EpochIdentifier: epochtypes.DAY_EPOCH,
Expand Down Expand Up @@ -196,7 +176,7 @@ func (s *KeeperTestSuite) TestInitiateMigration() {
s.Require().Equal(uint64(types.CelestiaUnbondingPeriodDays), hostZone.UnbondingPeriod, "unbonding period")

s.Require().False(hostZone.RedemptionsEnabled, "redemptions enabled")
s.Require().Equal(expectedTotalDelegations, hostZone.TotalDelegations, "total delegations")
s.Require().Equal(totalDelegations, hostZone.TotalDelegations, "total delegations")

// Confirm balances were transferred
stakeibcDepositAccount := sdk.MustAccAddressFromBech32(hostZone.DepositAddress)
Expand Down

0 comments on commit df38a31

Please sign in to comment.