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: add script to upgrade Pool Voter #47

Merged
merged 8 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
2 changes: 1 addition & 1 deletion contracts/interfaces/IOmnichainStaking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ interface IOmnichainStaking is IVotes {
// An omni-chain staking contract that allows users to stake their veNFT
// and get some voting power. Once staked the voting power is available cross-chain.

function unstakeToken(uint256 tokenId) external;
function unstakeAndWithdraw(uint256 tokenId) external;

// function totalSupply() external view returns (uint256);

Expand Down
3 changes: 2 additions & 1 deletion contracts/locker/BaseLocker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ contract BaseLocker is
/// @dev Only possible if the lock has expired
function withdraw(uint256 _tokenId) public virtual nonReentrant {
require(
_isAuthorized(ownerOf(_tokenId), msg.sender, _tokenId),
_isAuthorized(ownerOf(_tokenId), msg.sender, _tokenId) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't have to modify the lockers now. It'll be a pain to upgrade the lockers as well.

It'll be a lot better to withdraw the tokens within the staking contract itself and send it over to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved here: 866d93e

address(staking) == msg.sender,
"caller is not owner nor approved"
);

Expand Down
50 changes: 30 additions & 20 deletions contracts/locker/staking/OmnichainStakingBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -165,27 +165,13 @@ abstract contract OmnichainStakingBase is
}

/**
* @dev Unstakes a regular token NFT and transfers it back to the user.
* @param tokenId The ID of the regular token NFT to unstake.
* @notice A single withdraw function to unstake NFT, transfer it back to
* the user, and also withdraw all tokens for `tokenId` from the locker.
* @param tokenId The ID of the regular token NFT to unstake and withdraw.
*/
function unstakeToken(uint256 tokenId) external updateReward(msg.sender) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we can keep both functions it'll be great. unstakeToken and unstakeAndWithdraw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed here: 87ca53c

require(lockedByToken[tokenId] != address(0), "!tokenId");
address lockedBy_ = lockedByToken[tokenId];
if (_msgSender() != lockedBy_)
revert InvalidUnstaker(_msgSender(), lockedBy_);

delete lockedByToken[tokenId];
lockedTokenIdNfts[_msgSender()] = deleteAnElement(
lockedTokenIdNfts[_msgSender()],
tokenId
);

// reset and burn voting power
_burn(msg.sender, tokenPower[tokenId]);
tokenPower[tokenId] = 0;
votingPowerCombined.reset(msg.sender);

locker.safeTransferFrom(address(this), msg.sender, tokenId);
function unstakeAndWithdraw(uint256 tokenId) external {
Copy link
Contributor

Choose a reason for hiding this comment

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

we need a nonRentrant hook just for safety

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed here: 87ca53c

_unstakeToken(tokenId);
locker.withdraw(tokenId);
}

/**
Expand Down Expand Up @@ -408,6 +394,30 @@ abstract contract OmnichainStakingBase is
}
}

/**
* @dev Unstakes a regular token NFT and transfers it back to the user.
* @param tokenId The ID of the regular token NFT to unstake.
*/
function _unstakeToken(uint256 tokenId) internal updateReward(msg.sender) {
require(lockedByToken[tokenId] != address(0), "!tokenId");
address lockedBy_ = lockedByToken[tokenId];
if (_msgSender() != lockedBy_)
revert InvalidUnstaker(_msgSender(), lockedBy_);

delete lockedByToken[tokenId];
lockedTokenIdNfts[_msgSender()] = deleteAnElement(
lockedTokenIdNfts[_msgSender()],
tokenId
);

// reset and burn voting power
_burn(msg.sender, tokenPower[tokenId]);
Copy link
Contributor

Choose a reason for hiding this comment

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

checks and balance. move this after the line 415

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@deadshotryker could you please explain this, IMO this shouldn't be after line 415.

tokenPower[tokenId] = 0;
votingPowerCombined.reset(msg.sender);

locker.safeTransferFrom(address(this), msg.sender, tokenId);
Copy link
Contributor

Choose a reason for hiding this comment

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

here if we sent the nft to the contract itself, we can withdraw the tokens and send it back to the user in one transaction itself without having to give special approvals.

consider adding a to arguement to the unstakeToken fn.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed here: 87ca53c

}

/**
* @dev Deletes an element from an array.
* @param elements The array to delete from.
Expand Down
3 changes: 2 additions & 1 deletion contracts/voter/PoolVoter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ contract PoolVoter is
* @param _staking The address of the staking token (VE token).
* @param _reward The address of the reward token.
*/
function init(address _staking, address _reward) external reinitializer(1) {
function init(address _staking, address _reward, address _votingPowerCombined) external reinitializer(1) {
staking = IVotes(_staking);
reward = IERC20(_reward);
__ReentrancyGuard_init();
__Ownable_init(msg.sender);
votingPowerCombined = _votingPowerCombined;
}

/**
Expand Down
28 changes: 28 additions & 0 deletions scripts/upgrade-poolVoter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {ethers, upgrades} from "hardhat";

const ZERO_TOKEN_ADDRESS = "0x78354f8DcCB269a615A7e0a24f9B0718FDC3C7A7";
const OMNICHAIN_STAKING_ADDRESS = "0xf374229a18ff691406f99CCBD93e8a3f16B68888";

async function main() {

const poolVoterProxy = "0x5346e9ab27D7874Db95993667D1Cb8338913f0aF"
const poolVoter = await ethers.getContractFactory("PoolVoter");

console.log("Upgradig to new PoolVoter implementation");

if (ZERO_TOKEN_ADDRESS.length && OMNICHAIN_STAKING_ADDRESS.length) {

const upgradedPoolVoter = await upgrades.upgradeProxy(poolVoterProxy, poolVoter);
const tx = await upgradedPoolVoter.init(OMNICHAIN_STAKING_ADDRESS, ZERO_TOKEN_ADDRESS);
await tx.wait();

console.log("PoolVoter upgraded to", upgradedPoolVoter.address);
} else {
throw new Error("Invalid init arguments");
}
}

main().catch((error) => {
console.error(error);
process.exitCode = 1;
});
54 changes: 54 additions & 0 deletions test/fork/VotingPowerCombined/linea.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { expect, should } from "chai";
import { e18, initMainnetUser } from "../../fixtures/utils";
import { VestedZeroNFT, VotingPowerCombined } from "../../../typechain-types";
import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers";
import { time } from "@nomicfoundation/hardhat-network-helpers";
import { setForkBlock } from "../utils";
import { getNetworkDetails } from "../constants";
import {
Contract,
ContractTransactionResponse,
parseEther,
parseUnits,
} from "ethers";
import { ethers } from "hardhat";
import { getGovernanceContracts } from "../helper";

const FORK = process.env.FORK === "true";
const FORKED_NETWORK = process.env.FORKED_NETWORK ?? "";
const BLOCK_NUMBER = 5969983;

const STAKING_ADDRESS = "0xf374229a18ff691406f99CCBD93e8a3f16B68888";
const REWARD_ADDRESS = "0x78354f8DcCB269a615A7e0a24f9B0718FDC3C7A7";
const OWNER = "0x0F6e98A756A40dD050dC78959f45559F98d3289d";

if (FORK) {
let votingPowerCombined: VotingPowerCombined;
let deployerForked: SignerWithAddress;
describe.only("VotingPowerCombined ForkTests", async () => {
beforeEach(async () => {
votingPowerCombined = await ethers.getContractAt(
"VotingPowerCombined",
"0x2666951A62d82860E8e1385581E2FB7669097647"
);
[deployerForked] = await ethers.getSigners();
await setForkBlock(BLOCK_NUMBER);

//

const poolVoterFactory = await ethers.getContractFactory("PoolVoter");
const poolVoter = await poolVoterFactory.deploy();
await poolVoter.init(STAKING_ADDRESS, REWARD_ADDRESS, votingPowerCombined.target);

const owner = await initMainnetUser(OWNER);
await votingPowerCombined.connect(owner).setAddresses(STAKING_ADDRESS, REWARD_ADDRESS, poolVoter.target);
});

it("Should reset voting power", async () => {
const resetterWallet = await initMainnetUser("0x7Ff4e6A2b7B43cEAB1fC07B0CBa00f834846ADEd", parseEther('100'));

const resetTransaction = votingPowerCombined.connect(resetterWallet).reset(resetterWallet.address);
await expect(resetTransaction).to.not.be.revertedWith('Invalid reset performed');
});
});
}
Loading