feat: claim rewards #48
Conversation
h3lio5
requested review from
d-roak,
elielnfinic and
hoangquocvietuet
as code owners
| @@ -25,6 +25,8 @@ service Msg { | |||
| rpc JoinDeal (MsgJoinDeal ) returns (MsgJoinDealResponse ); | |||
| rpc LeaveDeal (MsgLeaveDeal ) returns (MsgLeaveDealResponse ); | |||
| rpc SubmitProgress (MsgSubmitProgress ) returns (MsgSubmitProgressResponse ); | |||
| rpc claimRewards (MsgClaimRewards ) returns (MsgClaimRewardsResponse ); | |||
| rpc withdrawResidue (MsgWithdrawResidue ) returns (MsgWithdrawResidueResponse ); | |||
There was a problem hiding this comment.
claimRewards: providers claiming rewards
withdrawResidue: deal creator withdrawing any residue (a bit of context: rewards split evenly between epochs, if no one claims rewards before deal_expiry_claim_window (7 days but we can change it), the creator can withdraw it)
There was a problem hiding this comment.
I think we disapproved this, provider should be able to withdraw whenever they want.
Co-authored-by: Oak <[email protected]>
…ain-node into settle-rewards
| @@ -25,6 +25,8 @@ service Msg { | |||
| rpc JoinDeal (MsgJoinDeal ) returns (MsgJoinDealResponse ); | |||
| rpc LeaveDeal (MsgLeaveDeal ) returns (MsgLeaveDealResponse ); | |||
| rpc SubmitProgress (MsgSubmitProgress ) returns (MsgSubmitProgressResponse ); | |||
| rpc claimRewards (MsgClaimRewards ) returns (MsgClaimRewardsResponse ); | |||
| rpc withdrawResidue (MsgWithdrawResidue ) returns (MsgWithdrawResidueResponse ); | |||
There was a problem hiding this comment.
I think we disapproved this, provider should be able to withdraw whenever they want.
x/subscription/keeper/msg_deal.go
Outdated
| if int64(msg.EndBlock) < ctx.BlockHeight() { | ||
| return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "end block must be greater than current block height") | ||
| if msg.NumEpochs != 0 { | ||
| if deal.StartBlock+msg.NumEpochs*deal.EpochSize < currentBlock { |
There was a problem hiding this comment.
Let's discuss this : wouldn't it be better to append the epochs to the existing epochs : so for me It could be something like deal.EndBlock + msg.NumEpochs * deal.EpochSize to have the new EndBlock
There was a problem hiding this comment.
this is to prevent the deal duration from shrinking in size such that the new endblock is lower than current block, i.e., you can't set the end block to an expired block haha.
There was a problem hiding this comment.
to clarify, we are not storing the end block anywhere. The above check is just to prevent shrinking the deal duration such that it is expired (i.e., new end_block < current_block)
| @@ -29,15 +31,18 @@ func (k msgServer) SubmitProgress(goCtx context.Context, msg *types.MsgSubmitPro | |||
| return nil, errorsmod.Wrap(sdkerrors.ErrUnauthorized, "only the provider can submit progress") | |||
| } | |||
|
|
|||
| deal, found := k.GetDeal(ctx, subscription.DealId) | |||
| currentEpoch := (currentBlock - deal.StartBlock + 1) / deal.EpochSize | |||
There was a problem hiding this comment.
Should we have a utility function to compute the current epoch?
| StartBlock: subscriptionStartBlock, | ||
| EndBlock: deal.EndBlock, | ||
| StartEpoch: subscriptionStartEpoch, | ||
| EndEpoch: (deal.EpochSize*deal.NumEpochs + 1) / deal.EpochSize, |
There was a problem hiding this comment.
Make it easier to calculate: EndEpoch = subscriptionStartEpoch + numEpochs - 1.
| deal.Status = types.Deal_ACTIVE | ||
| } else { | ||
| subscriptionStartBlock = deal.StartBlock | ||
| // start from the first epoch | ||
| subscriptionStartEpoch = 0 |
There was a problem hiding this comment.
If subscriptionStartEpoch has an index of 1, it should have a value of 1 here.
| // 7 days equivalent epochs | ||
| const DEAL_EXPIRY_CLAIM_WINDOW = 144 | ||
|
|
||
| func ConvertBlockToEpoch(blockOffset, epochSize uint64) uint64 { |
There was a problem hiding this comment.
We need to correct the formula in ConvertBlockToEpoch and use it globally!
| progressSize := uint64(len(hashesSet)) | ||
| progressDeal = getUpdatedProgressDeal(progressDeal, provider, progressSize, deal.RewardPerEpoch, currentEpoch) | ||
| k.SetProgressDealAtEpoch(ctx, subscription.DealId, currentEpoch, progressDeal) | ||
| k.AddProgressEpochsProvider(ctx, provider, subscriptionId, currentEpoch) | ||
| return &types.MsgSubmitProgressResponse{}, nil | ||
| } | ||
|
|
||
| initialProgressSize := len(progress) |
There was a problem hiding this comment.
Shouldn't we move the line initialProgressSize := len(progress) right before the !found if-else case?
Changes:
fixes #40