Skip to content
This repository has been archived by the owner on Jan 13, 2023. It is now read-only.

Implement Limit Orders #276

Merged
merged 5 commits into from
Jul 18, 2019
Merged

Implement Limit Orders #276

merged 5 commits into from
Jul 18, 2019

Conversation

BrendanChou
Copy link

No description provided.

@BrendanChou BrendanChou force-pushed the bc_limitorders branch 4 times, most recently from 5437f01 to 29cf40c Compare July 10, 2019 22:24
Copy link
Member

@AntonioJuliano AntonioJuliano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WIP - still have a lot to look at

contracts/external/lib/Bytes.sol Outdated Show resolved Hide resolved
contracts/external/lib/Bytes.sol Outdated Show resolved Hide resolved
contracts/external/lib/Bytes.sol Outdated Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Outdated Show resolved Hide resolved
Copy link
Member

@AntonioJuliano AntonioJuliano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Finished adding comments for now

contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Outdated Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Outdated Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
src/modules/LimitOrders.ts Outdated Show resolved Hide resolved
src/modules/LimitOrders.ts Outdated Show resolved Hide resolved
src/modules/LimitOrders.ts Outdated Show resolved Hide resolved
src/modules/LimitOrders.ts Show resolved Hide resolved
src/modules/operate/AccountOperation.ts Outdated Show resolved Hide resolved
@BrendanChou BrendanChou force-pushed the bc_limitorders branch 2 times, most recently from c723b24 to aac31c4 Compare July 11, 2019 22:52
@BrendanChou BrendanChou force-pushed the bc_limitorders branch 6 times, most recently from 6542925 to ed75ef3 Compare July 12, 2019 23:11
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
Require.that(
newFillAmount <= order.makerAmount,
FILE,
"Cannot overfill order"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to do this?

contracts/external/traders/LimitOrders.sol Outdated Show resolved Hide resolved
contracts/external/traders/LimitOrders.sol Show resolved Hide resolved
src/lib/SignatureHelper.ts Outdated Show resolved Hide resolved
src/lib/SignatureHelper.ts Outdated Show resolved Hide resolved
src/modules/LimitOrders.ts Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
src/modules/operate/AccountOperation.ts Show resolved Hide resolved
@BrendanChou BrendanChou force-pushed the bc_limitorders branch 3 times, most recently from 095140c to 43dce32 Compare July 17, 2019 20:45
contracts/external/lib/TypedSignature.sol Outdated Show resolved Hide resolved
Require.that(
newFillAmount <= order.makerAmount,
FILE,
"Cannot overfill order"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I meant can you add those two things to the revert message

@BrendanChou BrendanChou force-pushed the bc_limitorders branch 3 times, most recently from f4ba218 to 43d9f07 Compare July 17, 2019 22:18
@BrendanChou BrendanChou merged commit 0d3ba13 into master Jul 18, 2019
@BrendanChou BrendanChou deleted the bc_limitorders branch July 18, 2019 00:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants