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.

Sign up for GitHub

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

[Process Exit Bounty] Add process exit bounty for SE #665

Merged
merged 26 commits into from
Aug 12, 2020
Merged

[Process Exit Bounty] Add process exit bounty for SE #665

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
853c3be
feat: add process exit bounty for SE
souradeep-das Jul 28, 2020
0741796
fix: pass in msg.sender to exitgame
souradeep-das Jul 29, 2020
28c8afe
style: lint js
souradeep-das Jul 29, 2020
89fefef
fix: coverage
souradeep-das Jul 29, 2020
b48c204
revert package.json
souradeep-das Jul 31, 2020
18dc9c9
merge v2.0.0 on branch
souradeep-das Jul 31, 2020
ad65388
fix: modify LC tests
souradeep-das Jul 31, 2020
a4c45bb
style: add newline eof
souradeep-das Jul 31, 2020
904bb4a
Merge branch 'v2.0.0' into souradeep/exit_bounty
souradeep-das Aug 3, 2020
50e4686
fix: modify python tests
souradeep-das Aug 6, 2020
51e3eff
merge v2.0.0 on branch
souradeep-das Aug 6, 2020
e5d620a
fix: rem gasprice form py tests
souradeep-das Aug 6, 2020
bd33161
fix: revert plasma_framework.py
souradeep-das Aug 6, 2020
ba78534
fix: revert plasma_framework.py
souradeep-das Aug 6, 2020
d75f2ff
feat: add Bounty lib tests
souradeep-das Aug 6, 2020
f0e7077
style: js lint
souradeep-das Aug 6, 2020
e3b527a
try python tests
souradeep-das Aug 7, 2020
04af275
add to python tests
souradeep-das Aug 7, 2020
9ea8faa
add w3 python tests
souradeep-das Aug 7, 2020
93ea584
fix: py tests
souradeep-das Aug 7, 2020
2d2b5c6
fix: lint py
souradeep-das Aug 7, 2020
6c78825
fix: lint py
souradeep-das Aug 7, 2020
95ee8d7
fix: minor remove unused w3
souradeep-das Aug 11, 2020
5d63ef8
style: use gasprice as args in lib
souradeep-das Aug 12, 2020
8be513c
style: add eof line
souradeep-das Aug 12, 2020
bb49316
style: remove unused contract
souradeep-das Aug 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plasma_framework/contracts/src/exits/payment/PaymentExitGame.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ contract PaymentExitGame is IExitProcessor, OnlyFromAddress, PaymentStandardExit
* @param token Token (ERC20 address or address(0) for ETH) of the exiting output
* @param processor The processExit initiator
*/
function processExit(uint168 exitId, uint256, address token, address payable processor) external onlyFrom(address(plasmaFramework)) {
function processExit(uint168 exitId, uint256, address token, address payable processor) external onlyFrom(address(paymentExitGameArgs.framework)) {
boolafish marked this conversation as resolved.
Show resolved Hide resolved
if (ExitId.isStandardExit(exitId)) {
PaymentStandardExitRouter.processStandardExit(exitId, token, processor);
} else {
Expand Down
2 changes: 2 additions & 0 deletions plasma_framework/contracts/src/exits/utils/ExitBounty.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ library ExitBounty {

/**
* @notice Returns the Process Exit Bounty size for standard exits
* @dev See https://github.com/omgnetwork/plasma-contracts/issues/658 for discussion about size
* 107000 is the approx gas usage for calling processExit()
*/
function processStandardExitBountySize() internal view returns (uint256) {
return 107000 * tx.gasprice;
souradeep-das marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

@boolafish boolafish Aug 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • As for this implementation and design, it might be better to pass in gas price as args instead of using tx.gasprice for easier testing. I was actually surprise you can call a view function and pass in the gas price in the call to the Wrapper contract 😅 as it is not consuming any gas.
  • However, if we are adding upgrade mechanism, I think it will be easier to be the same as what we do for bond. So we don't use tx.gasprice to do the calculation but hardcode one price there.
  • Minor: slightly concern the abstraction might break a bit as ExitBounty quite likely would need a storage when introducing upgrade mechanism. But would leave that as out of scope just a warning this seems to break in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passing gas price in args will indeed be better 👍 .
For the upgrade mechanism, i was thinking more in lines of updating the multiplier "K" instead of a new value for the size, so we would not need to update the bounty size frequently and can accommodate frequent gas changes? What do you think?

Expand Down
10 changes: 6 additions & 4 deletions plasma_framework/python_tests/testlang/testlang.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,20 @@ class StandardExit:
exitable (boolean): whether will exit at processing
output_id (str): output exit identifier (not exit id)
bond_size (int): value of paid bond
bounty_size (int): value of process exit bounty
"""

def __init__(self, exitable, utxo_pos, output_id, exit_target, amount, bond_size):
def __init__(self, exitable, utxo_pos, output_id, exit_target, amount, bond_size, bounty_size):
self.owner = exit_target
self.amount = amount
self.position = utxo_pos
self.exitable = exitable
self.output_id = output_id
self.bond_size = bond_size
self.bounty_size = bounty_size

def to_list(self):
return [self.owner, self.amount, self.position, self.exitable, self.output_id, self.bond_size]
return [self.owner, self.amount, self.position, self.exitable, self.output_id, self.bond_size, self.bounty_size]

def __str__(self):
return self.to_list().__str__()
Expand Down Expand Up @@ -224,9 +226,9 @@ def start_standard_exit_with_tx_body(self, output_id, output_tx, account, bond=N
transactions = block.transactions
merkle = FixedMerkle(16, list(map(lambda tx: tx.encoded, transactions)))
proof = merkle.create_membership_proof(output_tx.encoded)
bond = bond if bond is not None else self.root_chain.standardExitBond()
bond = bond if bond is not None else self.root_chain.standardExitBond() + self.root_chain.processStandardExitBounty()
self.root_chain.startStandardExit(output_id, output_tx.encoded, proof,
**{'value': bond, 'from': account.address})
**{'value': bond, 'from': account.address, 'gasPrice': 100})

def challenge_standard_exit(self, output_id, spend_id, input_index=None, signature=None):
spend_tx = self.child_chain.get_transaction(spend_id)
Expand Down
21 changes: 12 additions & 9 deletions plasma_framework/python_tests/tests/contracts/root_chain/test_process_exits.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ def prepare_exitable_utxo(testlang, owners, amount, outputs, num_outputs=1):


@pytest.mark.parametrize("num_outputs", [1, 2, 3, 4])
def test_process_exits_standard_exit_should_succeed(testlang, num_outputs, plasma_framework):
def test_process_exits_standard_exit_should_succeed(testlang, w3, num_outputs, plasma_framework):
amount = 100
utxo_pos, output_owner = prepare_exitable_utxo(testlang, [], amount, [], num_outputs)

pre_balance = testlang.get_balance(output_owner)
testlang.flush_events()

testlang.start_standard_exit(utxo_pos, output_owner)
gasCost = w3.eth.last_gas_used * 100
_, _, exit_id = plasma_framework.getNextExit(plasma_framework.eth_vault_id, NULL_ADDRESS_HEX)
start_exit_events = testlang.flush_events()

Expand All @@ -45,7 +46,7 @@ def test_process_exits_standard_exit_should_succeed(testlang, num_outputs, plasm
('ExitFinalized', {"exitId": exit_id}),
('ProcessedExitsNum', {'processedNum': 1, 'token': NULL_ADDRESS_HEX})])

assert testlang.get_balance(output_owner) == pre_balance + amount
assert testlang.get_balance(output_owner) == pre_balance + amount - gasCost - testlang.root_chain.processStandardExitBounty()


def test_successful_process_exit_should_clear_exit_fields_and_set_output_as_spent(testlang):
Expand Down Expand Up @@ -401,7 +402,7 @@ def test_finalize_in_flight_exit_finalizes_only_piggybacked_outputs(testlang, pl
assert not plasma_framework.isOutputFinalized(output_id_1)


def test_finalize_exits_priority_for_in_flight_exits_corresponds_to_the_age_of_youngest_input(testlang):
def test_finalize_exits_priority_for_in_flight_exits_corresponds_to_the_age_of_youngest_input(testlang, w3):
souradeep-das marked this conversation as resolved.
Show resolved Hide resolved
owner, amount = testlang.accounts[0], 100
deposit_0_id = testlang.deposit(owner, amount)
deposit_1_id = testlang.deposit(owner, amount)
Expand All @@ -424,7 +425,7 @@ def test_finalize_exits_priority_for_in_flight_exits_corresponds_to_the_age_of_y
balance = testlang.get_balance(owner)

testlang.process_exits(NULL_ADDRESS, testlang.get_standard_exit_id(spend_00_id), 1)
assert testlang.get_balance(owner) == balance + 30 + testlang.root_chain.standardExitBond()
assert testlang.get_balance(owner) == balance + 30 + testlang.root_chain.standardExitBond() + testlang.root_chain.processStandardExitBounty()

balance = testlang.get_balance(owner)
testlang.process_exits(NULL_ADDRESS, testlang.get_in_flight_exit_id(spend_1_id), 1)
Expand All @@ -433,7 +434,7 @@ def test_finalize_exits_priority_for_in_flight_exits_corresponds_to_the_age_of_y

balance = testlang.get_balance(owner)
testlang.process_exits(NULL_ADDRESS, testlang.get_standard_exit_id(spend_2_id), 1)
assert testlang.get_balance(owner) == balance + 100 + testlang.root_chain.standardExitBond()
assert testlang.get_balance(owner) == balance + 100 + testlang.root_chain.standardExitBond() + testlang.root_chain.processStandardExitBounty()


def test_finalize_in_flight_exit_with_erc20_token_should_succeed(testlang, token, plasma_framework):
Expand Down Expand Up @@ -912,7 +913,7 @@ def test_should_not_allow_to_withdraw_outputs_from_two_ifes_marked_as_canonical_
# assert alice_token_balance == alice_token_balance_before


def test_not_challenged_standard_exit_blocks_ife_output_exit(testlang, plasma_framework, token):
def test_not_challenged_standard_exit_blocks_ife_output_exit(testlang, w3, plasma_framework, token):
alice, amount_token = testlang.accounts[1], 200
caroline, amount_eth_small, amount_eth_big = testlang.accounts[2], 1, 100

Expand All @@ -931,6 +932,7 @@ def test_not_challenged_standard_exit_blocks_ife_output_exit(testlang, plasma_fr
caroline_eth_balance_before = testlang.get_balance(caroline)

testlang.start_standard_exit(deposit_id_eth_small, caroline)
gasCost = w3.eth.last_gas_used * 100
testlang.start_in_flight_exit(swap_tx_id)

testlang.piggyback_in_flight_exit_output(swap_tx_id, 0, alice)
Expand All @@ -951,7 +953,7 @@ def test_not_challenged_standard_exit_blocks_ife_output_exit(testlang, plasma_fr
assert caroline_token_balance == caroline_token_balance_before
# but gets her Eth back
caroline_eth_balance = testlang.get_balance(caroline)
assert caroline_eth_balance == caroline_eth_balance_before + amount_eth_big + amount_eth_small
assert caroline_eth_balance == caroline_eth_balance_before + amount_eth_big + amount_eth_small - gasCost - testlang.root_chain.processStandardExitBounty()

# alice gets tokens
alice_token_balance = token.balanceOf(alice.address)
Expand All @@ -961,7 +963,7 @@ def test_not_challenged_standard_exit_blocks_ife_output_exit(testlang, plasma_fr
assert alice_eth_balance == alice_eth_balance_before


def test_challenged_standard_exit_does_not_block_ife_output_exit(testlang, plasma_framework, token):
def test_challenged_standard_exit_does_not_block_ife_output_exit(testlang, w3, plasma_framework, token):
alice, amount_token = testlang.accounts[1], 200
caroline, amount_eth_small, amount_eth_big = testlang.accounts[2], 1, 100

Expand All @@ -980,6 +982,7 @@ def test_challenged_standard_exit_does_not_block_ife_output_exit(testlang, plasm
caroline_eth_balance_before = testlang.get_balance(caroline)

testlang.start_standard_exit(deposit_id_eth_small, caroline)
gasCost = w3.eth.last_gas_used * 100
testlang.challenge_standard_exit(deposit_id_eth_small, swap_tx_id)
testlang.start_in_flight_exit(swap_tx_id)

Expand All @@ -1001,7 +1004,7 @@ def test_challenged_standard_exit_does_not_block_ife_output_exit(testlang, plasm
assert caroline_token_balance == caroline_token_balance_before + amount_token
# and does not get the Eth back
caroline_eth_balance = testlang.get_balance(caroline)
assert caroline_eth_balance == caroline_eth_balance_before - testlang.root_chain.standardExitBond()
assert caroline_eth_balance == caroline_eth_balance_before - testlang.root_chain.standardExitBond() - gasCost - testlang.root_chain.processStandardExitBounty()

# alice exits with her Eth output
alice_eth_balance = testlang.get_balance(alice)
Expand Down
3 changes: 3 additions & 0 deletions plasma_framework/python_tests/tests/tests_utils/plasma_framework.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,9 @@ def childBlockInterval(self):
def standardExitBond(self):
return self.payment_exit_game.startStandardExitBondSize()

def processStandardExitBounty(self):
return 107000 * 100
souradeep-das marked this conversation as resolved.
Show resolved Hide resolved

def inFlightExitBond(self):
return self.payment_exit_game.startIFEBondSize()

Expand Down
16 changes: 9 additions & 7 deletions plasma_framework/test/endToEndTests/FastExit.e2e.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ contract(
gasPrice: this.dummyGasPrice,
});

this.startStandardExitTxValue = this.startStandardExitBondSize.add(this.processExitBountySize);

this.framework.addExitQueue(config.registerKeys.vaultId.eth, ETH);

this.liquidity = await Liquidity.new(this.framework.address, { from: authority });
Expand Down Expand Up @@ -202,7 +204,7 @@ contract(
depositUtxoPos,
{
from: bob,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
},
),
Expand Down Expand Up @@ -230,7 +232,7 @@ contract(
depositUtxoPos,
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
},
),
Expand All @@ -257,7 +259,7 @@ contract(
depositUtxoPos,
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
},
);
Expand Down Expand Up @@ -305,7 +307,7 @@ contract(
depositUtxoPos,
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
},
),
Expand Down Expand Up @@ -455,7 +457,7 @@ contract(
depositUtxoPos,
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
},
);
Expand Down Expand Up @@ -580,7 +582,7 @@ contract(
depositUtxoPos,
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
},
);
Expand Down Expand Up @@ -719,7 +721,7 @@ contract(
depositUtxoPos,
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
},
);
Expand Down
19 changes: 10 additions & 9 deletions plasma_framework/test/endToEndTests/PaymentStandardExit.e2e.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai
this.processExitBountySize = await this.exitBountyHelper.processStandardExitBountySize({
gasPrice: this.dummyGasPrice,
});
this.startStandardExitTxValue = this.startStandardExitBondSize.add(this.processExitBountySize);
};

const aliceDepositsETH = async () => {
Expand Down Expand Up @@ -150,7 +151,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai
};
await this.exitGame.startStandardExit(args, {
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
});
});
Expand Down Expand Up @@ -241,7 +242,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai

await this.exitGame.startStandardExit(args, {
from: bob,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
});
});
Expand All @@ -261,7 +262,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai

this.bobBalanceBeforeProcessExit = new BN(await web3.eth.getBalance(bob));

this.bobtx = await this.framework.processExits(config.registerKeys.vaultId.eth, ETH, 0, 1, {
this.processTx = await this.framework.processExits(config.registerKeys.vaultId.eth, ETH, 0, 1, {
from: bob,
});
});
Expand All @@ -272,7 +273,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai
.add(this.startStandardExitBondSize)
.add(this.processExitBountySize)
.add(new BN(this.transferTxObject.outputs[0].amount))
.sub(await spentOnGas(this.bobtx.receipt));
.sub(await spentOnGas(this.processTx.receipt));

expect(actualBobBalanceAfterProcessExit).to.be.bignumber.equal(expectedBobBalance);
});
Expand All @@ -296,7 +297,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai

await this.exitGame.startStandardExit(this.startStandardExitArgs, {
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
});

Expand Down Expand Up @@ -352,7 +353,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai
await expectRevert(
this.exitGame.startStandardExit(this.startStandardExitArgs, {
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
}),
'Exit has already started.',
Expand Down Expand Up @@ -432,7 +433,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai

await this.exitGame.startStandardExit(args, {
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
value: this.startStandardExitTxValue,
gasPrice: this.dummyGasPrice,
});
});
Expand All @@ -455,7 +456,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai
this.bobEthBalanceBeforeProcessExit = new BN(await web3.eth.getBalance(bob));
this.aliceErc20BalanceBeforeProcessExit = new BN(await this.erc20.balanceOf(alice));

this.bobtx = await this.framework.processExits(
this.processTx = await this.framework.processExits(
config.registerKeys.vaultId.erc20,
this.erc20.address,
0,
Expand All @@ -477,7 +478,7 @@ contract('PaymentExitGame - Standard Exit - End to End Tests', ([_deployer, _mai
const actualBobEthBalanceAfterProcessExit = new BN(await web3.eth.getBalance(bob));
const expectedBobEthBalance = this.bobEthBalanceBeforeProcessExit
.add(this.processExitBountySize)
.sub(await spentOnGas(this.bobtx.receipt));
.sub(await spentOnGas(this.processTx.receipt));

expect(actualBobEthBalanceAfterProcessExit)
.to.be.bignumber.equal(expectedBobEthBalance);
Expand Down
Loading