Skip to content

Conversation

@chrishobcroft
Copy link
Collaborator

Some comments:

  • I renamed claimCollateralAndPayment to claimSellOrderCollateralAndPayment for the LptSellOrder management side of things, because it needed to be differentiated from the equivalent function on the LptBuyOrder management side, which I called claimBuyOrderPaymentAndCollateral. I considered reclaimSellOrderCollateralAndClaimPayment and reclaimBuyOrderPaymentAndClaimCollateral but found them too long-winded.

  • I did not add tests, but if you want to write them (which I hope you do), then I would be very grateful if you would submit them as a pull request for me to review.

  • I'm sure I'm missing a require somewhere, or I got something mixed up.

I'm sure I'm missing a require somewhere...
@chrishobcroft chrishobcroft requested a review from willjgriff May 22, 2019 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant