0xProject / 0x-launch-kit-frontend

Apache License 2.0
112 stars 207 forks source link

Shouldn't need to unlock with when you're making a market buy #505

Closed burnsben closed 5 years ago

burnsben commented 5 years ago
Screen Shot 2019-05-30 at 12 46 15 PM

This should generally default to using ETH with the forwarder contract. Shouldn't need to unlock wETH.

fvictorio commented 5 years ago

We only use the forwarder contract in the erc721 marketplace. Should we work on changing this?

burnsben commented 5 years ago

Yes. The erc20 marketplace needs this for market buys. We were under the impression that this would be included in the MVP. We highlighted this in the blog post as one of the features.

burnsben commented 5 years ago
Screen Shot 2019-05-31 at 3 22 15 PM

Shouldn't need to convert or unlock.

mariano-aguero commented 5 years ago

There is a work in progress in this branch hotfix/add-fowarder-contract-to-market-buy

unjapones commented 5 years ago

@burnsben we have a WIP branch that uses the forwarder and we found an edgy case. Let's assume the following:

  1. The user has 5 ETH + 5 wETH = 10 ETH + wETH total balance.
  2. Wants to perform a market buy that requires 10 ETH.
  3. MetaMaksk shows that the contract interaction involves 10 ETH and that the account doesn't have enough funds

In this case, he/she would need to unwrap ETH in order to perform the action. We can insert the wrap eth step somewhere in the flow, but is not that a little bit odd?

unjapones commented 5 years ago

Just in case, the anwser for the above was:

This was also added to the PR #549 description.