Copy dutch auction asset data utils to @0x/order-utils #1943
Conversation
xianny
force-pushed
the
multilang-1933/move-dutch-auction-utils
branch
from
a6453a8 to
fadaa17
Compare
| beginTimeSeconds: new BigNumber(1562807905), | ||
| beginAmount: new BigNumber(5), | ||
| dutchAuctionAssetData: | ||
| '0xf47261b000000000000000000000000034d402f14d58e001d8efbe6585051bf9706aa064000000000000000000000000000000000000000000000000000000005d268e610000000000000000000000000000000000000000000000000000000000000005', |
There was a problem hiding this comment.
Where did this come from? I looked over in contract-wrappers and couldn't find anything analogous.
It would be nice to cite a source in the comments here, either where you copied it from, or how you generated it. Would be even better to do some round trip encoding/decoding testing. Up to you how far to take it, given your intended scope with this PR.
There was a problem hiding this comment.
This is from a unit test run using Ganache in contract-wrappers.
Round trip encoding/decoding sounds useful -- what would it be like in this case? Something other than encoding/decoding the ERC20 asset data + the dutch auction info?
| beginTimeSeconds, | ||
| beginAmount, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
Now that we have the logic in order-utils, which is a dependency of contract-wrappers, let's have the methods in contract-wrappers call these same methods under-the-hood. That way, we still keep it exposed there, but our code remains DRY.
xianny
force-pushed
the
multilang-1933/move-dutch-auction-utils
branch
from
8286eb3 to
bfa6349
Compare
|
Update, I rebased on development and force-pushed to fix |
Description
Copies to
@0x/order-utilsencoding and decoding methods for Dutch Auction asset data.Does not remove the methods from
@0x/contract-wrappersas we will deprecate them wholesale later.Resolves #1933
Testing instructions
Unit tests
Types of changes
Checklist:
[WIP]if necessary.