This repository has been archived by the owner on Jul 9, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 465
Simulate Maker transfer in order validation #1714
Merged
dekz
merged 23 commits into
development
from
feature/order-utils/one-sided-transfer-validation
Mar 28, 2019
Merged
Simulate Maker transfer in order validation #1714
dekz
merged 23 commits into
development
from
feature/order-utils/one-sided-transfer-validation
Mar 28, 2019
Conversation
This file contains 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
dekz
changed the title
Simulate Maker transfer in order validation
[WIP]Simulate Maker transfer in order validation
Mar 20, 2019
dekz
changed the title
[WIP]Simulate Maker transfer in order validation
Simulate Maker transfer in order validation
Mar 21, 2019
dekz
force-pushed
the
feature/order-utils/one-sided-transfer-validation
branch
from
March 21, 2019 13:22
7556438
to
33f9f0a
Compare
fabioberger
suggested changes
Mar 21, 2019
contracts/erc20/contracts/test/UntransferrableDummyERC20Token.sol
Outdated
Show resolved
Hide resolved
packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
Outdated
Show resolved
Hide resolved
packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
Outdated
Show resolved
Hide resolved
packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
Outdated
Show resolved
Hide resolved
packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
Outdated
Show resolved
Hide resolved
dekz
requested review from
BMillman19,
fragosti and
steveklebanoff
as code owners
March 21, 2019 16:17
dekz
force-pushed
the
feature/order-utils/one-sided-transfer-validation
branch
from
March 21, 2019 16:27
13ccc03
to
d295421
Compare
fabioberger
approved these changes
Mar 25, 2019
packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
Outdated
Show resolved
Hide resolved
dekz
force-pushed
the
feature/order-utils/one-sided-transfer-validation
branch
from
March 26, 2019 10:48
7e35108
to
1144770
Compare
…per.ts Co-Authored-By: dekz <[email protected]>
…per.ts Co-Authored-By: dekz <[email protected]>
Co-Authored-By: dekz <[email protected]>
dekz
force-pushed
the
feature/order-utils/one-sided-transfer-validation
branch
from
March 28, 2019 13:20
1144770
to
1f2214f
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
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.
Description
Simulate a one sided maker transfer in order validation.
This will trigger the case where the maker has the sufficient balance and allowance, but the asset is non-tradeable. For example in a Paused contract or when the maker has not performed some action to unlock their account.
Pulled from a number of SRA endpoints and walked through the orderbooks with this change. This has flagged a few orders which are currently sitting on orderbooks which are invalid. I.e there are a few tokens where the Maker is "frozen" on live orderbooks.
By default the
order.takerAddress
is used to simulate the receiver from maker. With Open orderbooks this results in theNULL_ADDRESS
This also has a few false positives where some ERC20 tokens cannot be transferred to theNULL_ADDRESS
, e.g Bloom token.In this case the developer is in the best position to determine an alternative address to use. The receiver address during the simulation can be set by specifying
simulationTakerAddress
inValidateOrderFillableOpts
. Whilst 0x could assign a nonNULL_ADDRESS
default, this is unlikely to be fool proof across ALL token standards and networks.Testing instructions
Pulled from a number of SRA endpoints and walked through the orderbooks with this change.
Types of changes
Checklist:
[WIP]
if necessary.