Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a wrapper contract that allows users to use ETH instead of WETH #404

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

penandlim
Copy link
Contributor

@penandlim penandlim commented Jun 15, 2021

SwapEthWrapper.sol is a wrapper contract of Swap.sol variants. It allows users to use ETH instead of WETH whenever interacting with this contract.

The params associated with the WETH9 token in the underlying pool should match the msg.value. For more details please take a look at the test cases for addLiquidity() or swap() functions

@penandlim penandlim linked an issue Jun 15, 2021 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Jun 15, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@363b018). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #404   +/-   ##
=========================================
  Coverage          ?   99.42%           
=========================================
  Files             ?       15           
  Lines             ?     1036           
  Branches          ?      154           
=========================================
  Hits              ?     1030           
  Misses            ?        6           
  Partials          ?        0           
Impacted Files Coverage Δ
contracts/Swap.sol 100.00% <ø> (ø)
contracts/SwapEthWrapper.sol 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 363b018...f74d7ab. Read the comment docs.

@penandlim penandlim marked this pull request as ready for review June 15, 2021 04:24
} else {
IWETH9(weth).withdraw(amounts[i]);
// slither-disable-next-line arbitrary-send
(bool success, ) = msg.sender.call{value: amounts[i]}("");
Copy link

Choose a reason for hiding this comment

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

Why performing the send like this instead of a normal transfer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if this is a legit concern as of today but in case the gas requirements for certain smart contract interactions change, .transfer could fail.

https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

@alphastorm alphastorm force-pushed the master branch 6 times, most recently from 5ac5eb5 to 9291414 Compare July 30, 2021 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make wrapper contracts for allowing direct ETH interactions
3 participants