Add regression test on malicious stETH recovery by recoverERC721 (selfOwnedStETHBurner)
#444
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
Feat: merge-ready protocol
Add a test to address the issue #443 regarding missing explicit check on the posibility to recover stETH through the `recoverERC721` func.
folkyatina
reviewed
Nov 23, 2022
Comment on lines
809
to
826
| assertBn(await lido.balanceOf(anotherAccount), stETH(0)) | ||
| // submit 10 ETH to mint 10 stETH | ||
| await web3.eth.sendTransaction({ from: anotherAccount, to: lido.address, value: ETH(10) }) | ||
| // check 10 stETH minted on balance | ||
| assertBn(await lido.balanceOf(anotherAccount), stETH(10)) | ||
| // transfer 5 stETH to the burner account | ||
| await lido.transfer(burner.address, stETH(5), { from: anotherAccount }) | ||
| // transfer 5 stETH to voting | ||
| await lido.transfer(voting, stETH(5), { from: anotherAccount }) | ||
|
|
||
| // request 5 stETH to be burned later | ||
| await lido.approve(burner.address, stETH(5), { from: voting }) | ||
| await burner.requestBurnMyStETH(stETH(5), { from: voting }) | ||
|
|
||
| // check balances one last time | ||
| assertBn(await lido.balanceOf(anotherAccount), stETH(0)) | ||
| assertBn(await lido.balanceOf(voting), stETH(0)) | ||
| assertBn(await lido.balanceOf(burner.address), stETH(10)) |
There was a problem hiding this comment.
It's not clear for me, why we need to put 10 stETH and trying to recover only 1 and why a such complicated way was used to top up the burner.
There was a problem hiding this comment.
Hey, addressed the issues within the updated code, thank you 🙏
The top-up process is rather complicated because of design requirements (only voting can request the burning & explicit allowance required).
Please check out the updated code 👍
folkyatina
approved these changes
Dec 1, 2022
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.
Add a test to address issue #443 regarding missing explicit check
on the possibility to recover stETH through the
recoverERC721func.