Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Remove assertion that protocol fee != zero #2278

Merged
merged 2 commits into from
Oct 22, 2019
Merged

Remove assertion that protocol fee != zero #2278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ac715f
Protocol fee amount is not enforced by staking contract
hysz Oct 22, 2019
c44e16a
Removed assertion that protocol fee != 0 from staking contract.
hysz Oct 22, 2019
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
8 changes: 8 additions & 0 deletions contracts/staking/CHANGELOG.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
{
"note": "Add more overflow safeguards to `LibFixedMath`",
"pr": 2255
},
{
"note": "Refactored finalization state.",
"pr": 2276
},
{
"note": "Removed protocol fee != 0 assertion.",
"pr": 2278
}
]
},
Expand Down
12 changes: 2 additions & 10 deletions contracts/staking/contracts/src/fees/MixinExchangeFees.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,19 +161,11 @@ contract MixinExchangeFees is
private
view
{
if (protocolFeePaid == 0) {
LibRichErrors.rrevert(
LibStakingRichErrors.InvalidProtocolFeePaymentError(
LibStakingRichErrors.ProtocolFeePaymentErrorCodes.ZeroProtocolFeePaid,
protocolFeePaid,
msg.value
)
);
}
// The protocol fee must equal the value passed to the contract; unless
// the value is zero, in which case the fee is taken in WETH.
if (msg.value != protocolFeePaid && msg.value != 0) {
LibRichErrors.rrevert(
LibStakingRichErrors.InvalidProtocolFeePaymentError(
LibStakingRichErrors.ProtocolFeePaymentErrorCodes.MismatchedFeeAndPayment,
protocolFeePaid,
msg.value
)
Expand Down
11 changes: 2 additions & 9 deletions contracts/staking/contracts/src/libs/LibStakingRichErrors.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ library LibStakingRichErrors {
CanOnlyDecreaseOperatorShare
}

enum ProtocolFeePaymentErrorCodes {
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏

ZeroProtocolFeePaid,
MismatchedFeeAndPayment
}

enum InitializationErrorCodes {
MixinSchedulerAlreadyInitialized,
MixinParamsAlreadyInitialized
Expand Down Expand Up @@ -104,9 +99,9 @@ library LibStakingRichErrors {
bytes4 internal constant INVALID_PARAM_VALUE_ERROR_SELECTOR =
0xfc45bd11;

// bytes4(keccak256("InvalidProtocolFeePaymentError(uint8,uint256,uint256)"))
// bytes4(keccak256("InvalidProtocolFeePaymentError(uint256,uint256)"))
bytes4 internal constant INVALID_PROTOCOL_FEE_PAYMENT_ERROR_SELECTOR =
0xefd6cb33;
0x31d7a505;

// bytes4(keccak256("PreviousEpochNotFinalizedError(uint256,uint256)"))
bytes4 internal constant PREVIOUS_EPOCH_NOT_FINALIZED_ERROR_SELECTOR =
Expand Down Expand Up @@ -252,7 +247,6 @@ library LibStakingRichErrors {
}

function InvalidProtocolFeePaymentError(
ProtocolFeePaymentErrorCodes errorCodes,
uint256 expectedProtocolFeePaid,
uint256 actualProtocolFeePaid
)
Expand All @@ -262,7 +256,6 @@ library LibStakingRichErrors {
{
return abi.encodeWithSelector(
INVALID_PROTOCOL_FEE_PAYMENT_ERROR_SELECTOR,
errorCodes,
expectedProtocolFeePaid,
actualProtocolFeePaid
);
Expand Down
18 changes: 0 additions & 18 deletions contracts/staking/test/unit_tests/protocol_fees_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,6 @@ blockchainTests('Protocol Fees unit tests', env => {
return expect(tx).to.revertWith(expectedError);
});

it('should revert if `protocolFeePaid` is zero with zero value sent', async () => {
const tx = testContract.payProtocolFee.awaitTransactionSuccessAsync(
makerAddress,
payerAddress,
ZERO_AMOUNT,
{ from: exchangeAddress, value: ZERO_AMOUNT },
);
const expectedError = new StakingRevertErrors.InvalidProtocolFeePaymentError(
StakingRevertErrors.ProtocolFeePaymentErrorCodes.ZeroProtocolFeePaid,
ZERO_AMOUNT,
ZERO_AMOUNT,
);
return expect(tx).to.revertWith(expectedError);
});

it('should revert if `protocolFeePaid` is zero with non-zero value sent', async () => {
const tx = testContract.payProtocolFee.awaitTransactionSuccessAsync(
makerAddress,
Expand All @@ -113,7 +98,6 @@ blockchainTests('Protocol Fees unit tests', env => {
{ from: exchangeAddress, value: DEFAULT_PROTOCOL_FEE_PAID },
);
const expectedError = new StakingRevertErrors.InvalidProtocolFeePaymentError(
StakingRevertErrors.ProtocolFeePaymentErrorCodes.ZeroProtocolFeePaid,
ZERO_AMOUNT,
DEFAULT_PROTOCOL_FEE_PAID,
);
Expand All @@ -128,7 +112,6 @@ blockchainTests('Protocol Fees unit tests', env => {
{ from: exchangeAddress, value: DEFAULT_PROTOCOL_FEE_PAID.minus(1) },
);
const expectedError = new StakingRevertErrors.InvalidProtocolFeePaymentError(
StakingRevertErrors.ProtocolFeePaymentErrorCodes.MismatchedFeeAndPayment,
DEFAULT_PROTOCOL_FEE_PAID,
DEFAULT_PROTOCOL_FEE_PAID.minus(1),
);
Expand All @@ -143,7 +126,6 @@ blockchainTests('Protocol Fees unit tests', env => {
{ from: exchangeAddress, value: DEFAULT_PROTOCOL_FEE_PAID.plus(1) },
);
const expectedError = new StakingRevertErrors.InvalidProtocolFeePaymentError(
StakingRevertErrors.ProtocolFeePaymentErrorCodes.MismatchedFeeAndPayment,
DEFAULT_PROTOCOL_FEE_PAID,
DEFAULT_PROTOCOL_FEE_PAID.plus(1),
);
Expand Down
4 changes: 4 additions & 0 deletions packages/order-utils/CHANGELOG.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@
{
"note": "Renamed `OnlyCallableByPoolOperatorOrMakerError` to `OnlyCallableByPoolOperatorError`.",
"pr": 2250
},
{
"note": "Removed protocol fee != 0 error.",
"pr": 2278
}
],
"timestamp": 1570135330
Expand Down
10 changes: 2 additions & 8 deletions packages/order-utils/src/staking_revert_errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ export enum OperatorShareErrorCodes {
CanOnlyDecreaseOperatorShare,
}

export enum ProtocolFeePaymentErrorCodes {
ZeroProtocolFeePaid,
MismatchedFeeAndPayment,
}

export enum InvalidParamValueErrorCodes {
InvalidCobbDouglasAlpha,
InvalidRewardDelegatedStakeWeight,
Expand Down Expand Up @@ -144,14 +139,13 @@ export class InvalidParamValueError extends RevertError {

export class InvalidProtocolFeePaymentError extends RevertError {
constructor(
errorCode?: ProtocolFeePaymentErrorCodes,
Copy link
Member

Choose a reason for hiding this comment

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

We can remove the ProtocolFeePaymentErrorCodes enum too right?

expectedProtocolFeePaid?: BigNumber | number | string,
actualProtocolFeePaid?: BigNumber | number | string,
) {
super(
'InvalidProtocolFeePaymentError',
'InvalidProtocolFeePaymentError(uint8 errorCode, uint256 expectedProtocolFeePaid, uint256 actualProtocolFeePaid)',
{ errorCode, expectedProtocolFeePaid, actualProtocolFeePaid },
'InvalidProtocolFeePaymentError(uint256 expectedProtocolFeePaid, uint256 actualProtocolFeePaid)',
{ expectedProtocolFeePaid, actualProtocolFeePaid },
);
}
}
Expand Down