WIP: Refactor + Exit NFT #16
Conversation
jdkanani
commented
Aug 18, 2018
•
jdkanani
commented
- Use open-zeppelin for ERC contracts
- Break down RootChain contract into pieces for better readability
- Add NFT for the exit for plasma. Create NFT on exit and burn on actual withdraw or on the challenge
- Add burn withdraw for reusable account
- Exit without burn (this will make account un-usable)
- Re-use of the account once exited without the burn
- Only one exit is allowed per account
- Add deposit withdraw
- Add challenges for all types of exits
- Add test-cases for each case
|
@ashishrp @shogochiai @vaibhavchellani would love your review. It's WIP! |
contracts/lib/Common.sol
Outdated
| * @param a uint256 number | ||
| * @param b uint256 number | ||
| */ | ||
| function max(uint256 a, uint256 b) internal pure returns (uint256) { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Yeah I will add. Missed it.
There was a problem hiding this comment.
Added in new commit 9d65877
|
Added For more info, check trufflesuite/truffle#555 |
contracts/root/WithdrawManager.sol
Outdated
| ); | ||
|
|
||
| // withdraw | ||
| _withdraw( |
There was a problem hiding this comment.
- This
_withdraw()function is called by both burnProofWithdrawal-logic and exitWithdrawal-logic - This function appends withdrawal to priorityQueue finally
- burnProofWithdrawal must call
WETH.withdrawalorERC20.transferrather than_withdraw
Is this statement correct? @jdkanani
There was a problem hiding this comment.
No. burnProofWithdrawal will also call _withdraw() as operator can just create coins out of thin air, then burn it and create checkpoint without providing data - data unavailability issue.
So, if you burn then you can reuse your account. But if you don't burn and start exit without it, anyone can come and challenge if you reuse your account.
Reuse account == make transaction on Matic chain after exit
There was a problem hiding this comment.
as operator can just create coins out of thin air, then burn it and create checkpoint without providing data - data unavailability issue.
make sense :)
if you don't burn and start exit without it, anyone can come and challenge if you reuse your account.
Additionally anyone can cause state transition (e.g. balance addition) and the exit will be malicious.
So this exit function must be either "burnt exit" or "last resort"
| } | ||
|
|
||
| // withdraw tokens | ||
| function withdrawTokens( |
There was a problem hiding this comment.
This function was used in withdrawal-bridge, but the w-bridger is deprecated. So this function must be removed for the sake of readability.
There was a problem hiding this comment.
This function is to withdraw without burn (last resort). When user is done with Matic and Matic chain is no longer functional (extreme case)
We will update withdrawal-bridge once this pull request is done.
contracts/root/WithdrawManager.sol
Outdated
| // If an exit was successfully challenged, owner would be address(0). | ||
| address exitOwner = ExitNFT(exitNFTContract).ownerOf(utxoPos); | ||
| if (exitOwner != address(0)) { | ||
| if (_token == wethToken) { |
There was a problem hiding this comment.
This condition scope had better to be separated function because possibly called from withdrawal() function.
There was a problem hiding this comment.
I didn't get what you saying.
There was a problem hiding this comment.
never mind, question solved by other answer.
contracts/root/WithdrawManager.sol
Outdated
| @@ -147,7 +147,7 @@ contract WithdrawManager is DepositManager { | |||
| } | |||
|
|
|||
| // withdraw tokens | |||
| function withdraw( | |||
| function withdrawBurntTokens( | |||
There was a problem hiding this comment.
Q. How long does it take until child-chain burnProof confirmed?
Asm. ~=3600sec, until committed to rootchain @jdkanani
|
@syuhei176 can you take a look at the structure? I will create a diagram to explain in more details. |
|
This pull request is getting really getting big. We will merge this and keep working on other issues. |
|
yes makes sense |
Add Exit NFT for faster exit and refactor
