chore: Return excess to buyer

2 files changed, 58 insertions, 29 deletions

diff --git a/packages/contracts/contracts/extensions/DutchAuction/DutchAuction.sol b/packages/contracts/contracts/extensions/DutchAuction/DutchAuction.sol
index 225e33929..dea836da9 100644
--- a/packages/contracts/contracts/extensions/DutchAuction/DutchAuction.sol
+++ b/packages/contracts/contracts/extensions/DutchAuction/DutchAuction.sol
@@ -94,11 +94,20 @@ contract DutchAuction {
         // Return any spread to the seller
         uint256 leftMakerAssetSpreadAmount = matchedFillResults.leftMakerAssetSpreadAmount;
         if (leftMakerAssetSpreadAmount > 0) {
-            // Assume auction is for ERC20
+            // 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);
-            address makerAddress = sellOrder.makerAddress;
-            IERC20Token(token).transfer(makerAddress, leftMakerAssetSpreadAmount);
+            if (sellerExcessAmount > 0) {
+                address makerAddress = sellOrder.makerAddress;
+                IERC20Token(token).transfer(makerAddress, sellerExcessAmount);
+            }
+            if (buyerExcessAmount > 0) {
+                address takerAddress = buyOrder.makerAddress;
+                IERC20Token(token).transfer(takerAddress, buyerExcessAmount);
+            }
         }
         return matchedFillResults;
     }
@@ -116,7 +125,7 @@ contract DutchAuction {
         // We assume auctionBeginTimeSeconds and auctionBeginAmount are appended to the makerAssetData
         uint256 auctionBeginTimeSeconds = order.makerAssetData.readUint256(makerAssetDataLength-64);
         uint256 auctionBeginAmount = order.makerAssetData.readUint256(makerAssetDataLength-32);
-        // require(order.expirationTimeSeconds > auctionBeginTimeSeconds, "INVALID_BEGIN_TIME");
+        require(order.expirationTimeSeconds > auctionBeginTimeSeconds, "INVALID_BEGIN_TIME");
         uint256 auctionDurationSeconds = order.expirationTimeSeconds-auctionBeginTimeSeconds;
         uint256 minAmount = order.takerAssetAmount;
         // solhint-disable-next-line not-rely-on-time
diff --git a/packages/contracts/test/extensions/dutch_auction.ts b/packages/contracts/test/extensions/dutch_auction.ts
index 207da6796..28e72858c 100644
--- a/packages/contracts/test/extensions/dutch_auction.ts
+++ b/packages/contracts/test/extensions/dutch_auction.ts
@@ -28,9 +28,8 @@ chaiSetup.configure();
 const expect = chai.expect;
 const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
 const DECIMALS_DEFAULT = 18;
-const ZERO = Web3Wrapper.toBaseUnitAmount(new BigNumber(0), DECIMALS_DEFAULT);
 
-describe(ContractName.DutchAuction, () => {
+describe.only(ContractName.DutchAuction, () => {
     let makerAddress: string;
     let owner: string;
     let takerAddress: string;
@@ -186,8 +185,8 @@ describe(ContractName.DutchAuction, () => {
             makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), DECIMALS_DEFAULT),
             takerAssetAmount: auctionEndAmount,
             expirationTimeSeconds: auctionEndTimeSeconds,
-            makerFee: ZERO,
-            takerFee: ZERO,
+            makerFee: constants.ZERO_AMOUNT,
+            takerFee: constants.ZERO_AMOUNT,
         };
         // Default buy order is for the auction begin price
         const buyerDefaultOrderParams = {
@@ -244,23 +243,36 @@ describe(ContractName.DutchAuction, () => {
             expect(auctionDetails.currentAmount).to.be.bignumber.equal(auctionEndAmount);
             expect(auctionDetails.beginAmount).to.be.bignumber.equal(auctionBeginAmount);
         });
-        it('should match orders and send excess to seller', async () => {
-            const txHash = await dutchAuctionContract.matchOrders.sendTransactionAsync(
-                buyOrder,
+        it('should match orders at current amount and send excess to buyer', async () => {
+            let auctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(sellOrder);
+            buyOrder = await buyerOrderFactory.newSignedOrderAsync({
+                makerAssetAmount: auctionDetails.currentAmount.times(2),
+            });
+            const receipt = await web3Wrapper.awaitTransactionSuccessAsync(
+                await dutchAuctionContract.matchOrders.sendTransactionAsync(
+                    buyOrder,
+                    sellOrder,
+                    buyOrder.signature,
+                    sellOrder.signature,
+                    {
+                        from: takerAddress,
+                    },
+                ),
+            );
+            auctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(
                 sellOrder,
-                buyOrder.signature,
-                sellOrder.signature,
-                {
-                    from: takerAddress,
-                },
+                {},
+                parseInt(receipt.blockNumber as any, 16),
             );
-            await web3Wrapper.awaitTransactionSuccessAsync(txHash);
             const newBalances = await erc20Wrapper.getBalancesAsync();
             expect(newBalances[dutchAuctionContract.address][wethContract.address]).to.be.bignumber.equal(
                 constants.ZERO_AMOUNT,
             );
             expect(newBalances[makerAddress][wethContract.address]).to.be.bignumber.equal(
-                erc20Balances[makerAddress][wethContract.address].plus(buyOrder.makerAssetAmount),
+                erc20Balances[makerAddress][wethContract.address].plus(auctionDetails.currentAmount),
+            );
+            expect(newBalances[takerAddress][wethContract.address]).to.be.bignumber.equal(
+                erc20Balances[takerAddress][wethContract.address].minus(auctionDetails.currentAmount),
             );
         });
         it('should have valid getAuctionDetails at a block in the future', async () => {
@@ -293,6 +305,7 @@ describe(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,
@@ -305,7 +318,7 @@ describe(ContractName.DutchAuction, () => {
             await web3Wrapper.awaitTransactionSuccessAsync(txHash);
             const newBalances = await erc20Wrapper.getBalancesAsync();
             expect(newBalances[makerAddress][wethContract.address]).to.be.bignumber.equal(
-                erc20Balances[makerAddress][wethContract.address].plus(buyOrder.makerAssetAmount),
+                erc20Balances[makerAddress][wethContract.address].plus(auctionDetails.currentAmount),
             );
             expect(newBalances[feeRecipientAddress][zrxToken.address]).to.be.bignumber.equal(
                 erc20Balances[feeRecipientAddress][zrxToken.address].plus(sellOrder.makerFee),
@@ -315,6 +328,7 @@ describe(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,
@@ -327,7 +341,7 @@ describe(ContractName.DutchAuction, () => {
             await web3Wrapper.awaitTransactionSuccessAsync(txHash);
             const newBalances = await erc20Wrapper.getBalancesAsync();
             expect(newBalances[makerAddress][wethContract.address]).to.be.bignumber.equal(
-                erc20Balances[makerAddress][wethContract.address].plus(buyOrder.makerAssetAmount),
+                erc20Balances[makerAddress][wethContract.address].plus(auctionDetails.currentAmount),
             );
             expect(newBalances[feeRecipientAddress][zrxToken.address]).to.be.bignumber.equal(
                 erc20Balances[feeRecipientAddress][zrxToken.address].plus(buyOrder.makerFee),
@@ -420,19 +434,25 @@ describe(ContractName.DutchAuction, () => {
                     takerAssetAmount: new BigNumber(1),
                     takerAssetData: sellOrder.makerAssetData,
                 });
-                const txHash = await dutchAuctionContract.matchOrders.sendTransactionAsync(
-                    buyOrder,
+                const receipt = await web3Wrapper.awaitTransactionSuccessAsync(
+                    await dutchAuctionContract.matchOrders.sendTransactionAsync(
+                        buyOrder,
+                        sellOrder,
+                        buyOrder.signature,
+                        sellOrder.signature,
+                        {
+                            from: takerAddress,
+                        },
+                    ),
+                );
+                const auctionDetails = await dutchAuctionContract.getAuctionDetails.callAsync(
                     sellOrder,
-                    buyOrder.signature,
-                    sellOrder.signature,
-                    {
-                        from: takerAddress,
-                    },
+                    {},
+                    parseInt(receipt.blockNumber as any, 16),
                 );
-                await web3Wrapper.awaitTransactionSuccessAsync(txHash);
                 const newBalances = await erc20Wrapper.getBalancesAsync();
                 expect(newBalances[makerAddress][wethContract.address]).to.be.bignumber.equal(
-                    erc20Balances[makerAddress][wethContract.address].plus(buyOrder.makerAssetAmount),
+                    erc20Balances[makerAddress][wethContract.address].plus(auctionDetails.currentAmount),
                 );
                 const newOwner = await erc721Token.ownerOf.callAsync(makerAssetId);
                 expect(newOwner).to.be.bignumber.equal(takerAddress);