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

feat(redelegate stake): init gar implementaion PE-7058 #192

Open
wants to merge 30 commits into
base: develop
Choose a base branch
from

Conversation

fedellen
Copy link
Contributor

@fedellen fedellen commented Nov 14, 2024

TODO:

  • redelegate from vaults (do we put into a vault, or as a delegation)
  • add pruning of redelegates when seven epochs reached
  • finish unit tests
  • init integration tests
    • extend integration tests
  • getReDelegationFee handler
  • Handle event API -- caclucate delegated stake
  • better PR description
  • more maybe

src/main.lua Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 97.53086% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.35%. Comparing base (10f880d) to head (dafe932).

Files with missing lines Patch % Lines
src/gar.lua 97.48% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #192      +/-   ##
===========================================
+ Coverage    92.02%   92.35%   +0.32%     
===========================================
  Files           11       11              
  Lines         2609     2746     +137     
===========================================
+ Hits          2401     2536     +135     
- Misses         208      210       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fedellen fedellen marked this pull request as ready for review November 15, 2024 21:48
src/main.lua Outdated Show resolved Hide resolved
local redelegationFeeRate = gar.getRedelegationFee(delegateAddress).redelegationFeeRate

local redelegationFee = math.ceil(stakeToTakeFromSource * (redelegationFeeRate / 100))
local stakeToDelegate = stakeToTakeFromSource - redelegationFee
Copy link
Contributor

Choose a reason for hiding this comment

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

So the redelegator chooses an amount of stake they are willing to participate in the move, and they basically get that amount minus any fees redelegated?

The other approach would be to move the amount they want to move and assess fees on TOP of that. Just want to make sure we're aligned with the intention from the white paper.

local existingStake = existingDelegate.delegatedStake
local requiredMinimumStake = sourceGateway.settings.minDelegatedStake
local maxAllowedToWithdraw = existingStake - requiredMinimumStake
assert(
Copy link
Contributor

Choose a reason for hiding this comment

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

It's interesting that we have this barrier here since the user could always commence a withdraw of their stake and then begin a redelegation. We grapple with this a bit with funding plans where, since we acknowledge this potential user workaround, we err on the side of a good UX for the user and simply "break a 20" on their minimum stake if necessary. We'll use it all if we have to, or we'll leave behind the excess in a withdraw vault. I'm wondering why we wouldn't just do the same here (we can check with Jon).

src/gar.lua Outdated
)

-- If the delegate has enough stake to redelegate, move the stake. If its all the stake, remove the delegate
if existingDelegate.delegatedStake == stakeToTakeFromSource then
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only appropriate if the delegate has no withdraw vaults. We'll have to fix this.

local minimumStakeForGatewayAndDelegate
if existingTargetDelegate and existingTargetDelegate.delegatedStake ~= 0 then
-- It already has a stake that is not zero
minimumStakeForGatewayAndDelegate = 1 -- Delegate must provide at least one additional IO
Copy link
Contributor

@arielmelendez arielmelendez Nov 16, 2024

Choose a reason for hiding this comment

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

Is this supposed to be 1 IO or 1 mIO? The unit is mIO but the comment says IO.

src/gar.lua Outdated
redelegations = (previousRedelegations and previousRedelegations.redelegations or 0) + 1,
}

-- prune user from allow list, if necessary, to save memory
Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite how this works. User goes back on the allowlist on the source gateway IF they've been pruned, otherwise their active balance or vaults "holds their spot" on the gateway. On the destination gateway, we take them out of the allowlist lookup if we create a new delegation for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants