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

Implement MoreVP exits and Predicates #78

Merged
merged 29 commits into from Jun 11, 2019
Merged

Implement MoreVP exits and Predicates #78

merged 29 commits into from Jun 11, 2019

Conversation

@atvanguard
Copy link
Member

atvanguard commented May 30, 2019

Specification: https://ethresear.ch/t/account-based-plasma-morevp/5480

Tests added:

Contract: ERC20Predicate
    startExitWithBurntTokens
      ✓ Valid exit with burnt tokens (6748ms, 565592 gas)
    startExit
      - reference: deposit - exitTx: fullBurn
      ✓ reference: incomingTransfer - exitTx: fullBurn (11391ms, 667491 gas)
      ✓ reference: outgoingTransfer - exitTx: fullBurn (11317ms, 667036 gas)
      ✓ reference: deposit - exitTx: outgoingTransfer (8052ms, 689434 gas)
      ✓ reference: counterparty balance (Deposit) - exitTx: incomingTransfer (6802ms, 688879 gas)
      ✓ reference: counterparty balance (Transfer) - exitTx: incomingTransfer (9848ms, 670641 gas)
      ✓ reference: own balance (Deposit) and counterparty balance (Deposit) - exitTx: incomingTransfer (10258ms, 1057193 gas)
      ✓ reference: own balance (outgoingTransfer) and counterparty balance (incomingTransfer) - exitTx: incomingTransfer (13785ms, 835961 gas)
    verifyDeprecation
      ✓ reference: Deposit - challenge: spend - exit: Burn (11892ms, 1118596 gas)
      ✓ should not be able to challenge with the in-flight tx from which the exit was started (8458ms, 1031373 gas)

  Contract: ERC721Predicate
    startExitWithBurntTokens
      ✓ Valid exit with burnt tokens (6613ms, 566558 gas)
    startExit
      ✓ reference: incomingTransfer - exitTx: burn (10509ms, 636681 gas)
      ✓ reference: Deposit - exitTx: burn (8116ms, 656126 gas)
      ✓ reference: counterparty balance (Transfer) - exitTx: incomingTransfer (9834ms, 641884 gas)
    verifyDeprecation
      - write test
@atvanguard atvanguard requested review from 0xAshish and jdkanani May 31, 2019
@atvanguard atvanguard changed the title Implement MoreVP exits and Predicates Implement MoreVP exits and Predicates :sparkles: May 31, 2019
contracts/root/predicates/ERC721Predicate.sol Outdated Show resolved Hide resolved
contracts/root/predicates/ERC20Predicate.sol Outdated Show resolved Hide resolved
contracts/common/Registry.sol Outdated Show resolved Hide resolved
contracts/common/Registry.sol Outdated Show resolved Hide resolved
contracts/common/Registry.sol Outdated Show resolved Hide resolved
contracts/common/Registry.sol Outdated Show resolved Hide resolved
@0xAshish

This comment has been minimized.

Copy link
Member

0xAshish commented Jun 3, 2019

@atvanguard whole Predicates thing can be converted into Lib since these don't have any storage of own.

Copy link
Member

0xAshish left a comment

it won't let me request changes without any comment so here is comment. 😅

@atvanguard atvanguard changed the title Implement MoreVP exits and Predicates :sparkles: Implement MoreVP exits and Predicates Jun 4, 2019
@atvanguard

This comment has been minimized.

Copy link
Member Author

atvanguard commented Jun 4, 2019

@atvanguard whole Predicates thing can be converted into Lib since these don't have any storage of own.

@0xAshish Predicates require these in the storage

uint256 constant internal MAX_LOGS = 10;
IWithdrawManager internal withdrawManager;
@jdkanani jdkanani self-requested a review Jun 4, 2019
@atvanguard

This comment has been minimized.

Copy link
Member Author

atvanguard commented Jun 4, 2019

Can we have gas estimation for ERC20/721?

Gas requirements:

  • Exiting with burnt tokens (happy case) requires ~ 400k gas.
  • MoreVP exits require gas in the range of ~ 470k - 700k
Contract: ERC20Predicate
    startExitWithBurntTokens
startExitWithBurntTokens (gas used):  393338
      ✓ Valid exit with burnt tokens (6423ms, 565892 gas)
    startExit
      - reference: deposit - exitTx: fullBurn
startExit (gas used):  482077
      ✓ reference: incomingTransfer - exitTx: fullBurn (9508ms, 670339 gas)
startExit (gas used):  481538
      ✓ reference: outgoingTransfer - exitTx: fullBurn (10913ms, 669736 gas)
startExit (gas used):  504085
      ✓ reference: deposit - exitTx: outgoingTransfer (6614ms, 692199 gas)
startExit (gas used):  503446
      ✓ reference: counterparty balance (Deposit) - exitTx: incomingTransfer (7498ms, 691644 gas)
startExit (gas used):  485227
      ✓ reference: counterparty balance (Transfer) - exitTx: incomingTransfer (10506ms, 673469 gas)
startExit (gas used):  683924
      ✓ reference: own balance (Deposit) and counterparty balance (Deposit) - exitTx: incomingTransfer (10900ms, 1060408 gas)
startExit (gas used):  650631
      ✓ reference: own balance (outgoingTransfer) and counterparty balance (incomingTransfer) - exitTx: incomingTransfer (10771ms, 838809 gas)
    verifyDeprecation
startExit (gas used):  503338
verifyDeprecation (gas used):  243559
      ✓ reference: Deposit - challenge: spend - exit: Burn (11954ms, 1123253 gas)
startExit (gas used):  503722
      ✓ should not be able to challenge with the in-flight tx from which the exit was started (6952ms, 1036177 gas)

  Contract: ERC721Predicate
    startExitWithBurntTokens
startExitWithBurntTokens (gas used):  394196
      ✓ Valid exit with burnt tokens (6408ms, 566750 gas)
    startExit
startExit (gas used):  451461
      ✓ reference: incomingTransfer - exitTx: burn (10452ms, 639659 gas)
startExit (gas used):  469990
      ✓ reference: Deposit - exitTx: burn (6482ms, 658124 gas)
startExit (gas used):  456560
      ✓ reference: counterparty balance (Transfer) - exitTx: incomingTransfer (9483ms, 644822 gas)
    verifyDeprecation
      - write test

@0xAshish @jdkanani

@atvanguard atvanguard force-pushed the dev-morevp branch from 1f164c6 to 15da0a6 Jun 8, 2019
Copy link
Member

0xAshish left a comment

Looks great to me, left few comments.

@atvanguard atvanguard merged commit 95af8d3 into master Jun 11, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@atvanguard atvanguard deleted the dev-morevp branch Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.