Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add claim rewards ica #961

Merged
merged 19 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
18b3e3e
airdrop testing
riley-stride Mar 28, 2023
db50e48
merge
riley-stride May 2, 2023
c5a94dd
Merge branch 'main' of ssh://github.com/Stride-Labs/stride
riley-stride May 27, 2023
4509194
Merge branch 'main' of ssh://github.com/Stride-Labs/stride
riley-stride Aug 21, 2023
d85a004
Merge branch 'main' of ssh://github.com/Stride-Labs/stride
riley-stride Sep 14, 2023
64b4dbb
Merge branch 'main' of ssh://github.com/Stride-Labs/stride
riley-stride Sep 20, 2023
6ff129b
Merge branch 'main' of ssh://github.com/Stride-Labs/stride
riley-stride Oct 25, 2023
aeaf4ef
feat: claim accrued staking rewards epochly
riley-stride Oct 25, 2023
6f1c436
claim working
riley-stride Nov 7, 2023
b3c2c88
Merge branch 'main' into add-claim-rewards-ica
riley-stride Nov 7, 2023
393708b
batch claimReward ICAs
riley-stride Nov 30, 2023
8479b3f
address sam pr comments
riley-stride Nov 30, 2023
35e7498
refactor batching
riley-stride Dec 6, 2023
8b9393a
only submit claim msgs if any exist
riley-stride Dec 6, 2023
8326927
Merge branch 'main' into add-claim-rewards-ica
riley-stride Dec 6, 2023
3f7aacd
reference actual ClaimRewardsICABatchSize in test
riley-stride Dec 6, 2023
d308057
Merge branch 'add-claim-rewards-ica' of ssh://github.com/Stride-Labs/…
riley-stride Dec 6, 2023
1141cf7
Merge branch 'main' into add-claim-rewards-ica
riley-stride Dec 7, 2023
cff7d45
Merge branch 'main' into add-claim-rewards-ica
riley-stride Dec 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions x/stakeibc/keeper/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochInfo epochstypes.EpochInf
delegationInterval := k.GetParam(ctx, types.KeyDelegateInterval)
reinvestInterval := k.GetParam(ctx, types.KeyReinvestInterval)

// Claim accrued staking rewards at the beginning of the epoch
k.ClaimAccruedStakingRewards(ctx)

// Create a new deposit record for each host zone and the grab all deposit records
k.CreateDepositRecordsForEpoch(ctx, epochNumber)
depositRecords := k.RecordsKeeper.GetAllDepositRecord(ctx)
Expand Down Expand Up @@ -148,6 +151,18 @@ func (k Keeper) SetWithdrawalAddress(ctx sdk.Context) {
}
}

// Claim staking rewards for each host zone
func (k Keeper) ClaimAccruedStakingRewards(ctx sdk.Context) {
k.Logger(ctx).Info("Claiming Accrued Staking Rewards...")

for _, hostZone := range k.GetAllActiveHostZone(ctx) {
err := k.ClaimAccruedStakingRewardsOnHost(ctx, hostZone)
if err != nil {
k.Logger(ctx).Error(fmt.Sprintf("Unable to claim accrued staking rewards on %s, err: %s", hostZone.ChainId, err))
}
}
}

// Updates the redemption rate for each host zone
// At a high level, the redemption rate is equal to the amount of native tokens locked divided by the stTokens in existence.
// The equation is broken down further into the following sub-components:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package keeper_test

import (
"fmt"

"cosmossdk.io/math"
_ "github.com/stretchr/testify/suite"

epochtypes "github.com/Stride-Labs/stride/v16/x/epochs/types"
types "github.com/Stride-Labs/stride/v16/x/stakeibc/types"
)

// constant number of zero delegations
const numZeroDelegations = 37
const claimRewardsICABatchSize = 10
sampocs marked this conversation as resolved.
Show resolved Hide resolved

func (s *KeeperTestSuite) ClaimAccruedStakingRewardsOnHost() {
// Create a delegation ICA channel for the ICA submission
owner := types.FormatICAAccountOwner(HostChainId, types.ICAAccountType_DELEGATION)
channelId, portId := s.CreateICAChannel(owner)

// Create validators
validators := []*types.Validator{}
numberGTClaimRewardsBatchSize := int(50)
for i := 0; i < numberGTClaimRewardsBatchSize; i++ {

// set most delegations to 5, some to 0
valDelegation := math.NewInt(5)
if i > (numberGTClaimRewardsBatchSize - numZeroDelegations) {
valDelegation = math.NewInt(0)
}
validators = append(validators, &types.Validator{
Address: fmt.Sprintf("val-%d", i),
Delegation: valDelegation,
})
}

// Create host zone
hostZone := types.HostZone{
ChainId: HostChainId,
DelegationIcaAddress: "delegation",
WithdrawalIcaAddress: "withdrawal",
Validators: validators,
}

// Create epoch tracker for ICA timeout
strideEpoch := types.EpochTracker{
EpochIdentifier: epochtypes.STRIDE_EPOCH,
NextEpochStartTime: uint64(s.Coordinator.CurrentTime.UnixNano() + 30_000_000_000), // used for timeout
}
s.App.StakeibcKeeper.SetEpochTracker(s.Ctx, strideEpoch)

// Get start sequence number to confirm ICA was set
startSequence, found := s.App.IBCKeeper.ChannelKeeper.GetNextSequenceSend(s.Ctx, portId, channelId)
s.Require().True(found)

// Call claim accrued rewards to submit ICAs
err := s.App.StakeibcKeeper.ClaimAccruedStakingRewardsOnHost(s.Ctx, hostZone)
s.Require().NoError(err, "no error expected when accruing rewards")

// Confirm sequence number incremented by the number of txs
// where the number of txs is equal:
// (total_validators - validators_with_zero_delegation) / batch_size
batchSize := (numberGTClaimRewardsBatchSize - numZeroDelegations) / claimRewardsICABatchSize
expectedEndSequence := startSequence + uint64(batchSize)
actualEndSequence, found := s.App.IBCKeeper.ChannelKeeper.GetNextSequenceSend(s.Ctx, portId, channelId)
s.Require().True(found)
s.Require().Equal(expectedEndSequence, actualEndSequence, "sequence number should have incremented")

// Attempt to call it with a host zone without a delegation ICA address, it should fail
invalidHostZone := hostZone
invalidHostZone.DelegationIcaAddress = ""
err = s.App.StakeibcKeeper.ClaimAccruedStakingRewardsOnHost(s.Ctx, hostZone)
s.Require().ErrorContains(err, "ICA account not found")

// Attempt to call it with a host zone without a withdrawal ICA address, it should fail
invalidHostZone = hostZone
invalidHostZone.WithdrawalIcaAddress = ""
err = s.App.StakeibcKeeper.ClaimAccruedStakingRewardsOnHost(s.Ctx, hostZone)
s.Require().ErrorContains(err, "ICA account not found")

// Attempt to call claim with an invalid connection ID on the host zone so the ica fails
invalidHostZone = hostZone
invalidHostZone.ConnectionId = ""
err = s.App.StakeibcKeeper.ClaimAccruedStakingRewardsOnHost(s.Ctx, hostZone)
s.Require().ErrorContains(err, "Failed to SubmitTxs")
sampocs marked this conversation as resolved.
Show resolved Hide resolved
}
53 changes: 53 additions & 0 deletions x/stakeibc/keeper/msg_server_submit_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ import (
icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
)

const (
ClaimRewardsICABatchSize = 10
)

func (k Keeper) DelegateOnHost(ctx sdk.Context, hostZone types.HostZone, amt sdk.Coin, depositRecord recordstypes.DepositRecord) error {
// the relevant ICA is the delegate account
owner := types.FormatICAAccountOwner(hostZone.ChainId, types.ICAAccountType_DELEGATION)
Expand Down Expand Up @@ -153,6 +157,55 @@ func (k Keeper) SetWithdrawalAddressOnHost(ctx sdk.Context, hostZone types.HostZ
return nil
}

func (k Keeper) ClaimAccruedStakingRewardsOnHost(ctx sdk.Context, hostZone types.HostZone) error {
// Fetch the relevant ICA
if hostZone.DelegationIcaAddress == "" {
return errorsmod.Wrapf(types.ErrICAAccountNotFound, "delegation ICA not found for %s", hostZone.ChainId)
}
if hostZone.WithdrawalIcaAddress == "" {
return errorsmod.Wrapf(types.ErrICAAccountNotFound, "withdrawal ICA not found for %s", hostZone.ChainId)
}
k.Logger(ctx).Info(utils.LogWithHostZone(hostZone.ChainId, "Withdrawal Address: %s, Delegator Address: %s",
hostZone.WithdrawalIcaAddress, hostZone.DelegationIcaAddress))

validators := hostZone.Validators

// Build multi-message transaction to withdraw rewards from each validator
// batching txs into groups of ClaimRewardsICABatchSize messages, to ensure they will fit in the host's blockSize

// Calculate the number of batches
numBatches := (len(validators) + ClaimRewardsICABatchSize - 1) / ClaimRewardsICABatchSize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a dumb question, can we do numBatches := len(validators) / ClaimRewardsICABatchSize ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha yes good point, I think so. But I removed this now in favor of the approach Sam recommended above.


// Iterate over the batches
for batch := 0; batch < numBatches; batch++ {
start := batch * ClaimRewardsICABatchSize
end := (batch + 1) * ClaimRewardsICABatchSize
if end > len(validators) {
end = len(validators)
}
sampocs marked this conversation as resolved.
Show resolved Hide resolved
msgs := []proto.Message{}
// Iterate over the items within the batch
for _, val := range validators[start:end] {
// skip withdrawing rewards
if val.Delegation.IsZero() {
continue
}
msg := &distributiontypes.MsgWithdrawDelegatorReward{
DelegatorAddress: hostZone.DelegationIcaAddress,
ValidatorAddress: val.Address,
}
msgs = append(msgs, msg)
}

_, err := k.SubmitTxsStrideEpoch(ctx, hostZone.ConnectionId, msgs, types.ICAAccountType_DELEGATION, "", nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: because we skip assembling the msg if val.Delegation.IsZero, should we have a check here of if len(msgs) > 0 then submit the Txs, and otherwise just continue with the loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eye, added here.

if err != nil {
return errorsmod.Wrapf(err, "Failed to SubmitTxs for %s, %s, %s", hostZone.ConnectionId, hostZone.ChainId, msgs)
}
}

return nil
}

// Submits an ICQ for the withdrawal account balance
func (k Keeper) UpdateWithdrawalBalance(ctx sdk.Context, hostZone types.HostZone) error {
k.Logger(ctx).Info(utils.LogWithHostZone(hostZone.ChainId, "Submitting ICQ for withdrawal account balance"))
Expand Down
Loading