Skip to content

Commit

Permalink
Merge pull request #668 from sarvesh-ost/add_max_reward_check_in_conf…
Browse files Browse the repository at this point in the history 
…irm_message

Penalty is now burned on revert stake and redeem instead of progress …
  • Loading branch information
@benjaminbollen
benjaminbollen authored Mar 14, 2019
2 parents ed12fa7 + 07542a9 commit c3bfe3c
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 285 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ The current release uses a "anchors" to provide state roots from remote chains.

### Notable Changes

* Penalty is now burned on revert stake and revert redeem instead of returning on progress stake with proof and progree redeem with proof([#668](https://github.com/OpenST/mosaic-contracts/pull/668)).
* MerklePatriciaProof library can now verify valid proof with extension nodes ([#651](https://github.com/OpenSTFoundation/mosaic-contracts/pull/651)).
* Contracts' ABIs and BINs are provided via npm package [@openstfoundation/mosaic-contracts](https://www.npmjs.com/package/@openstfoundation/mosaic-contracts) ([#619](https://github.com/OpenSTFoundation/mosaic-contracts/pull/619), [#637](https://github.com/OpenSTFoundation/mosaic-contracts/pull/637)).
* Contracts are now separated into "Gateway" and "Core" contracts ([#221](https://github.com/OpenSTFoundation/mosaic-contracts/pull/221)).
Expand Down
60 changes: 7 additions & 53 deletions contracts/gateway/EIP20CoGateway.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -550,10 +550,6 @@ contract EIP20CoGateway is GatewayBase {
);

MessageBus.Message storage message = messages[_messageHash];
MessageBus.MessageStatus outboxMessageStatus =
messageBox.outbox[_messageHash];

redeemAmount_ = redeems[_messageHash].amount;

MessageBus.progressOutboxWithProof(
messageBox,
Expand All @@ -564,50 +560,8 @@ contract EIP20CoGateway is GatewayBase {
MessageBus.MessageStatus(_messageStatus)
);

uint256 bountyAmount = redeems[_messageHash].bounty;
(redeemer_, redeemAmount_) =
progressRedeemInternal(_messageHash, message, true, bytes32(0));

// Return revert penalty to redeemer if message is already progressed
// and can't be reverted anymore.
tryReturnPenaltyToRedeemer(
address(uint160(redeemer_)), // cast to address payable
outboxMessageStatus,
MessageBus.MessageStatus(_messageStatus),
bountyAmount
);
}

/**
* @notice Return the revert penalty to the redeemer. Only valid for
* a message transition from DeclaredRevocation -> Progressed.
*
* @dev Should only be called from progressRedeemWithProof. This function
* exists to avoid a stack too deep error.
*
* @param _redeemer Redeemer address.
* @param _outboxMessageStatus Message status before progressing.
* @param _inboxMessageStatus Message status after progressing.
* @param _bountyAmount Bounty amount to use for calculating penalty.
*/
function tryReturnPenaltyToRedeemer(
address payable _redeemer,
MessageBus.MessageStatus _outboxMessageStatus,
MessageBus.MessageStatus _inboxMessageStatus,
uint256 _bountyAmount
)
private
{
if (_outboxMessageStatus != MessageBus.MessageStatus.DeclaredRevocation) {
return;
}
if (_inboxMessageStatus != MessageBus.MessageStatus.Progressed) {
return;
}

// Penalty charged to redeemer for revert redeem.
uint256 penalty = penaltyFromBounty(_bountyAmount);
_redeemer.transfer(penalty);
}

/**
Expand Down Expand Up @@ -664,6 +618,10 @@ contract EIP20CoGateway is GatewayBase {
redeemerNonce_ = message.nonce;
amount_ = redeems[_messageHash].amount;


// Burn penalty amount. Reentrancy is protected by message bus states.
burner.transfer(penalty);

emit RevertRedeemDeclared(
_messageHash,
redeemer_,
Expand All @@ -674,7 +632,7 @@ contract EIP20CoGateway is GatewayBase {

/**
* @notice Complete revert redeem by providing the merkle proof.
* It will burn facilitator bounty and redeemer penalty.
* It will burn facilitator bounty.
*
* @dev Message bus ensures correct execution sequence of methods and also
* provides safety mechanism for any possible re-entrancy attack.
Expand Down Expand Up @@ -751,13 +709,9 @@ contract EIP20CoGateway is GatewayBase {
// Return the redeem amount back.
EIP20Interface(utilityToken).transfer(message.sender, amount_);

// Penalty charged to redeemer.
uint256 penalty = penaltyFromBounty(bounty);

uint256 amountToBurn = bounty.add(penalty);

// Burn bounty and penalty.
burner.transfer(amountToBurn);
// Burn bounty.
burner.transfer(bounty);

emit RedeemReverted(
_messageHash,
Expand Down
59 changes: 3 additions & 56 deletions contracts/gateway/EIP20Gateway.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,6 @@ contract EIP20Gateway is GatewayBase {

// Get the message object
MessageBus.Message storage message = messages[_messageHash];
MessageBus.MessageStatus outboxMessageStatus =
messageBox.outbox[_messageHash];

MessageBus.progressOutboxWithProof(
messageBox,
Expand All @@ -487,58 +485,12 @@ contract EIP20Gateway is GatewayBase {
MessageBus.MessageStatus(_messageStatus)
);

uint256 bountyAmount = stakes[_messageHash].bounty;
(staker_, stakeAmount_) = progressStakeInternal(
_messageHash,
message,
bytes32(0),
true
);

// Return revert penalty to staker if message is already progressed
// and can't be reverted anymore.
tryReturnPenaltyToStaker(
staker_,
outboxMessageStatus,
MessageBus.MessageStatus(_messageStatus),
bountyAmount
);
}

/**
* @notice Return the revert penalty to the staker. Only valid for
* a message transition from DeclaredRevocation -> Progressed.
*
* @dev Should only be called from progressStakeWithProof. This function
* exists to avoid a stack too deep error.
*
* @param _staker Staker address.
* @param _outboxMessageStatus Message status before progressing.
* @param _inboxMessageStatus Message status after progressing.
* @param _bountyAmount Bounty amount to use for calculating penalty.
*/
function tryReturnPenaltyToStaker(
address _staker,
MessageBus.MessageStatus _outboxMessageStatus,
MessageBus.MessageStatus _inboxMessageStatus,
uint256 _bountyAmount
)
private
{
if (_outboxMessageStatus != MessageBus.MessageStatus.DeclaredRevocation) {
return;
}
if (_inboxMessageStatus != MessageBus.MessageStatus.Progressed) {
return;
}

// Penalty charged to staker for revert stake.
uint256 penalty = penaltyFromBounty(_bountyAmount);
// transfer the penalty amount
require(
baseToken.transfer(_staker, penalty),
"Penalty amount transfer to staker failed"
);
}

/**
Expand Down Expand Up @@ -588,9 +540,9 @@ contract EIP20Gateway is GatewayBase {
// Penalty charged to staker for revert stake.
uint256 penalty = penaltyFromBounty(stakes[_messageHash].bounty);

// Transfer the penalty amount.
// Transfer the penalty amount to burner.
require(
baseToken.transferFrom(msg.sender, address(this), penalty),
baseToken.transferFrom(msg.sender, burner, penalty),
"Staker must approve gateway for penalty amount."
);

Expand All @@ -605,7 +557,7 @@ contract EIP20Gateway is GatewayBase {
/**
* @notice Complete revert stake by providing the merkle proof.
* This method will return stake amount to staker and burn
* facilitator bounty and staker penalty.
* facilitator bounty.
*
* @dev Message bus ensures correct execution sequence of methods and also
* provides safety mechanism for any possible re-entrancy attack.
Expand Down Expand Up @@ -683,11 +635,6 @@ contract EIP20Gateway is GatewayBase {

// Burn facilitator bounty.
baseToken.transfer(burner, stakeBounty);
// Penalty charged to staker.
uint256 penalty = penaltyFromBounty(stakeBounty);

// Burn staker penalty.
baseToken.transfer(burner, penalty);

emit StakeReverted(
_messageHash,
Expand Down
62 changes: 0 additions & 62 deletions test/gateway/eip20_cogateway/progress_redeem_with_proof.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,68 +509,6 @@ contract('EIP20CoGateway.progressRedeemWithProof() ', (accounts) => {
);
});

it('should return penalty to redeemer when the message status in source is '
+ 'declared revocation and in the target is progressed', async () => {
facilitator = accounts[8];
redeemer = redeemParams.redeemer;

await web3.eth.sendTransaction(
{
to: eip20CoGateway.address,
from: facilitator,
value: penaltyAmount,
},
);

await eip20CoGateway.setOutboxStatus(
redeemParams.messageHash,
MessageStatusEnum.DeclaredRevocation,
);

const initialFacilitatorEthBalance = await Utils.getBalance(facilitator);
const initialRedeemerEthBalance = await Utils.getBalance(redeemer);
const initialCoGatewayEthBalance = await Utils.getBalance(eip20CoGateway.address);

await setStorageRoot();

const tx = await eip20CoGateway.progressRedeemWithProof(
redeemParams.messageHash,
proofData.gateway.progress_unstake.proof_data.storageProof[0].serializedProof,
new BN(proofData.gateway.progress_unstake.proof_data.block_number, 16),
MessageStatusEnum.Progressed,
{ from: facilitator },
);

const finalFacilitatorEthBalance = await Utils.getBalance(facilitator);
const finalRedeemerEthBalance = await Utils.getBalance(redeemer);
const finalCoGatewayEthBalance = await Utils.getBalance(eip20CoGateway.address);

const expectedFinalFacilitatorETHBalance = initialFacilitatorEthBalance
.add(bountyAmount)
.subn(tx.receipt.gasUsed);

const expectedFinalRedeemerETHBalance = initialRedeemerEthBalance
.add(penaltyAmount);

assert.strictEqual(
finalFacilitatorEthBalance.eq(expectedFinalFacilitatorETHBalance),
true,
`Facilitator's base token balance ${finalFacilitatorEthBalance.toString(10)} should be equal to ${expectedFinalFacilitatorETHBalance.toString(10)}`,
);

assert.strictEqual(
finalRedeemerEthBalance.eq(expectedFinalRedeemerETHBalance),
true,
`Redeemer's base token balance ${finalRedeemerEthBalance.toString(10)} should be equal to ${expectedFinalRedeemerETHBalance.toString(10)}`,
);

assert.strictEqual(
finalCoGatewayEthBalance.eq(initialCoGatewayEthBalance.sub(bountyAmount).sub(penaltyAmount)),
true,
`CoGateway's base token balance ${finalCoGatewayEthBalance.toString(10)} should be equal to ${initialCoGatewayEthBalance.sub(bountyAmount).sub(penaltyAmount)}.`,
);
});

it('should decrease token supply for utility token', async () => {
const initialTotalSupply = await utilityToken.totalSupply.call();
const initialCoGatewayBalance = await utilityToken.balanceOf(eip20CoGateway.address);
Expand Down
10 changes: 3 additions & 7 deletions test/gateway/eip20_cogateway/progress_revert_redeem.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ contract('EIP20CoGateway.progressRevertRedeem()', (accounts) => {
);
});

it('should burn bounty and penalty amount', async () => {
it('should burn bounty', async () => {
const burnerAddress = await cogateway.burner.call();

const cogatewayInitialTokenBalance = await Utils.getBalance(
Expand All @@ -219,12 +219,8 @@ contract('EIP20CoGateway.progressRevertRedeem()', (accounts) => {
);
const burnerFinalTokenBalance = await Utils.getBalance(burnerAddress);

// Penalty is paid by redeemer and it is burned with bounty on
// message revocation.
const penalty = bountyAmount.muln(PENALTY_MULTIPLIER);

const expectedCoGatewayBalance = cogatewayInitialTokenBalance.sub(
bountyAmount.add(penalty),
bountyAmount,
);
assert.strictEqual(
cogatewayFinalTokenBalance.eq(expectedCoGatewayBalance),
Expand All @@ -241,7 +237,7 @@ contract('EIP20CoGateway.progressRevertRedeem()', (accounts) => {
);

const expectedBurnerBalance = burnerInitialTokenBalance.add(
bountyAmount.add(penalty),
bountyAmount,
);
assert.strictEqual(
burnerFinalTokenBalance.eq(expectedBurnerBalance),
Expand Down
Loading

0 comments on commit c3bfe3c

Please sign in to comment.