Brian/linear auction library #219

bweick · 2018-09-28T00:15:58Z

No description provided.

…k that auction starts at correct price.

bweick · 2018-09-28T00:17:13Z

Integrations with RebalancingSetToken will come next. The interface and constantPriceLibrary I'll be using for integration/tests are already included in this PR.

…uld not be run in coverage.

coveralls · 2018-09-28T00:34:33Z

Pull Request Test Coverage Report for Build 1947

2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.0004%) to 99.858%

Totals
Change from base Build 1905:	0.0004%
Covered Lines:	510
Relevant Lines:	510

💛 - Coveralls

felix2feng · 2018-09-28T02:12:12Z

contracts/mocks/core/lib/ConstantPriceAuctionLibrary.sol

+
+
+/**
+ * @title LinearAuctionLibrary


Knit: ConstantPriceAuctionLibrary

Whoops...yes +1

felix2feng · 2018-09-28T02:13:20Z

contracts/core/lib/auction-price-libraries/LinearAuctionLibrary.sol

+import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol";
+
+/**
+ * @title LinearAuctionLibrary


IMO, we can remove the suffix Library. The other lib files do not have it. Thus, it would be consistent.

felix2feng · 2018-09-28T02:14:12Z

contracts/mocks/core/lib/ConstantPriceAuctionLibrary.sol

+ *
+ */
+
+contract ConstantPriceAuctionLibrary {


If a manager proposes a constant price library, couldn't it be very likely that a rebalance could never end?

It's for testing, not for actual deployment. Why I threw it in the mocks folder.

felix2feng · 2018-09-28T02:15:30Z

test/core/lib/auction-price-libraries/linearAuctionLibrary.spec.ts

+        subjectAuctionStartPrice
+      );
+      expect(returnedPrice).to.be.bignumber.equal(expectedPrice);
+    });


What happens if the price hits 0? If possible, what if it becomes negative?

Maybe we can add unit test cases for these.

It can't hit 0...

felix2feng · 2018-09-28T02:16:24Z

utils/RebalancingTokenWrapper.ts

    return {unitShares, issueAmount};
  }
+
+    public getExpectedLinearAuctionPrice(


Knit: spacing

felix2feng · 2018-09-28T02:16:59Z

utils/RebalancingTokenWrapper.ts

+    curveCoefficient: BigNumber,
+    auctionStartPrice: BigNumber
+  ): BigNumber {
+    const contractTimeIncrement = new BigNumber(30);


Put this in a constant file?

asoong

Specs for constant price curve?

asoong · 2018-09-28T18:28:43Z

contracts/core/lib/auction-price-libraries/LinearAuctionPriceCurve.sol

+pragma solidity 0.4.24;
+
+import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol";
+


For the future. Inconsistent spacing. Two lines here

asoong · 2018-09-28T18:28:48Z

contracts/core/lib/auction-price-libraries/LinearAuctionPriceCurve.sol

+ * Contract used in rebalancing auctions to calculate price based off of a linear curve
+ *
+ */
+


asoong · 2018-09-28T18:29:23Z

contracts/core/lib/auction-price-libraries/LinearAuctionPriceCurve.sol

+        uint256 _curveCoefficient
+    )
+        external
+        returns (uint256)


view modifier?

asoong · 2018-09-28T18:30:25Z

contracts/core/lib/auction-price-libraries/LinearAuctionPriceCurve.sol

+        returns (uint256)
+    {
+        // Increment price every 30 seconds
+        uint256 timeIncrement = 30;


Don't need to define this since it's not being reused. Clarify the 30 in the comment below

asoong · 2018-09-28T18:31:23Z

contracts/mocks/core/lib/ConstantAuctionPriceCurve.sol

+        uint256 _curveCoefficient
+    )
+        external
+        returns (uint256)


asoong · 2018-09-28T18:32:50Z

contracts/mocks/core/lib/ConstantAuctionPriceCurve.sol

+ * @title ConstantAuctionPriceCurve
+ * @author Set Protocol
+ *
+ * Contract used in rebalancing auctions to calculate price based off of a linear curve


Update comment here. What does constant curve mean? Constant ratio right?

asoong · 2018-09-28T18:34:06Z

contracts/mocks/core/lib/ConstantAuctionPriceCurve.sol

+    /*
+     * Return constant price amount
+     *
+     * @param  _auctionStartTime          Time of auction start


Remove these parameters and clarify they are unused but there to conform to IAuctionLibrary. See: https://github.com/SetProtocol/set-protocol-contracts/pull/218/files

asoong · 2018-09-28T18:35:17Z

contracts/mocks/core/lib/ConstantAuctionPriceCurve.sol

+     * @param  _curveCoefficient          The slope (or convexity) of the price curve
+     */
+    function getCurrentPrice(
+        uint256 _auctionStartTime,


Remove these parameter names. Just leave uint256,

asoong · 2018-09-28T18:35:59Z

test/core/lib/auction-price-libraries/linearAuctionPriceCurve.spec.ts

+import { ERC20Wrapper } from '@utils/erc20Wrapper';
+import { CoreWrapper } from '@utils/coreWrapper';
+import { RebalancingTokenWrapper } from '@utils/RebalancingTokenWrapper';
+


Remove whitespace

asoong · 2018-09-28T18:37:13Z

utils/constants.ts

 export const STANDARD_QUANTITY_ISSUED: BigNumber = ether(10);
 export const UNLIMITED_ALLOWANCE_IN_BASE_UNITS = new BigNumber(2).pow(256).minus(1);
 export const ZERO: BigNumber = new BigNumber(0);
+export const AUCTION_TIME_INCREMENT = new BigNumber(30);


What does time denote in these tests? Blocks? Or unix time

unix seconds. Like it does on the blockchain.

…ctionPriceCurve and hard-coded instead. Added view modifier to getCurrentPrice function.

felix2feng

Approve sans knits

felix2feng · 2018-09-28T19:38:13Z

contracts/core/lib/auction-price-libraries/IAuctionPriceCurve.sol

+pragma solidity 0.4.24;
+
+/**
+ * @title IAuctionLibrary


Knit IAuctionPriceCurve

felix2feng · 2018-09-28T19:39:53Z

contracts/core/lib/auction-price-libraries/IAuctionPriceCurve.sol

+ * @title IAuctionLibrary
+ * @author Set Protocol
+ *
+ * The IAuctionLibrary interface provides a structured way to interact with any AuctionLibrary


Knit IAuctionPriceCurve

felix2feng · 2018-09-28T19:40:52Z

test/core/lib/auction-price-libraries/linearAuctionPriceCurve.spec.ts

+const { expect } = chai;
+
+
+contract('LinearAuctionLibrary', accounts => {


LinearAuctionPriceCurve

bweick added 5 commits September 27, 2018 16:30

First stab at auction libraries.

968c172

Linear library finished with tests.

5adb017

Added expectedPrice function to rebalancing token wrapper. Added chec…

4107d5a

…k that auction starts at correct price.

Added and edited comments.

12ec6e5

Removed .only

733d204

bweick requested review from a team, asoong and felix2feng September 28, 2018 00:16

Moved constantPriceAuctionLibrary since it is not tested and thus sho…

071eb0e

…uld not be run in coverage.

felix2feng reviewed Sep 28, 2018

View reviewed changes

bweick added 2 commits September 28, 2018 11:01

Renamed contracts. Added auction time increment constant.

be1af07

Removed .only

074e119

asoong reviewed Sep 28, 2018

View reviewed changes

bweick added 2 commits September 28, 2018 11:42

Added awaits to blockchain calls.

f83009a

Formatting and comments. Removed timeIncrement constant from LinearAu…

f5d5500

…ctionPriceCurve and hard-coded instead. Added view modifier to getCurrentPrice function.

felix2feng approved these changes Sep 28, 2018

View reviewed changes

bweick added 2 commits September 28, 2018 12:44

Edited missed renaming.

d826daa

Remove .only

6134966

bweick merged commit 5d42638 into master Sep 28, 2018

bweick deleted the brian/linear_auction_library branch September 28, 2018 21:06

		pragma solidity 0.4.24;

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

		const { expect } = chai;


		contract('LinearAuctionLibrary', accounts => {

Brian/linear auction library #219

Brian/linear auction library #219

Uh oh!

Conversation

bweick commented Sep 28, 2018

Uh oh!

bweick commented Sep 28, 2018

Uh oh!

coveralls commented Sep 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 1947

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asoong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felix2feng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

coveralls commented Sep 28, 2018 •

edited

Loading