Skip to content
This repository has been archived by the owner on Jan 18, 2023. It is now read-only.

Commit

Permalink
Refactor modulo to use SafeMath and utilize safePower (#424)
Browse files Browse the repository at this point in the history
* Fix len

* Add require that 0 is greater than 0

* Implicit overflow already
  • Loading branch information
felix2feng committed Mar 19, 2019
1 parent 33668bf commit be1726a
Show file tree
Hide file tree
Showing 13 changed files with 131 additions and 24 deletions.
4 changes: 2 additions & 2 deletions contracts/core/extensions/CoreIssuance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ contract CoreIssuance is

// Validate quantity is multiple of natural unit
require(
_quantity % setToken.naturalUnit() == 0,
_quantity.mod(setToken.naturalUnit()) == 0,
"Core: Invalid quantity"
);

Expand Down Expand Up @@ -417,7 +417,7 @@ contract CoreIssuance is

// Validate quantity is multiple of natural unit
require(
_quantity % naturalUnit == 0,
_quantity.mod(naturalUnit) == 0,
"Core: Invalid quantity"
);

Expand Down
3 changes: 2 additions & 1 deletion contracts/core/lib/CoreIssuanceLibrary.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pragma experimental "ABIEncoderV2";
import { SafeMath } from "openzeppelin-solidity/contracts/math/SafeMath.sol";

import { IVault } from "../interfaces/IVault.sol";
import { CommonMath } from "../../lib/CommonMath.sol";


/**
Expand Down Expand Up @@ -113,7 +114,7 @@ library CoreIssuanceLibrary {
// Loop through and decrement vault balances for the set, withdrawing if requested
for (uint256 i = 0; i < componentCount; i++) {
// Calculate bit index of current component
uint256 componentBitIndex = 2 ** i;
uint256 componentBitIndex = CommonMath.safePower(2, i);

// Transfer to user unless component index is included in _toExclude
if ((_toExclude & componentBitIndex) != 0) {
Expand Down
2 changes: 1 addition & 1 deletion contracts/core/modules/RebalancingTokenIssuanceModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ contract RebalancingTokenIssuanceModule is
);

require(
baseSetBalance % baseSetNaturalUnit == 0,
baseSetBalance.mod(baseSetNaturalUnit) == 0,
"RebalancingTokenIssuanceModule.getBaseSetRedeemQuantity: Base Redemption must be multiple of natural unit"
);

Expand Down
3 changes: 2 additions & 1 deletion contracts/core/modules/lib/ExchangeExecution.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pragma solidity 0.5.4;

import { SafeMath } from "openzeppelin-solidity/contracts/math/SafeMath.sol";

import { CommonMath } from "../../../lib/CommonMath.sol";
import { ExchangeHeaderLibrary } from "../../lib/ExchangeHeaderLibrary.sol";
import { ExchangeIssuanceLibrary } from "./ExchangeIssuanceLibrary.sol";
import { ExchangeWrapperLibrary } from "../../lib/ExchangeWrapperLibrary.sol";
Expand Down Expand Up @@ -70,7 +71,7 @@ contract ExchangeExecution is
);

// Verify exchange has not already been called
uint256 exchangeBitIndex = 2 ** header.exchange;
uint256 exchangeBitIndex = CommonMath.safePower(2, header.exchange);
require(
(calledExchanges & exchangeBitIndex) == 0,
"ExchangeExecution.executeExchangeOrders: Exchange already called"
Expand Down
5 changes: 4 additions & 1 deletion contracts/core/modules/lib/ExchangeIssuanceLibrary.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

pragma solidity 0.5.4;

import { SafeMath } from "openzeppelin-solidity/contracts/math/SafeMath.sol";

import { ICore } from "../../interfaces/ICore.sol";
import { ISetToken } from "../../interfaces/ISetToken.sol";
import { IVault } from "../../interfaces/IVault.sol";
Expand All @@ -28,6 +30,7 @@ import { IVault } from "../../interfaces/IVault.sol";
* The ExchangeIssuanceLibrary contains functions for validating exchange order data
*/
library ExchangeIssuanceLibrary {
using SafeMath for uint256;

// ============ Structs ============

Expand Down Expand Up @@ -62,7 +65,7 @@ library ExchangeIssuanceLibrary {

// Make sure Issue quantity is multiple of the Set natural unit
require(
_quantity % ISetToken(_set).naturalUnit() == 0,
_quantity.mod(ISetToken(_set).naturalUnit()) == 0,
"ExchangeIssuanceLibrary.validateQuantity: Quantity must be multiple of natural unit"
);
}
Expand Down
3 changes: 2 additions & 1 deletion contracts/core/tokens/SetToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { SafeMath } from "openzeppelin-solidity/contracts/math/SafeMath.sol";
import { ERC20 } from "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";

import { Bytes32 } from "../../lib/Bytes32.sol";
import { CommonMath } from "../../lib/CommonMath.sol";
import { ISetFactory } from "../interfaces/ISetFactory.sol";


Expand Down Expand Up @@ -153,7 +154,7 @@ contract SetToken is

// This is the minimum natural unit possible for a Set with these components.
require(
_naturalUnit >= uint256(10) ** (uint256(18).sub(minDecimals)),
_naturalUnit >= CommonMath.safePower(10, uint256(18).sub(minDecimals)),
"SetToken.constructor: Invalid natural unit"
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ library StandardPlaceBidLibrary {

// Make sure that bid amount is multiple of minimum bid amount
require(
_quantity % _biddingParameters.minimumBid == 0,
_quantity.mod(_biddingParameters.minimumBid) == 0,
"RebalancingSetToken.placeBid: Must bid multiple of minimum bid"
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ library StandardProposeLibrary {
uint256 currentNaturalUnit = ISetToken(_proposeParameters.currentSet).naturalUnit();
uint256 nextSetNaturalUnit = ISetToken(_nextSet).naturalUnit();
require(
Math.max(currentNaturalUnit, nextSetNaturalUnit) %
Math.min(currentNaturalUnit, nextSetNaturalUnit) == 0,
Math.max(currentNaturalUnit, nextSetNaturalUnit).mod(
Math.min(currentNaturalUnit, nextSetNaturalUnit)
) == 0,
"RebalancingSetToken.propose: Invalid proposed Set natural unit"
);

Expand Down
24 changes: 24 additions & 0 deletions contracts/lib/CommonMath.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,30 @@ library CommonMath {
return 2 ** 256 - 1;
}

/**
* @dev Performs the power on a specified value, reverts on overflow.
*/
function safePower(
uint256 a,
uint256 pow
)
internal
pure
returns (uint256)
{
require(a > 0);

uint256 result = 1;
for (uint256 i = 0; i < pow; i++){
uint256 previousResult = result;

// Using safemath multiplication prevents overflows
result = previousResult.mul(a);
}

return result;
}

/**
* Checks for rounding errors and returns value of potential partial amounts of a principal
*
Expand Down
11 changes: 11 additions & 0 deletions contracts/mocks/lib/CommonMathMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ contract CommonMathMock {
return CommonMath.maxUInt256();
}

function testSafePower(
uint256 a,
uint256 pow
)
external
pure
returns (uint256 result)
{
return CommonMath.safePower(a, pow);
}

function testGetPartialAmount(
uint256 _principal,
uint256 _numerator,
Expand Down
14 changes: 6 additions & 8 deletions deployments/stages/5_rebalancing.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { SetProtocolUtils, SetProtocolTestUtils } from 'set-protocol-utils';
import BigNumber from 'bignumber.js';

import * as ABIDecoder from 'abi-decoder';

Expand Down Expand Up @@ -32,7 +31,6 @@ import { RebalancingSetTokenFactory } from '../../artifacts/ts/RebalancingSetTok
import { SetTokenFactory } from '../../artifacts/ts/SetTokenFactory';

import { DEPLOYED_SETS_INFO, DEPENDENCY } from '../deployedContractParameters';
import networkConstants from '../network-constants';
import constants from '../constants';

import {
Expand Down Expand Up @@ -121,9 +119,9 @@ export class RebalancingStage implements DeploymentStageInterface {
setParams.AUCTION_TIME_TO_PIVOT[this._networkConstant],
[
setParams.WBTC_MULTIPLIER.toString(),
setParams.WETH_MULTIPLIER.toString()
setParams.WETH_MULTIPLIER.toString(),
],
allocationBounds
allocationBounds,
],
}).encodeABI();

Expand Down Expand Up @@ -256,9 +254,9 @@ export class RebalancingStage implements DeploymentStageInterface {
DEPLOYED_SETS_INFO.ETHDAI_BTD.AUCTION_TIME_TO_PIVOT[this._networkConstant],
[
DEPLOYED_SETS_INFO.ETHDAI_BTD.DAI_MULTIPLIER.toString(),
DEPLOYED_SETS_INFO.ETHDAI_BTD.WETH_MULTIPLIER.toString()
DEPLOYED_SETS_INFO.ETHDAI_BTD.WETH_MULTIPLIER.toString(),
],
allocationBounds
allocationBounds,
],
}).encodeABI();

Expand Down Expand Up @@ -400,7 +398,7 @@ export class RebalancingStage implements DeploymentStageInterface {
DEPLOYED_SETS_INFO.BTCDAI_BTD.AUCTION_TIME_TO_PIVOT[this._networkConstant],
[
DEPLOYED_SETS_INFO.BTCDAI_BTD.DAI_MULTIPLIER.toString(),
DEPLOYED_SETS_INFO.BTCDAI_BTD.WBTC_MULTIPLIER.toString()
DEPLOYED_SETS_INFO.BTCDAI_BTD.WBTC_MULTIPLIER.toString(),
],
allocationBounds,
],
Expand Down Expand Up @@ -454,7 +452,7 @@ export class RebalancingStage implements DeploymentStageInterface {
}

async deployBTCDaiRebalancingSetToken(): Promise<RebalancingSetTokenContract> {
const name = DEPLOYED_SETS_INFO.BTCDAI_BTD.SET_NAME
const name = DEPLOYED_SETS_INFO.BTCDAI_BTD.SET_NAME;
let address = await getContractAddress(name);

if (address) {
Expand Down
7 changes: 5 additions & 2 deletions deployments/test/5_rebalancing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ import {
getNetworkConstant
} from '../utils/output-helper';

import { calculateInitialSetUnits, calculateGeneralInitialSetUnits, calculateAllocationBounds } from '../utils/rebalancing';
import {
calculateInitialSetUnits,
calculateGeneralInitialSetUnits,
calculateAllocationBounds,
} from '../utils/rebalancing';
import { getWeb3Instance } from '../utils/blockchain';

import { BTCDaiRebalancingManager } from '../../artifacts/ts/BTCDaiRebalancingManager';
Expand All @@ -26,7 +30,6 @@ import { WhiteList } from '../../artifacts/ts/WhiteList';
import { DEPLOYED_SETS_INFO } from '../deployedContractParameters';
import dependencies from '../dependencies';
import constants from '../constants';
import networkConstants from '../network-constants';

describe('Deployment: Rebalancing', () => {

Expand Down
72 changes: 68 additions & 4 deletions test/contracts/lib/commonMath.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ contract('CommonMathMock', accounts => {

let commonMathLibrary: CommonMathMockContract;

beforeEach(async () => {
commonMathLibrary = await libraryMockWrapper.deployCommonMathLibraryAsync();
});

describe('#testMaxUInt256', async () => {
const caller: Address = ownerAccount;

beforeEach(async () => {
commonMathLibrary = await libraryMockWrapper.deployCommonMathLibraryAsync();
});

async function subject(): Promise<BigNumber> {
return commonMathLibrary.testMaxUInt256.callAsync(
{ from: caller },
Expand All @@ -46,6 +46,70 @@ contract('CommonMathMock', accounts => {
});
});

describe('#testSafePower', async () => {
let subjectBase: BigNumber;
let subjectPower: BigNumber;
const caller: Address = ownerAccount;

beforeEach(async () => {
subjectBase = new BigNumber(2);
subjectPower = new BigNumber(5);
});

async function subject(): Promise<BigNumber> {
return commonMathLibrary.testSafePower.callAsync(
subjectBase,
subjectPower,
{ from: caller },
);
}

it('returns the correct value', async () => {
const result = await subject();

const expectedResult =
new BigNumber(subjectBase).pow(subjectPower.toNumber());
expect(result).to.be.bignumber.equal(expectedResult);
});

describe('when the the base is 1', async () => {
beforeEach(async () => {
subjectBase = new BigNumber(1);
subjectPower = new BigNumber(5);
});

it('returns the correct value', async () => {
const result = await subject();

const expectedResult =
new BigNumber(subjectBase).pow(subjectPower.toNumber());
expect(result).to.be.bignumber.equal(expectedResult);
});
});

describe('when the values overflow', async () => {
beforeEach(async () => {
subjectBase = new BigNumber(10000);
subjectPower = new BigNumber(10).pow(99);
});

it('should revert', async () => {
await expectRevertError(subject());
});
});

describe('when the the base is 0', async () => {
beforeEach(async () => {
subjectBase = new BigNumber(0);
subjectPower = new BigNumber(5);
});

it('should revert', async () => {
await expectRevertError(subject());
});
});
});

describe('getPartialAmount', async () => {
let subjectPrincipal: BigNumber;
let subjectNumerator: BigNumber;
Expand Down

0 comments on commit be1726a

Please sign in to comment.