SIP-112: Swap ETH for WETH in contract logic by liamzebedee · Pull Request #432 · Synthetixio/SIPs

liamzebedee · 2021-03-26T04:13:26Z

No description provided.

vercel · 2021-03-26T04:13:30Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/synthetixio/sips/12kecZ4xjV9uAcnmujSLbuyiX9fq
✅ Preview: https://sips-git-fork-liamzebedee-master-synthetixio.vercel.app

kaleb-keny

Hey all good with the changes, thank you for incorporating them, although need to ask one thing, what if the user sends both wETH and ETH at the same time... I think then the logic would be to wrap the ETH sent added to the wETH received and then proceed with the calculations? Or does it revert because it kind of adds too much complexity to dealing with summing (meaning the cost of working with the contract would increase significantly)...
Also I believe i need to amend all the test cases to incorporate the case of receiving wETH and receiving ETH. As in the latter we don't have wrapping, while in the former ETH is wrapped... What do you think? I could help with that.

liamzebedee · 2021-03-28T23:27:52Z

what if the user sends both wETH and ETH at the same time... I think then the logic would be to wrap the ETH sent added to the wETH received and then proceed with the calculations?

Good question, I didn't clarify this. How much demand is there for paying with both WETH+ETH? My guess is towards the low end, ready to be refuted here.

liamzebedee · 2021-03-28T23:49:34Z

Also I believe i need to amend all the test cases to incorporate the case of receiving wETH and receiving ETH. As in the latter we don't have wrapping, while in the former ETH is wrapped... What do you think? I could help with that.

If you'd like to write the test cases, that'd be great and I'll review. I think this change is mostly technical, rather than conceptual. So let me know how much detail you think is necessary. I'm still learning about SIP's and who reads em.

If it helps, here's a bit of a breakdown of the logic of accepting both:

mint(amount)
    if there is msg.value sent, then `amount` = msg.value, and the ETH has been received by the contract
    else, we assume amount is in WETH, and we transfer `amount` WETH to the contract

burn(amount, receiveETH)
    if receiveETH = true, then we allow withdrawals

kaleb-keny · 2021-03-29T02:54:02Z

Good question, I didn't clarify this. How much demand is there for paying with both WETH+ETH? My guess is towards the low end, ready to be refuted here.

Yeah, i'd rather not have to deal with this complication 😅 but in case it does happen... of someone calling the contract and sending wETH and ETH at the same time, I'd rather not have any ETH locked in the contract without sETH being minted... As that would break the logic on burning sETH.... since more sETH can be burned later than is minted...

liamzebedee · 2021-03-29T07:16:35Z

in case it does happen... of someone calling the contract and sending wETH and ETH at the same time, I'd rather not have any ETH locked in the contract without sETH being minted

Yeah fully agree with what you're saying here 👍

Clint and I had a bit of discussion today on #core about the design. I will probably design the contracts such that there is a base contract which only uses wETH, and then some utility code (possibly a utility contract) which allows the user to transact with ETH. This will simplify the core interactions of what we're discussing, while allowing users to still interact with ETH. What are your thoughts / do you see any problems?

kaleb-keny · 2021-03-29T07:30:03Z

in case it does happen... of someone calling the contract and sending wETH and ETH at the same time, I'd rather not have any ETH locked in the contract without sETH being minted

Yeah fully agree with what you're saying here 👍

Clint and I had a bit of discussion today on #core about the design. I will probably design the contracts such that there is a base contract which only uses wETH, and then some utility code (possibly a utility contract) which allows the user to transact with ETH. This will simplify the core interactions of what we're discussing, while allowing users to still interact with ETH. What are your thoughts / do you see any problems?

Yeah that would work I think, so just to confirm if I understood it right, the utility function is @payable and does not accept any tokens, what it can do basically wrap the ETH and sends it across to the main core wrapper function. The main core wrapper is no @payble so does not accept any ETH and can only take in wETH.....
The minting/burning takes place on the core function... I think that would work just fine 👍🏼 As long we the core-contract can't hold any ETH by design... I think the implementation would be clean 😅

kaleb-keny · 2021-03-29T17:12:48Z

Alright after some thinking, I think it's fine if wETH is sent to the contract (without minting) and later burned ... as it'll just lower the debt on all minters... So i guess it's fine

Test cases include the case of user depositing wETH or depositing ETH and withdrawing ETH or wETH

seems like a problem with ruby handling many levels of indentation

After the feasibility study, I decided a robust design would involve a core contract handling WETH, and a utility contract that handles native ether.

- Including bounds on burning and minting amounts - Including bounds on burnFeeRate and mintFeeRate - Including test case of minting with both ETH and WETH simultaneously

liamzebedee · 2021-04-07T22:43:34Z

sips/sip-112.md


+#### Key Bounds
+
+- The upper bound on the amount of  *Minting* is determined with a helper function, `capacity`, defined as `maxETH` less `wETH` locked in the contract. In case the user attempts to mint an amount greater than the upper bound, then `capacity` is minted and the residual is returned to the user (please refer to test cases for calculation specs).


capacity is actually max(maxETH - weth, 0) if we're to be precise?

sure, updated the full function

liamzebedee · 2021-04-07T22:44:58Z

sips/sip-112.md


+#### Key Bounds
+
+- The upper bound on the amount of  *Minting* is determined with a helper function, `capacity`, defined as `maxETH` less `wETH` locked in the contract. In case the user attempts to mint an amount greater than the upper bound, then `capacity` is minted and the residual is returned to the user (please refer to test cases for calculation specs).


nitpick - can we use consistent naming for wETH or WETH (I prefer the latter).

old habits die hard, 😅 all wETH changed to WETH

liamzebedee · 2021-04-07T22:46:08Z

sips/sip-112.md

+
+- The upper bound on the amount of  *Minting* is determined with a helper function, `capacity`, defined as `maxETH` less `wETH` locked in the contract. In case the user attempts to mint an amount greater than the upper bound, then `capacity` is minted and the residual is returned to the user (please refer to test cases for calculation specs).
+
+- The upper bound on the amount of *Burning* is computed as `wETH` locked in the contract multiplied `(1+burnFeeRate)` (please refer to test cases for calculation specs).  


🙏 thanks for clarifying this one.

- Giving more scope on the capacity function - change of `wETH` to `WTH`

Swap ETH for WETH in contract logic

eac3fe0

liamzebedee requested a review from kaleb-keny March 26, 2021 04:13

vercel bot deployed to Preview March 26, 2021 04:13 View deployment

allow burn to process ETH or WETH

32e9243

vercel bot deployed to Preview March 26, 2021 04:21 View deployment

kaleb-keny reviewed Mar 27, 2021

View reviewed changes

liamzebedee mentioned this pull request Mar 29, 2021

SIP-112: EtherWrapper Synthetixio/synthetix#1178

Merged

18 tasks

Update Test Cases

4be2d07

Test cases include the case of user depositing wETH or depositing ETH and withdrawing ETH or wETH

vercel bot deployed to Preview March 30, 2021 19:27 View deployment

fixing indentation

845f871

vercel bot deployed to Preview March 30, 2021 19:31 View deployment

fixing indentation 2.0

5f15081

vercel bot deployed to Preview March 30, 2021 19:34 View deployment

indentation 3.0

7bde250

seems like a problem with ruby handling many levels of indentation

vercel bot deployed to Preview March 30, 2021 19:37 View deployment

Specify INativeEtherWrapper contract

1071df5

After the feasibility study, I decided a robust design would involve a core contract handling WETH, and a utility contract that handles native ether.

vercel bot deployed to Preview April 7, 2021 07:15 View deployment

Bounds on Minting/Burning

a74da5b

- Including bounds on burning and minting amounts - Including bounds on burnFeeRate and mintFeeRate - Including test case of minting with both ETH and WETH simultaneously

vercel bot deployed to Preview April 7, 2021 18:08 View deployment

liamzebedee commented Apr 7, 2021

View reviewed changes

capacity function + wETH fix

652b0dc

- Giving more scope on the capacity function - change of `wETH` to `WTH`

vercel bot deployed to Preview April 8, 2021 05:10 View deployment

kaleb-keny merged commit cf13938 into Synthetixio:master Apr 8, 2021


		#### Key Bounds

		- The upper bound on the amount of Minting is determined with a helper function, `capacity`, defined as `maxETH` less `wETH` locked in the contract. In case the user attempts to mint an amount greater than the upper bound, then `capacity` is minted and the residual is returned to the user (please refer to test cases for calculation specs).


		- The upper bound on the amount of Minting is determined with a helper function, `capacity`, defined as `maxETH` less `wETH` locked in the contract. In case the user attempts to mint an amount greater than the upper bound, then `capacity` is minted and the residual is returned to the user (please refer to test cases for calculation specs).

		- The upper bound on the amount of Burning is computed as `wETH` locked in the contract multiplied `(1+burnFeeRate)` (please refer to test cases for calculation specs).

Conversation

liamzebedee commented Mar 26, 2021

Uh oh!

vercel bot commented Mar 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaleb-keny left a comment

Choose a reason for hiding this comment

Uh oh!

liamzebedee commented Mar 28, 2021

Uh oh!

liamzebedee commented Mar 28, 2021

Uh oh!

kaleb-keny commented Mar 29, 2021

Uh oh!

liamzebedee commented Mar 29, 2021

Uh oh!

kaleb-keny commented Mar 29, 2021

Uh oh!

kaleb-keny commented Mar 29, 2021

Uh oh!

liamzebedee Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

kaleb-keny Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

liamzebedee Apr 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaleb-keny Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

liamzebedee Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel bot commented Mar 26, 2021 •

edited

Loading

liamzebedee Apr 7, 2021 •

edited

Loading