[Draft] Vaults UX suggestions #893
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DiRaiks
commented
Dec 6, 2024
Jeday
reviewed
Dec 6, 2024
folkyatina
reviewed
Dec 6, 2024
folkyatina
reviewed
Dec 6, 2024
folkyatina
reviewed
Dec 6, 2024
folkyatina
reviewed
Dec 6, 2024
| @@ -159,6 +233,15 @@ contract Dashboard is AccessControlEnumerable { | |||
| _withdraw(_recipient, _ether); | |||
| } | |||
|
|
|||
| /** | |||
| * @notice Withdraws stETH tokens from the staking vault to wrapped ether. Approvals for the passed amounts should be done before. | |||
There was a problem hiding this comment.
Sorry, didn't get what it should do. RN, ether is still on the dashboard contract. Should it be transferred to msg.sender or some other recepient?
There was a problem hiding this comment.
@folkyatina The idea is to send to the fundByWeth method the amount of WETH that needs to be "unwrap" to the ETH and then fund Vault
WETH withdraw method https://github.com/gnosis/canonical-weth/blob/master/contracts/WETH9.sol#L38
Now I understand that it seems that this code will not work correctly
Co-authored-by: Aleksei Potapkin <[email protected]>
Co-authored-by: Aleksei Potapkin <[email protected]>
… into feat/vaults-suggestions
Jeday
requested changes
Dec 10, 2024
tamtamchik
added
solidity
DiRaiks
changed the base branch from
feat/vaults
to
feat-immutable-operator-in-vault
|
Hardhat Unit Tests Coverage SummaryDiff against masterResults for commit: 3d5fbb8 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…estions # Conflicts: # package.json # test/0.8.25/vaults/dashboard/dashboard.test.ts # yarn.lock
Comment on lines
305
to
314
| function mintWstETH( | ||
| address _recipient, | ||
| uint256 _tokens | ||
| ) external payable virtual onlyRole(DEFAULT_ADMIN_ROLE) fundAndProceed { | ||
| _mint(address(this), _tokens); | ||
|
|
||
| stETH.approve(address(wstETH), _tokens); | ||
| uint256 wstETHAmount = wstETH.wrap(_tokens); | ||
| wstETH.transfer(_recipient, wstETHAmount); | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
…feat/steth-permit
fix: StETH harness contract for dashboard tests
…estions # Conflicts: # test/0.8.25/vaults/dashboard/dashboard.test.ts
… into feat/vaults-suggestions
chore: extract some vaults helpers to library
…ance/core into feat/vaults-suggestions
…estions # Conflicts: # contracts/0.8.25/vaults/StakingVault.sol
tamtamchik
approved these changes
Dec 20, 2024
| * @notice Funds the staking vault with wrapped ether. Approvals for the passed amounts should be done before. | ||
| * @param _wethAmount Amount of wrapped ether to fund the staking vault with | ||
| */ | ||
| function fundByWeth(uint256 _wethAmount) external virtual onlyRole(DEFAULT_ADMIN_ROLE) { |
There was a problem hiding this comment.
Suggested change
| function fundByWeth(uint256 _wethAmount) external virtual onlyRole(DEFAULT_ADMIN_ROLE) { | |
| function fundWeth(uint256 _wethAmount) external virtual onlyRole(DEFAULT_ADMIN_ROLE) { |
| * @param _recipient Address of the recipient | ||
| * @param _ether Amount of ether to withdraw | ||
| */ | ||
| function withdrawToWeth(address _recipient, uint256 _ether) external virtual onlyRole(DEFAULT_ADMIN_ROLE) { |
There was a problem hiding this comment.
Suggested change
| function withdrawToWeth(address _recipient, uint256 _ether) external virtual onlyRole(DEFAULT_ADMIN_ROLE) { | |
| function withdrawWeth(address _recipient, uint256 _ether) external virtual onlyRole(DEFAULT_ADMIN_ROLE) { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A short summary of the changes.
Context
Problem
What problem this PR solves, link relevant issue if it exists
Solution
Your proposed solution