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

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

Merged
merged 26 commits into from
Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from 9 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
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ contract PaymentInFlightExitRouterMock is FailFastReentrancyGuard, PaymentInFlig
}

/** override and calls processInFlightExit for test */
function processExit(uint168 exitId, uint256, address ercContract) external {
function processExit(uint168 exitId, uint256, address ercContract, address payable) external {
PaymentInFlightExitRouter.processInFlightExit(exitId, ercContract);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ contract PaymentStandardExitRouterMock is PaymentStandardExitRouter {
}

/** override and calls processStandardExit for test */
function processExit(uint168 exitId, uint256, address ercContract) external {
PaymentStandardExitRouter.processStandardExit(exitId, ercContract);
function processExit(uint168 exitId, uint256, address ercContract, address payable processor) external {
PaymentStandardExitRouter.processStandardExit(exitId, ercContract, processor);
}

/** helper functions for testing */
Expand Down
11 changes: 11 additions & 0 deletions plasma_framework/contracts/mocks/exits/utils/ExitBountyWrapper.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pragma solidity 0.5.11;

import "../../../src/exits/utils/ExitBounty.sol";

contract ExitBountyWrapper {

function processStandardExitBountySize() public view returns (uint256) {
souradeep-das marked this conversation as resolved.
Show resolved Hide resolved
return ExitBounty.processStandardExitBountySize();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ contract DummyExitGame is IExitProcessor {
);

// override ExitProcessor interface
function processExit(uint168 exitId, uint256 vaultId, address ercContract) public {
function processExit(uint168 exitId, uint256 vaultId, address ercContract, address payable) public {
emit ExitFinalizedFromDummyExitGame(exitId, vaultId, ercContract);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ contract ReentrancyExitGame is IExitProcessor {

// override ExitProcessor interface
// This would call the processExits back to mimic reentracy attack
function processExit(uint168, uint256, address) public {
function processExit(uint168, uint256, address, address payable) public {
exitGameController.processExits(vaultId, testToken, 0, reentryMaxExitToProcess);
}

Expand Down
8 changes: 7 additions & 1 deletion plasma_framework/contracts/poc/fast_exits/Liquidity.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import "../../src/framework/models/BlockModel.sol";
import "../../src/utils/Merkle.sol";
import "../../src/exits/payment/routers/PaymentStandardExitRouter.sol";
import "openzeppelin-solidity/contracts/token/ERC721/ERC721Full.sol";
import "../../src/exits/utils/ExitBounty.sol";
souradeep-das marked this conversation as resolved.
Show resolved Hide resolved
import "openzeppelin-solidity/contracts/token/ERC20/IERC20.sol";
import "openzeppelin-solidity/contracts/token/ERC20/SafeERC20.sol";

Expand Down Expand Up @@ -165,7 +166,12 @@ contract Liquidity is ERC721Full {

FungibleTokenOutputModel.Output memory outputFromSecondTransaction
= decodedSecondTx.outputs[0];
exitData[exitId] = ExitData(msg.value, msg.sender, outputFromSecondTransaction.amount, outputFromSecondTransaction.token);
exitData[exitId] = ExitData(
msg.value - ExitBounty.processStandardExitBountySize(),
msg.sender,
outputFromSecondTransaction.amount,
outputFromSecondTransaction.token
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ library PaymentExitDataModel {
* @param exitTarget The address to which the exit withdraws funds
* @param amount The amount of funds to withdraw with this exit
* @param bondSize The size of the bond put up for this exit to start, and which is used to cover the cost of challenges
* @param bountySize The size of the bounty put up to cover the cost of processing the exit
*/
struct StandardExit {
bool exitable;
Expand All @@ -23,6 +24,7 @@ library PaymentExitDataModel {
address payable exitTarget;
uint256 amount;
uint256 bondSize;
uint256 bountySize;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ contract PaymentExitGame is IExitProcessor, OnlyFromAddress, PaymentStandardExit
* @notice Callback processes exit function for the PlasmaFramework to call
* @param exitId The exit ID
* @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) external onlyFrom(address(plasmaFramework)) {
function processExit(uint168 exitId, uint256, address token, address payable processor) external onlyFrom(address(plasmaFramework)) {
if (ExitId.isStandardExit(exitId)) {
PaymentStandardExitRouter.processStandardExit(exitId, token);
PaymentStandardExitRouter.processStandardExit(exitId, token, processor);
} else {
PaymentInFlightExitRouter.processInFlightExit(exitId, token);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ library PaymentChallengeStandardExit {

exitMap.exits[args.exitId].exitable = false;

SafeEthTransfer.transferRevertOnError(msg.sender, data.exitData.bondSize, self.safeGasStipend);
SafeEthTransfer.transferRevertOnError(msg.sender, data.exitData.bondSize + data.exitData.bountySize, self.safeGasStipend);

emit ExitChallenged(data.exitData.utxoPos);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ library PaymentProcessStandardExit {
uint256 amount
);

event BountyRewardFailed(
address indexed receiver,
uint256 amount
);

/**
* @notice Main logic function to process standard exit
* @dev emits ExitOmitted event if the exit is omitted
Expand All @@ -42,7 +47,8 @@ library PaymentProcessStandardExit {
Controller memory self,
PaymentExitDataModel.StandardExitMap storage exitMap,
uint168 exitId,
address token
address token,
address payable processor
)
public
{
Expand All @@ -57,11 +63,16 @@ library PaymentProcessStandardExit {
self.framework.flagOutputFinalized(exit.outputId, exitId);

// we do not want to block a queue if bond return is unsuccessful
bool success = SafeEthTransfer.transferReturnResult(exit.exitTarget, exit.bondSize, self.safeGasStipend);
if (!success) {
bool successBondReturn = SafeEthTransfer.transferReturnResult(exit.exitTarget, exit.bondSize, self.safeGasStipend);
if (!successBondReturn) {
emit BondReturnFailed(exit.exitTarget, exit.bondSize);
}

bool successBountyReturn = SafeEthTransfer.transferReturnResult(processor, exit.bountySize, self.safeGasStipend);
if (!successBountyReturn) {
emit BountyRewardFailed(processor, exit.bountySize);
}

if (token == address(0)) {
self.ethVault.withdraw(exit.exitTarget, exit.amount);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import "../../utils/ExitableTimestamp.sol";
import "../../utils/ExitId.sol";
import "../../utils/OutputId.sol";
import "../../utils/MoreVpFinalization.sol";
import "../../utils/ExitBounty.sol";
import "../../../transactions/PaymentTransactionModel.sol";
import "../../../utils/PosLib.sol";
import "../../../framework/PlasmaFramework.sol";
import "../../utils/ExitableTimestamp.sol";

library PaymentStartStandardExit {
using ExitableTimestamp for ExitableTimestamp.Calculator;
Expand Down Expand Up @@ -170,7 +170,8 @@ library PaymentStartStandardExit {
outputId: data.outputId,
exitTarget: msg.sender,
amount: data.output.amount,
bondSize: msg.value
bondSize: msg.value - ExitBounty.processStandardExitBountySize(),
bountySize: ExitBounty.processStandardExitBountySize()
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ contract PaymentStandardExitRouter is
public
payable
nonReentrant(framework)
onlyWithValue(startStandardExitBondSize())
onlyWithValue(startStandardExitBondSize() + ExitBounty.processStandardExitBountySize())
{
startStandardExitController.run(standardExitMap, args);
}
Expand All @@ -157,8 +157,9 @@ contract PaymentStandardExitRouter is
* @dev This function is designed to be called in the main processExit function, using internal
* @param exitId The standard exit ID
* @param token The token (in erc20 address or address(0) for ETH) of the exiting output
* @param processor The processExit initiator
*/
function processStandardExit(uint168 exitId, address token) internal {
processStandardExitController.run(standardExitMap, exitId, token);
function processStandardExit(uint168 exitId, address token, address payable processor) internal {
processStandardExitController.run(standardExitMap, exitId, token, processor);
}
}
12 changes: 12 additions & 0 deletions plasma_framework/contracts/src/exits/utils/ExitBounty.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
pragma solidity 0.5.11;

library ExitBounty {

/**
* @notice Returns the Process Exit Bounty size for standard exits
*/
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

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?

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ contract ExitGameController is ExitGameRegistry {
queue.delMin();
processedNum++;

processor.processExit(exitId, vaultId, token);
processor.processExit(exitId, vaultId, token, msg.sender);
boolafish marked this conversation as resolved.
Show resolved Hide resolved

if (queue.currentSize() == 0) {
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ interface IExitProcessor {
* @param exitId Unique ID for exit per tx type
* @param vaultId ID of the vault that funds the exit
* @param token Address of the token contract
* @param processor Address of the processExit intitiator
*/
function processExit(uint168 exitId, uint256 vaultId, address token) external;
function processExit(uint168 exitId, uint256 vaultId, address token, address payable processor) external;
}
58 changes: 49 additions & 9 deletions plasma_framework/test/endToEndTests/FastExit.e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const PaymentExitGame = artifacts.require('PaymentExitGame');
const PlasmaFramework = artifacts.require('PlasmaFramework');
const Liquidity = artifacts.require('../Liquidity');
const EthVault = artifacts.require('EthVault');
const ExitBounty = artifacts.require('ExitBountyWrapper');
const Erc20Vault = artifacts.require('Erc20Vault');
const ERC20Mintable = artifacts.require('ERC20Mintable');

Expand Down Expand Up @@ -35,10 +36,11 @@ contract(
alice = await web3.eth.personal.importRawKey(alicePrivateKey, password);
alice = web3.utils.toChecksumAddress(alice);
web3.eth.personal.unlockAccount(alice, password, 3600);
web3.eth.sendTransaction({ to: alice, from: richDad, value: web3.utils.toWei('1', 'ether') });
web3.eth.sendTransaction({ to: alice, from: richDad, value: web3.utils.toWei('2', 'ether') });
};

const deployStableContracts = async () => {
this.exitBountyHelper = await ExitBounty.new();
this.erc20 = await ERC20Mintable.new();
await this.erc20.mint(richDad, INITIAL_ERC20_SUPPLY);
};
Expand All @@ -59,6 +61,12 @@ contract(

this.startStandardExitBondSize = await this.exitGame.startStandardExitBondSize();

this.dummyGasPrice = 1000000;

this.processExitBountySize = await this.exitBountyHelper.processStandardExitBountySize({
gasPrice: this.dummyGasPrice,
});

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

this.liquidity = await Liquidity.new(this.framework.address, { from: authority });
Expand Down Expand Up @@ -192,7 +200,11 @@ contract(
rlpDepositTx,
depositInclusionProof,
depositUtxoPos,
{ from: bob, value: this.startStandardExitBondSize },
{
from: bob,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
souradeep-das marked this conversation as resolved.
Show resolved Hide resolved
gasPrice: this.dummyGasPrice,
},
),
'Was not called by the first Tx owner',
);
Expand All @@ -216,7 +228,11 @@ contract(
rlpDepositTx,
depositInclusionProof,
depositUtxoPos,
{ from: alice, value: this.startStandardExitBondSize },
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
gasPrice: this.dummyGasPrice,
},
),
"Provided Transaction isn't finalized or doesn't exist",
);
Expand All @@ -239,7 +255,11 @@ contract(
rlpDepositTx,
depositInclusionProof,
depositUtxoPos,
{ from: alice, value: this.startStandardExitBondSize },
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
gasPrice: this.dummyGasPrice,
},
);
});

Expand Down Expand Up @@ -283,7 +303,11 @@ contract(
rlpDepositTx,
depositInclusionProof,
depositUtxoPos,
{ from: alice, value: this.startStandardExitBondSize },
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
gasPrice: this.dummyGasPrice,
},
),
'Exit has already started.',
);
Expand Down Expand Up @@ -429,7 +453,11 @@ contract(
rlpDepositTx,
depositInclusionProof,
depositUtxoPos,
{ from: alice, value: this.startStandardExitBondSize },
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
gasPrice: this.dummyGasPrice,
},
);
});

Expand Down Expand Up @@ -550,7 +578,11 @@ contract(
rlpDepositTx,
depositInclusionProof,
depositUtxoPos,
{ from: alice, value: this.startStandardExitBondSize },
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
gasPrice: this.dummyGasPrice,
},
);
});

Expand Down Expand Up @@ -685,7 +717,11 @@ contract(
rlpDepositTx,
depositInclusionProof,
depositUtxoPos,
{ from: alice, value: this.startStandardExitBondSize },
{
from: alice,
value: this.startStandardExitBondSize.add(this.processExitBountySize),
gasPrice: this.dummyGasPrice,
},
);
});

Expand All @@ -711,7 +747,11 @@ contract(
rlpDepositTx,
depositInclusionProof,
depositUtxoPos,
{ from: bob, value: this.updatedStandardExitBondSize },
{
from: bob,
value: this.updatedStandardExitBondSize.add(this.processExitBountySize),
gasPrice: this.dummyGasPrice,
},
);
});

Expand Down
Loading