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: PIP-42 change token to POL #4

Merged
merged 96 commits into from
Aug 22, 2024
Merged

feat: PIP-42 change token to POL #4

merged 96 commits into from
Aug 22, 2024

Conversation

ZeroEkkusu
Copy link
Member

@ZeroEkkusu ZeroEkkusu commented Apr 10, 2024

Motivation

Transition StakeManager to POL.
See PIP-42 discussion

Solution

Use POL by default in StakeManager.

Add legacy support for MATIC <> POL conversion in StakeManager and ValidatorShare.

Todo

  • Set MATIC and POL tokens on reinitialization of StakeManager.
  • Governance: Convert all MATIC in StakeManager to POL.
  • Upgrade DefaultEmissionManager in pol-token repo to not unmigrate rewards. See pol-token/pull/60

Copy link

sonarcloud bot commented Apr 11, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@simonDos simonDos self-requested a review May 2, 2024 15:46
Copy link

@dev1644 dev1644 left a comment

Choose a reason for hiding this comment

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

The successful conversion from MATIC to POL is assured, whereas the reverse may not be as reliable. This discrepancy warrants further discussion. Presently, there is a trend of conversions from POL to MATIC occurring in the following operational contexts:

The _transferToken function includes an option to convert POL to MATIC, through following paths -

  • Drain actions performed by Governance
  • unstakeClaim
  • _liquidateRewards function, further segmented into:
    • withdrawRewards function
    • _unstake operations, encompassing:
      • forceUnstakeLegacy function
      • unstakeLegacy function
      • dethroneAndStakeLegacy function

contracts/staking/stakeManager/StakeManager.sol Outdated Show resolved Hide resolved
contracts/staking/stakeManager/StakeManager.sol Outdated Show resolved Hide resolved
uint256 heimdallFee /** for new validator */
) external onlyWhenUnlocked {
function confirmAuctionBid(uint256 validatorId, uint256 heimdallFee)
/**
Copy link

Choose a reason for hiding this comment

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

This comment seems out of place.

contracts/staking/stakeManager/StakeManager.sol Outdated Show resolved Hide resolved
contracts/staking/stakeManager/StakeManager.sol Outdated Show resolved Hide resolved
contracts/staking/stakeManager/StakeManager.sol Outdated Show resolved Hide resolved
contracts/staking/stakeManager/StakeManager.sol Outdated Show resolved Hide resolved
contracts/staking/stakeManager/StakeManager.sol Outdated Show resolved Hide resolved
contracts/staking/stakeManager/StakeManager.sol Outdated Show resolved Hide resolved

function _delegationDeposit(uint256 amount, address delegator, bool pol) internal returns (bool) {
IERC20 token_ = _getToken(pol);
bool result = token_.transferFrom(delegator, address(this), amount);
Copy link

Choose a reason for hiding this comment

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

Can refactor this -

_transferTokenFrom(delegator, address(this), amount, pol)

@ethyla ethyla changed the title feat: change token to POL feat: PIP-42 change token to POL Aug 1, 2024
Copy link

sonarcloud bot commented Aug 22, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
36.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@simonDos simonDos merged commit 4857f3f into dev Aug 22, 2024
1 of 2 checks passed
@simonDos simonDos deleted the feat/pol branch October 14, 2024 15:14
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.

5 participants