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

Direct conversion from staking contract to delegation pool #12256

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

alexfilip2
Copy link
Contributor

Description

This implements AIP: aptos-foundation/AIPs#311
The staker of a staking contract would be able to atomically convert it to a delegation pool of same operator and commission fee without incurring any validator downtime. The staker becomes the owner of the newly created delegation pool.

Test Plan

Module UTs validating core functionality and invariants.

Copy link

trunk-io bot commented Feb 27, 2024

⏱️ 21s total CI duration on this PR

Job Cumulative Duration Recent Runs
permission-check 6s 🟥🟥
permission-check 6s 🟥🟥
permission-check 5s 🟥🟥
permission-check 4s 🟥🟥

settingsfeedbackdocs ⋅ learn more about trunk.io

@junkil-park junkil-park self-requested a review March 6, 2024 01:38
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.1%. Comparing base (b703936) to head (aed470f).
Report is 828 commits behind head on main.

Current head aed470f differs from pull request most recent head 354b18f

Please upload reports for the commit 354b18f to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12256      +/-   ##
==========================================
- Coverage    71.1%    64.1%    -7.1%     
==========================================
  Files         795      792       -3     
  Lines      183022   176059    -6963     
==========================================
- Hits       130291   113005   -17286     
- Misses      52731    63054   +10323     

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

Copy link
Contributor

github-actions bot commented May 9, 2024

This issue is stale because it has been open 45 days with no activity. Remove the stale label, comment or push a commit - otherwise this will be closed in 15 days.

@github-actions github-actions bot added the Stale label May 9, 2024
@github-actions github-actions bot closed this May 24, 2024
@junkil-park junkil-park reopened this Jun 21, 2024
@github-actions github-actions bot removed the Stale label Jun 22, 2024
Copy link
Contributor

github-actions bot commented Aug 6, 2024

This issue is stale because it has been open 45 days with no activity. Remove the stale label, comment or push a commit - otherwise this will be closed in 15 days.

@github-actions github-actions bot added the Stale label Aug 6, 2024
@github-actions github-actions bot closed this Aug 21, 2024
@junkil-park junkil-park reopened this Oct 14, 2024
@github-actions github-actions bot removed the Stale label Oct 15, 2024
@georgemitenkov georgemitenkov removed their request for review November 7, 2024 10:47
Copy link
Contributor

@movekevin movekevin left a comment

Choose a reason for hiding this comment

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

Thank you for adding this conversion! I left a couple of comments and questions

inactive_shares,
pending_withdrawals: table::new<address, ObservedLockupCycle>(),
stake_pool_signer_cap,
// `staking_contract::distribute_internal` has already withdrawn any existing inactive stake
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should just do an assert that inactive == 0 just to be absolutely certain

Copy link
Contributor Author

@alexfilip2 alexfilip2 Nov 13, 2024

Choose a reason for hiding this comment

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

should we use the error::internal code as this is an invariant that's not expected to fail? we may also formally prove it

@alexfilip2 alexfilip2 force-pushed the convert-staking-contract-to-delegation-pool branch from 54337fe to 47e2d07 Compare November 13, 2024 14:02
@junkil-park junkil-park added the move-framework Issues related to the Framework modules/libraries label Nov 15, 2024
alexfilip2 and others added 3 commits December 13, 2024 22:25
- added a mockup spec to unblock the CI test failure
@junkil-park junkil-park added move-framework Issues related to the Framework modules/libraries and removed move-framework Issues related to the Framework modules/libraries labels Dec 18, 2024
Copy link
Contributor

@junkil-park junkil-park left a comment

Choose a reason for hiding this comment

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

After completing the localnet test, we just need to add the forge test here, and I think we’re good to go.

Copy link
Contributor

This issue is stale because it has been open 45 days with no activity. Remove the stale label, comment or push a commit - otherwise this will be closed in 15 days.

@github-actions github-actions bot added the Stale label Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
move-framework Issues related to the Framework modules/libraries Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants