feat: add script to upgrade Pool Voter #47
Conversation
| */ | ||
| function unstakeToken(uint256 tokenId) external updateReward(msg.sender) { |
There was a problem hiding this comment.
If we can keep both functions it'll be great. unstakeToken and unstakeAndWithdraw
There was a problem hiding this comment.
Addressed here: 87ca53c
contracts/locker/BaseLocker.sol
Outdated
| @@ -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) || | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Resolved here: 866d93e
| votingPowerCombined.reset(msg.sender); | ||
|
|
||
| locker.safeTransferFrom(address(this), msg.sender, tokenId); | ||
| function unstakeAndWithdraw(uint256 tokenId) external { |
There was a problem hiding this comment.
we need a nonRentrant hook just for safety
There was a problem hiding this comment.
Addressed here: 87ca53c
| tokenPower[tokenId] = 0; | ||
| votingPowerCombined.reset(msg.sender); | ||
|
|
||
| locker.safeTransferFrom(address(this), msg.sender, tokenId); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Addressed here: 87ca53c
| ); | ||
|
|
||
| // reset and burn voting power | ||
| _burn(msg.sender, tokenPower[tokenId]); |
There was a problem hiding this comment.
checks and balance. move this after the line 415
There was a problem hiding this comment.
@deadshotryker could you please explain this, IMO this shouldn't be after line 415.
No description provided.