Return filledAmount from fillOrderAsync #72
Conversation
LogvinovLeon
force-pushed
the
fill-order-amuont
branch
from
9fd1aaa
to
e227a14
Compare
LogvinovLeon
force-pushed
the
fill-order-amuont
branch
from
e227a14
to
42c61ec
Compare
| @@ -144,10 +145,11 @@ export class ExchangeWrapper extends ContractWrapper { | |||
| * execution the tokens cannot be transferred. | |||
| * @param takerAddress The user Ethereum address who would like to fill this order. | |||
| * Must be available via the supplied Web3.Provider passed to 0x.js. | |||
| * @return The amount of the order (in taker tokens baseUnits) that was filled. | |||
There was a problem hiding this comment.
The amount of the order that was filled (in taker token baseUnits).
| @@ -339,6 +339,24 @@ describe('ExchangeWrapper', () => { | |||
| expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) | |||
| .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount)); | |||
| }); | |||
| it('should return filled amount', async () => { | |||
There was a problem hiding this comment.
This test is more complex then this description. A simple should return filled amount test would be:
const signedOrder = await fillScenarios.createFillableSignedOrderAsync(
makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount,
);
const partialFillAmount = new BigNumber(3);
const filledAmount = await zeroEx.exchange.fillOrderAsync(
signedOrder, partialFillAmount, shouldCheckTransfer, takerAddress);
expect(filledAmount).to.be.bignumber.equal(partialFillAmount);
I would add that test and then rename this one to: should return the partially filled amount if the fill amount specified is greater then the amount available
|
Agreed with @abandeali1. Actually all the |
|
It's harder for the batch methods to return a single amount because it's not mandatory to use orders with the same |
|
Let's work on returning something useful for batch methods in a subsequent PR. |
This PR: