Skip to content

Commit

Permalink
Enhancements (Uniswap#30)
Browse files Browse the repository at this point in the history
* simplify code path in advance of optimization

* optimize mulDiv

change mul to take a uint32

make RESOLUTION and Q112 public

* add echidna test for h

* revert some of the changes

* 4.0.1-alpha
  • Loading branch information
NoahZinsmeister committed Dec 16, 2020
1 parent 30e87d0 commit c01640b
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 10 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/fuzz-testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,7 @@ jobs:
- name: BitMathEchidnaTest
run: |
echidna-test . --contract BitMathEchidnaTest --config echidna.config.yml
- name: FullMathEchidnaTest
run: |
echidna-test . --contract FullMathEchidnaTest --config echidna.config.yml
8 changes: 4 additions & 4 deletions contracts/libraries/FixedPoint.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ library FixedPoint {
uint256 _x;
}

uint8 private constant RESOLUTION = 112;
uint256 private constant Q112 = 0x10000000000000000000000000000;
uint256 private constant Q224 = 0x100000000000000000000000000000000000000000000000000000000;
uint8 public constant RESOLUTION = 112;
uint256 public constant Q112 = 0x10000000000000000000000000000; // 2**112
uint256 private constant Q224 = 0x100000000000000000000000000000000000000000000000000000000; // 2**224
uint256 private constant LOWER_MASK = 0xffffffffffffffffffffffffffff; // decimal of UQ*x112 (lower 112 bits)

// encode a uint112 as a UQ112x112
Expand Down Expand Up @@ -107,7 +107,7 @@ library FixedPoint {
}

// returns a UQ112x112 which represents the ratio of the numerator to the denominator
// lossy if either numerator or denominator is greater than 112 bits
// can be lossy
function fraction(uint256 numerator, uint256 denominator) internal pure returns (uq112x112 memory) {
require(denominator > 0, 'FixedPoint::fraction: division by zero');
if (numerator == 0) return FixedPoint.uq112x112(0);
Expand Down
8 changes: 6 additions & 2 deletions contracts/libraries/FullMath.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity >=0.4.0;
// taken from https://medium.com/coinmonks/math-in-solidity-part-3-percents-and-proportions-4db014e080b1
// license is CC-BY-4.0
library FullMath {
function fullMul(uint256 x, uint256 y) private pure returns (uint256 l, uint256 h) {
function fullMul(uint256 x, uint256 y) internal pure returns (uint256 l, uint256 h) {
uint256 mm = mulmod(x, y, uint256(-1));
l = x * y;
h = mm - l;
Expand Down Expand Up @@ -38,10 +38,14 @@ library FullMath {
uint256 d
) internal pure returns (uint256) {
(uint256 l, uint256 h) = fullMul(x, y);

uint256 mm = mulmod(x, y, d);
if (mm > l) h -= 1;
l -= mm;
require(h < d, 'FullMath::mulDiv: overflow');

if (h == 0) return l / d;

require(h < d, 'FullMath: FULLDIV_OVERFLOW');
return fullDiv(l, h, d);
}
}
15 changes: 15 additions & 0 deletions contracts/test/FullMathEchidnaTest.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// SPDX-License-Identifier: GPL-3.0-or-later

pragma solidity >=0.4.0;

import '../libraries/FullMath.sol';

contract FullMathEchidnaTest {
function checkH(uint256 x, uint256 y) public pure {
// if the mul doesn't overflow in 256-bit space, h should be 0
if (x == 0 || ((x * y) / x == y)) {
(, uint256 h) = FullMath.fullMul(x, y);
assert(h == 0);
}
}
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@uniswap/lib",
"version": "4.0.0-alpha",
"version": "4.0.1-alpha",
"description": "📖 Solidity libraries that are shared across Uniswap contracts",
"files": [
"contracts",
Expand Down
15 changes: 12 additions & 3 deletions test/FixedPoint.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,29 @@ describe('FixedPoint', () => {
})

describe('#mul', () => {
it('works for 0', async () => {
expect((await fixedPoint.mul([0], 1))[0]).to.eq(0)
expect((await fixedPoint.mul([1], 0))[0]).to.eq(0)
})

it('correct multiplication', async () => {
expect((await fixedPoint.mul([BigNumber.from(3).mul(Q112)], BigNumber.from(2)))[0]).to.eq(
BigNumber.from(3).mul(2).mul(Q112)
)
})

it('overflow', async () => {
await expect(fixedPoint.mul([BigNumber.from(1).mul(Q112)], BigNumber.from(2).pow(144))).to.be.revertedWith(
'FixedPoint::mul: overflow'
)
})

it('max of q112x112', async () => {
expect((await fixedPoint.mul([BigNumber.from(2).pow(112)], BigNumber.from(2).pow(112)))[0]).to.eq(
BigNumber.from(2).pow(224)
)
})

it('max without overflow, largest fixed point', async () => {
const maxMultiplier = BigNumber.from(2).pow(32)
expect((await fixedPoint.mul([BigNumber.from(2).pow(224).sub(1)], maxMultiplier))[0]).to.eq(
Expand All @@ -89,6 +97,7 @@ describe('FixedPoint', () => {
'FixedPoint::mul: overflow'
)
})

it('max without overflow, smallest fixed point', async () => {
const maxUint = BigNumber.from(2).pow(256).sub(1)
expect((await fixedPoint.mul([BigNumber.from(1)], maxUint))[0]).to.eq(maxUint)
Expand Down Expand Up @@ -304,7 +313,7 @@ describe('FixedPoint', () => {
// long division but makes fewer iterations
expect(
await fixedPoint.getGasCostOfDivuq([BigNumber.from(10).pow(10).mul(Q112)], [BigNumber.from(25).mul(Q112)])
).to.eq(1480)
).to.eq(1502)
})

it('gas cost of long division with all iterations', async () => {
Expand All @@ -314,7 +323,7 @@ describe('FixedPoint', () => {
[BigNumber.from(10).pow(10).mul(Q112)],
[BigNumber.from(3).mul(BigNumber.from(10).pow(10)).mul(Q112)]
)
).to.eq(1480)
).to.eq(1502)
})
})

Expand Down Expand Up @@ -364,7 +373,7 @@ describe('FixedPoint', () => {
it('gas cost of number greater than Q112 numbers', async () => {
expect(
await fixedPoint.getGasCostOfFraction(Q112.mul(BigNumber.from(2).pow(32).mul(2359)), Q112.mul(2360))
).to.eq(974)
).to.eq(996)
})
})

Expand Down

0 comments on commit c01640b

Please sign in to comment.