CancelOrder and specs. Added orderFills and orderCancels getters to C… #84
Conversation
| }); | ||
|
|
||
| // Make sure cancel order comes from maker | ||
| require(order.makerAddress == msg.sender); |
There was a problem hiding this comment.
add a message here. require(..., message)
There was a problem hiding this comment.
also, who is the maker here and who is the sender?
There was a problem hiding this comment.
Well they should be the same person. You don't want any random person to be able to cancel an order, only the order maker should be able to do that.
| }); | ||
|
|
||
| it("marks the correct amount as filled in orderFills mapping", async () => { | ||
|
|
| await subject(); | ||
|
|
||
| const filled = await core.orderFills.callAsync(issuanceOrderParams.orderHash); | ||
|
|
| assertTokenBalance(setToken, existingBalance.add(subjectQuantityToIssue), signerAddress); | ||
| }); | ||
|
|
||
| it("marks the correct amount as filled in orderFills mapping", async () => { |
There was a problem hiding this comment.
Make sure your test pulls the value before (even if it's 0), and checks that the ending value is what you intended. See: coreIssuance.spec.ts#transfers the required tokens from the user
| } | ||
|
|
||
| it("marks the correct amount as canceled in orderCancels mapping", async () => { | ||
|
|
| await subject(); | ||
|
|
||
| const canceled = await core.orderCancels.callAsync(issuanceOrderParams.orderHash); | ||
|
|
| require(order.makerAddress == msg.sender); | ||
|
|
||
| // Verify order is valid and return amount to be cancelled | ||
| validateOrder( |
There was a problem hiding this comment.
does this matter if the user is trying to cancel?
There was a problem hiding this comment.
Would say it does to avoid adding unnecessary data to storage, also this function will contain the openOrderAmount calculations in the next PR, so more of a placeholder.
Pull Request Test Coverage Report for Build 405
💛 - Coveralls |
|
Squash and rebase |
bweick
force-pushed
the
brian/cancel_order
branch
from
26261ee to
8bc28db
Compare
bweick
force-pushed
the
brian/cancel_order
branch
from
8bc28db to
783a457
Compare
|
|
||
| // Make sure cancel order comes from maker | ||
| require(order.makerAddress == msg.sender, INVALID_CANCEL_ORDER); | ||
|
|
There was a problem hiding this comment.
Also check that the order hasn't been already cancelled?
There was a problem hiding this comment.
Yeah my original comment addresses that...it's gonna be in the next PR.
4 participants
…oreState. Missing from this is logic reconciling fill and cancel amounts against openOrder amounts. That is to follow in the next PR.