diff options
Diffstat (limited to 'packages/contracts')
-rw-r--r-- | packages/contracts/contracts/extensions/DutchAuction/DutchAuction.sol | 46 | ||||
-rw-r--r-- | packages/contracts/test/extensions/dutch_auction.ts | 42 |
2 files changed, 58 insertions, 30 deletions
diff --git a/packages/contracts/contracts/extensions/DutchAuction/DutchAuction.sol b/packages/contracts/contracts/extensions/DutchAuction/DutchAuction.sol index dea836da9..7115e5bd5 100644 --- a/packages/contracts/contracts/extensions/DutchAuction/DutchAuction.sol +++ b/packages/contracts/contracts/extensions/DutchAuction/DutchAuction.sol @@ -32,8 +32,8 @@ contract DutchAuction { IExchange internal EXCHANGE; struct AuctionDetails { - uint256 beginTimeSeconds; // Auction begin time in seconds: sellOrder.makerAssetData - uint256 endTimeSeconds; // Auction end time in seconds: sellOrder.expiryTimeSeconds + uint256 beginTimeSeconds; // Auction begin unix timestamp: sellOrder.makerAssetData + uint256 endTimeSeconds; // Auction end unix timestamp: sellOrder.expiryTimeSeconds uint256 beginAmount; // Auction begin amount: sellOrder.makerAssetData uint256 endAmount; // Auction end amount: sellOrder.takerAssetAmount uint256 currentAmount; // Calculated amount given block.timestamp @@ -80,8 +80,6 @@ contract DutchAuction { require(auctionDetails.currentTimeSeconds >= auctionDetails.beginTimeSeconds, "AUCTION_NOT_STARTED"); // Ensure the auction has not expired. This will fail later in 0x but we can save gas by failing early require(sellOrder.expirationTimeSeconds > auctionDetails.currentTimeSeconds, "AUCTION_EXPIRED"); - // Ensure the auction goes from high to low - require(auctionDetails.beginAmount > auctionDetails.endAmount, "INVALID_AMOUNT"); // Validate the buyer amount is greater than the current auction amount require(buyOrder.makerAssetAmount >= auctionDetails.currentAmount, "INVALID_AMOUNT"); // Match orders, maximally filling `buyOrder` @@ -91,19 +89,34 @@ contract DutchAuction { buySignature, sellSignature ); - // Return any spread to the seller + // The difference in sellOrder.takerAssetAmount and current amount is given as spread to the matcher + // This may include additional spread from the buyOrder.makerAssetAmount and the currentAmount. + // e.g currentAmount is 30, sellOrder.takerAssetAmount is 10 and buyOrder.makerAssetamount is 40. + // 10 (40-30) is returned to the buyer, 20 (30-10) sent to the seller and 10 has previously + // been transferred to the seller during matchOrders uint256 leftMakerAssetSpreadAmount = matchedFillResults.leftMakerAssetSpreadAmount; if (leftMakerAssetSpreadAmount > 0) { + // ERC20 Asset data itself is encoded as follows: + // + // | Area | Offset | Length | Contents | + // |----------|--------|---------|-------------------------------------| + // | Header | 0 | 4 | function selector | + // | Params | | 1 * 32 | function parameters: | + // | | 4 | 12 + 20 | 1. token address | + bytes memory assetData = sellOrder.takerAssetData; + address token = assetData.readAddress(16); // Calculate the excess from the buy order. This can occur if the buyer sends in a higher // amount than the calculated current amount uint256 buyerExcessAmount = buyOrder.makerAssetAmount-auctionDetails.currentAmount; uint256 sellerExcessAmount = leftMakerAssetSpreadAmount-buyerExcessAmount; - bytes memory assetData = sellOrder.takerAssetData; - address token = assetData.readAddress(16); + // Return the difference between auctionDetails.currentAmount and sellOrder.takerAssetAmount + // to the seller if (sellerExcessAmount > 0) { address makerAddress = sellOrder.makerAddress; IERC20Token(token).transfer(makerAddress, sellerExcessAmount); } + // Return the difference between buyOrder.makerAssetAmount and auctionDetails.currentAmount + // to the buyer if (buyerExcessAmount > 0) { address takerAddress = buyOrder.makerAddress; IERC20Token(token).transfer(takerAddress, buyerExcessAmount); @@ -122,12 +135,26 @@ contract DutchAuction { returns (AuctionDetails memory auctionDetails) { uint256 makerAssetDataLength = order.makerAssetData.length; - // We assume auctionBeginTimeSeconds and auctionBeginAmount are appended to the makerAssetData + // It is unknown the encoded data of makerAssetData, we assume the last 64 bytes + // are the Auction Details encoding. + // Auction Details is encoded as follows: + // + // | Area | Offset | Length | Contents | + // |----------|--------|---------|-------------------------------------| + // | Params | | 2 * 32 | parameters: | + // | | -64 | 32 | 1. auction begin unix timestamp | + // | | -32 | 32 | 2. auction begin begin amount | + // ERC20 asset data length is 4+32, 64 for auction details results in min length if 100 + require(makerAssetDataLength > 10, "INVALID_ASSET_DATA"); uint256 auctionBeginTimeSeconds = order.makerAssetData.readUint256(makerAssetDataLength-64); uint256 auctionBeginAmount = order.makerAssetData.readUint256(makerAssetDataLength-32); + // Ensure the auction has a valid begin time require(order.expirationTimeSeconds > auctionBeginTimeSeconds, "INVALID_BEGIN_TIME"); uint256 auctionDurationSeconds = order.expirationTimeSeconds-auctionBeginTimeSeconds; + // Ensure the auction goes from high to low uint256 minAmount = order.takerAssetAmount; + require(auctionBeginAmount > minAmount, "INVALID_AMOUNT"); + uint256 amountDelta = auctionBeginAmount-minAmount; // solhint-disable-next-line not-rely-on-time uint256 timestamp = block.timestamp; auctionDetails.beginTimeSeconds = auctionBeginTimeSeconds; @@ -137,8 +164,9 @@ contract DutchAuction { auctionDetails.currentTimeSeconds = timestamp; uint256 remainingDurationSeconds = order.expirationTimeSeconds-timestamp; - uint256 amountDelta = auctionBeginAmount-minAmount; uint256 currentAmount = minAmount + (remainingDurationSeconds*amountDelta/auctionDurationSeconds); + // Check the bounds where we SafeMath was avoivded so the auction details can be queried prior + // and after the auction time. // If the auction has not yet begun the current amount is the auctionBeginAmount currentAmount = timestamp < auctionBeginTimeSeconds ? auctionBeginAmount : currentAmount; // If the auction has ended the current amount is the minAmount diff --git a/packages/contracts/test/extensions/dutch_auction.ts b/packages/contracts/test/extensions/dutch_auction.ts index ae614cbd8..b3408e27d 100644 --- a/packages/contracts/test/extensions/dutch_auction.ts +++ b/packages/contracts/test/extensions/dutch_auction.ts @@ -6,6 +6,7 @@ import { Web3Wrapper } from '@0x/web3-wrapper'; import * as chai from 'chai'; import ethAbi = require('ethereumjs-abi'); import * as ethUtil from 'ethereumjs-util'; +import * as _ from 'lodash'; import { DummyERC20TokenContract } from '../../generated-wrappers/dummy_erc20_token'; import { DummyERC721TokenContract } from '../../generated-wrappers/dummy_erc721_token'; @@ -29,7 +30,7 @@ const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); const DECIMALS_DEFAULT = 18; -describe.only(ContractName.DutchAuction, () => { +describe(ContractName.DutchAuction, () => { let makerAddress: string; let owner: string; let takerAddress: string; @@ -46,7 +47,6 @@ describe.only(ContractName.DutchAuction, () => { let buyerOrderFactory: OrderFactory; let erc20Wrapper: ERC20Wrapper; let erc20Balances: ERC20BalancesByOwner; - let tenMinutesInSeconds: number; let currentBlockTimestamp: number; let auctionBeginTimeSeconds: BigNumber; let auctionEndTimeSeconds: BigNumber; @@ -55,6 +55,8 @@ describe.only(ContractName.DutchAuction, () => { let sellOrder: SignedOrder; let buyOrder: SignedOrder; let erc721MakerAssetIds: BigNumber[]; + const tenMinutesInSeconds = 10 * 60; + async function increaseTimeAsync(): Promise<void> { const timestampBefore = await getLatestBlockTimestampAsync(); await web3Wrapper.increaseTimeAsync(5); @@ -62,10 +64,6 @@ describe.only(ContractName.DutchAuction, () => { // HACK send some transactions when a time increase isn't supported if (timestampAfter === timestampBefore) { await web3Wrapper.sendTransactionAsync({ to: makerAddress, from: makerAddress, value: new BigNumber(1) }); - await web3Wrapper.sendTransactionAsync({ to: makerAddress, from: makerAddress, value: new BigNumber(1) }); - await web3Wrapper.sendTransactionAsync({ to: makerAddress, from: makerAddress, value: new BigNumber(1) }); - await web3Wrapper.sendTransactionAsync({ to: makerAddress, from: makerAddress, value: new BigNumber(1) }); - await web3Wrapper.sendTransactionAsync({ to: makerAddress, from: makerAddress, value: new BigNumber(1) }); } } @@ -159,7 +157,6 @@ describe.only(ContractName.DutchAuction, () => { web3Wrapper.abiDecoder.addABI(zrxToken.abi); erc20Wrapper.addTokenOwnerAddress(dutchAuctionContract.address); - tenMinutesInSeconds = 10 * 60; currentBlockTimestamp = await getLatestBlockTimestampAsync(); // Default auction begins 10 minutes ago auctionBeginTimeSeconds = new BigNumber(currentBlockTimestamp).minus(tenMinutesInSeconds); @@ -229,8 +226,8 @@ describe.only(ContractName.DutchAuction, () => { expect(auctionDetails.beginAmount).to.be.bignumber.equal(auctionBeginAmount); }); it('should be be worth the end price at the end of the auction', async () => { - auctionBeginTimeSeconds = new BigNumber(currentBlockTimestamp - 1000); - auctionEndTimeSeconds = new BigNumber(currentBlockTimestamp - 100); + auctionBeginTimeSeconds = new BigNumber(currentBlockTimestamp - tenMinutesInSeconds * 2); + auctionEndTimeSeconds = new BigNumber(currentBlockTimestamp - tenMinutesInSeconds); sellOrder = await sellerOrderFactory.newSignedOrderAsync({ makerAssetData: extendMakerAssetData( assetDataUtils.encodeERC20AssetData(defaultMakerAssetAddress), @@ -244,9 +241,9 @@ describe.only(ContractName.DutchAuction, () => { expect(auctionDetails.beginAmount).to.be.bignumber.equal(auctionBeginAmount); }); it('should match orders at current amount and send excess to buyer', async () => { - const auctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); + const beforeAuctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); buyOrder = await buyerOrderFactory.newSignedOrderAsync({ - makerAssetAmount: auctionDetails.currentAmount.times(2), + makerAssetAmount: beforeAuctionDetails.currentAmount.times(2), }); await web3Wrapper.awaitTransactionSuccessAsync( await dutchAuctionContract.matchOrders.sendTransactionAsync( @@ -259,6 +256,7 @@ describe.only(ContractName.DutchAuction, () => { }, ), ); + const afterAuctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); const newBalances = await erc20Wrapper.getBalancesAsync(); expect(newBalances[dutchAuctionContract.address][wethContract.address]).to.be.bignumber.equal( constants.ZERO_AMOUNT, @@ -267,13 +265,13 @@ describe.only(ContractName.DutchAuction, () => { // between multiple calls to the same block. Which can move the amount in our case // ref: https://github.com/trufflesuite/ganache-core/issues/111 expect(newBalances[makerAddress][wethContract.address]).to.be.bignumber.gte( - erc20Balances[makerAddress][wethContract.address].plus(auctionDetails.currentAmount), + erc20Balances[makerAddress][wethContract.address].plus(afterAuctionDetails.currentAmount), ); - expect(newBalances[takerAddress][wethContract.address]).to.be.bignumber.equal( - erc20Balances[takerAddress][wethContract.address].minus(auctionDetails.currentAmount), + expect(newBalances[takerAddress][wethContract.address]).to.be.bignumber.gte( + erc20Balances[takerAddress][wethContract.address].minus(afterAuctionDetails.currentAmount), ); }); - it('should have valid getAuctionDetails at a block in the future', async () => { + it('should have valid getAuctionDetails at some block in the future', async () => { let auctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); const beforeAmount = auctionDetails.currentAmount; await increaseTimeAsync(); @@ -291,6 +289,8 @@ describe.only(ContractName.DutchAuction, () => { sellOrder.signature, { from: takerAddress, + // HACK geth seems to miscalculate the gas required intermittently + gas: 400000, }, ); await web3Wrapper.awaitTransactionSuccessAsync(txHash); @@ -303,7 +303,6 @@ describe.only(ContractName.DutchAuction, () => { sellOrder = await sellerOrderFactory.newSignedOrderAsync({ makerFee: new BigNumber(1), }); - const auctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); const txHash = await dutchAuctionContract.matchOrders.sendTransactionAsync( buyOrder, sellOrder, @@ -314,9 +313,10 @@ describe.only(ContractName.DutchAuction, () => { }, ); await web3Wrapper.awaitTransactionSuccessAsync(txHash); + const afterAuctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); const newBalances = await erc20Wrapper.getBalancesAsync(); expect(newBalances[makerAddress][wethContract.address]).to.be.bignumber.gte( - erc20Balances[makerAddress][wethContract.address].plus(auctionDetails.currentAmount), + erc20Balances[makerAddress][wethContract.address].plus(afterAuctionDetails.currentAmount), ); expect(newBalances[feeRecipientAddress][zrxToken.address]).to.be.bignumber.equal( erc20Balances[feeRecipientAddress][zrxToken.address].plus(sellOrder.makerFee), @@ -326,7 +326,6 @@ describe.only(ContractName.DutchAuction, () => { buyOrder = await buyerOrderFactory.newSignedOrderAsync({ makerFee: new BigNumber(1), }); - const auctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); const txHash = await dutchAuctionContract.matchOrders.sendTransactionAsync( buyOrder, sellOrder, @@ -338,8 +337,9 @@ describe.only(ContractName.DutchAuction, () => { ); await web3Wrapper.awaitTransactionSuccessAsync(txHash); const newBalances = await erc20Wrapper.getBalancesAsync(); + const afterAuctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); expect(newBalances[makerAddress][wethContract.address]).to.be.bignumber.gte( - erc20Balances[makerAddress][wethContract.address].plus(auctionDetails.currentAmount), + erc20Balances[makerAddress][wethContract.address].plus(afterAuctionDetails.currentAmount), ); expect(newBalances[feeRecipientAddress][zrxToken.address]).to.be.bignumber.equal( erc20Balances[feeRecipientAddress][zrxToken.address].plus(buyOrder.makerFee), @@ -432,7 +432,6 @@ describe.only(ContractName.DutchAuction, () => { takerAssetAmount: new BigNumber(1), takerAssetData: sellOrder.makerAssetData, }); - const auctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); await web3Wrapper.awaitTransactionSuccessAsync( await dutchAuctionContract.matchOrders.sendTransactionAsync( buyOrder, @@ -444,12 +443,13 @@ describe.only(ContractName.DutchAuction, () => { }, ), ); + const afterAuctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder); const newBalances = await erc20Wrapper.getBalancesAsync(); // HACK gte used here due to a bug in ganache where the timestamp can change // between multiple calls to the same block. Which can move the amount in our case // ref: https://github.com/trufflesuite/ganache-core/issues/111 expect(newBalances[makerAddress][wethContract.address]).to.be.bignumber.gte( - erc20Balances[makerAddress][wethContract.address].plus(auctionDetails.currentAmount), + erc20Balances[makerAddress][wethContract.address].plus(afterAuctionDetails.currentAmount), ); const newOwner = await erc721Token.ownerOf.callAsync(makerAssetId); expect(newOwner).to.be.bignumber.equal(takerAddress); |