This repository was archived by the owner on Mar 19, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 60
WIP: Cancel order for specific trading pair #239
Closed
prakal
wants to merge
4
commits into
LoopringSecondary:cancel-order-for-specific-trading-pair
from
prakal:cancel-order-for-specific-trading-pair
Closed
WIP: Cancel order for specific trading pair #239
prakal
wants to merge
4
commits into
LoopringSecondary:cancel-order-for-specific-trading-pair
from
prakal:cancel-order-for-specific-trading-pair
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
please create pull request.
…Sent from my iPhone
On Jan 14, 2018, at 3:03 AM, Pranav ***@***.***> wrote:
Adds additional tests for setTradingPairCutoff and some more minor changes. It also contains JS implementation of Keccak_256 for another test that verifies SHA combination as expected.
TODO:
should not fill orders which have a timestamp before the cutoff set by setTradingPairCutoff. test is not working because I am still trying to figure out how to set the order timestamps on the individual orders in testing, which would enable me to write tests for cutoff business logic. Some help would be great, @dong77
You can view, comment on, or merge this pull request online at:
#239
Commit Summary
modify function name, comment change
modify setCutoff to cancelAllOrders, fix minor syntax error in Claimable
add tests, add helper keccack_256 function
File Changes
M contracts/LoopringProtocol.sol (2)
M contracts/LoopringProtocolImpl.abi (2)
M contracts/LoopringProtocolImpl.sol (4)
M contracts/lib/Claimable.sol (2)
M package.json (1)
M test/testLoopringProtocolImpl.ts (154)
M util/order.ts (7)
Patch Links:
https://github.com/Loopring/protocol/pull/239.patch
https://github.com/Loopring/protocol/pull/239.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Contributor
Author
|
Not sure I understand, @dong77. This is the pull request that contains the changes. |
Contributor
|
I’m here for help. Come to talk to me anytime you want.
… On 14 Jan 2018, at 03:03, Pranav ***@***.*** ***@***.***>> wrote:
Adds additional tests for setTradingPairCutoff and some more minor changes. It also contains JS implementation of Keccak_256 for another test that verifies SHA combination as expected.
TODO:
should not fill orders which have a timestamp before the cutoff set by setTradingPairCutoff. test is not working because I am still trying to figure out how to set the order timestamps on the individual orders in testing, which would enable me to write tests for cutoff business logic. Some help would be great, @dong77 <https://github.com/dong77>
You can view, comment on, or merge this pull request online at:
#239 <#239>
Commit Summary
modify function name, comment change
modify setCutoff to cancelAllOrders, fix minor syntax error in Claimable
add tests, add helper keccack_256 function
File Changes
M contracts/LoopringProtocol.sol <https://github.com/Loopring/protocol/pull/239/files#diff-0> (2)
M contracts/LoopringProtocolImpl.abi <https://github.com/Loopring/protocol/pull/239/files#diff-1> (2)
M contracts/LoopringProtocolImpl.sol <https://github.com/Loopring/protocol/pull/239/files#diff-2> (4)
M contracts/lib/Claimable.sol <https://github.com/Loopring/protocol/pull/239/files#diff-3> (2)
M package.json <https://github.com/Loopring/protocol/pull/239/files#diff-4> (1)
M test/testLoopringProtocolImpl.ts <https://github.com/Loopring/protocol/pull/239/files#diff-5> (154)
M util/order.ts <https://github.com/Loopring/protocol/pull/239/files#diff-6> (7)
Patch Links:
https://github.com/Loopring/protocol/pull/239.patch <https://github.com/Loopring/protocol/pull/239.patch>
https://github.com/Loopring/protocol/pull/239.diff <https://github.com/Loopring/protocol/pull/239.diff>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#239>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABoYB_2_eYsvXO7fERs_hHRAuYu9TNj1ks5tKd7-gaJpZM4RdjZX>.
|
Contributor
Author
|
Thanks, its going to take me some time to get to the convention center, so remote help would be appreciated. I'll try to summarize what I am failing to understand: I am having issues with attaching a timeStamp on a particular order while testing. I am trying to do this to make sure that the |
Contributor
|
You don’t attach a timestamp to orders. Attach timestamp to trading pair ids.
…Sent from my iPhone
On Jan 14, 2018, at 11:16 AM, Pranav ***@***.***> wrote:
Thanks, its going to take me some time to get to the convention center, so remote help would be appreciated. I'll try to summarize what I am failing to understand:
I am having issues with attaching a timeStamp on a particular order while testing. I am trying to do this to make sure that the setTradingPairCutoff method works and cancels previous orders on the trading pairs as expected.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds additional tests for
setTradingPairCutoffand some more minor changes. It also contains JS implementation of Keccak_256 for another test that verifies SHA combination as expected.TODO:
should not fill orders which have a timestamp before the cutoff set by setTradingPairCutoff.test is not working because I am still trying to figure out how to set the order timestamps on the individual orders in testing, which would enable me to write tests for cutoff business logic. Some help would be great, @dong77