Add Collateral Swap Extension #29

sraver · 2023-06-16T13:04:09Z

Context

The Wido team (https://www.joinwido.com/) is submitting this PR to add a Collateral Swap extension for Compound v3. This work was created as part of the CGP grants.

More context about this work has been posted on the Compound Forum.

Sandbox

You can play with the extension here: https://app.compound.finance/extensions/sandbox?market=usdc-mainnet&sandboxsource=https%3A%2F%2Ffleek.ipfs.io%2Fipfs%2FQmQjT6NRo4Hvj7ZGdfA5UMLzN9ZSveN2oxQykugKZoFhgV%2Fembedded.html

Curation and Safety

Our answers to the Curation and Safety Questions can be found in the Forum Post.

Links to accompanying code

Operator repository: https://github.com/widolabs/wido-contracts/tree/main/contracts/compound
SDK repository: https://github.com/widolabs/wido-compound-sdk
SDK documentation: https://docs.joinwido.com/integrate-wido/compound-sdk

kevincheng96

Really neat extension! I have a few comments regarding the extension UI:

In the warning banner at the top, it would be good to explicitly call out that the contracts have not been audited yet.

The numbers are difficult to read, so I suggest adding comma separators or using abbreviations (K, M, B, etc.).
3. It's nice that the extension displays the expected amount of new collateral I'll receive as well as the position summary after the swap. I think it would be useful to also see the exchange rate I'm getting for the collateral. e.g. what's the MATIC / WETH price or the dollar price of MATIC that I am getting with this swap
4. The information toggle next to the fees is not working for me.

torreyatcitty · 2023-07-14T20:04:44Z

Cool stuff, I noticed 2 things.

Could we update this info text when the user is not on the correct wallet network or the correct Compound market selected?

Maybe something like Please ensure both the correct Compound Market is selected and your wallet is connected to the same network

We ran into some user error testing getting the extension to enable first time.

I noticed the following error when trying to do a swap. My swap eventually worked but I've attached a video and text dump of the error:

wido-swap-bug.mov

wido-swap-bug.txt

kevincheng96 · 2023-07-14T20:25:39Z

src/Extensions.ts

+      sign: '*',
+    },
+    source: {
+      url: "https://wido-extension-template.vercel.app/embedded.html"


Could we use an ipfs link instead? See the comet_migrator extension above for an example of how to configure it.

Would it be acceptable to use our backend to serve the "Beta" version, and only deploy the final version to IPFS?

Please let us know if deploying to IPFS even for the Beta launch is necessary and we will do that.

Note: In this post, we proposed the Beta to run for 1 month, in which we would collect feedback. After incorporating the feedback we can launch the final version.

Just spoke with the team about this and we think using IPFS links is necessary. Even though most third-party devs like yourself that are taking the time to build extensions are probably very trustworthy, we want to remove that layer of trust altogether and make sure that whatever is approved as an extension can't be changed without another set of approvals.

mazurroman · 2023-07-18T12:56:01Z

Hey @ajb413 @kevincheng96 @torreyatcitty thank you for your reviews. We will follow up soon

@kevincheng96 regarding your point 4: information toggle next to the fees is not working. Try hovering for longer time, the info should popup. We currently use browser's native way to display that info and it only works with hover. Is there a UI component by Compound that we could reuse for this element, to avoid building custom one?

kevincheng96 · 2023-07-18T17:25:07Z

Try hovering for longer time, the info should popup. We currently use browser's native way to display that info and it only works with hover. Is there a UI component by Compound that we could reuse for this element, to avoid building custom one?

Ah I see, I had to hover for a couple of seconds, but am able to see it now. Btw the cursor turns into the clickable icon when hovering over it, so may be another small fix that should be done. I don't think we currently have a custom toggle component, but will let @torreyatcitty confirm.

torreyatcitty · 2023-07-18T23:28:06Z

Hey @ajb413 @kevincheng96 @torreyatcitty thank you for your reviews. We will follow up soon

@kevincheng96 regarding your point 4: information toggle next to the fees is not working. Try hovering for longer time, the info should popup. We currently use browser's native way to display that info and it only works with hover. Is there a UI component by Compound that we could reuse for this element, to avoid building custom one?

We have a custom built tooltip component in our app that's probably a little too highly coupled with our architecture to just send over as is. Are you using React to build the front end?

Another super easy way if it's nothing fancy is to just do it all in css like it's done here: https://www.w3schools.com/css/tryit.asp?filename=trycss_tooltip

mazurroman · 2023-07-25T15:30:56Z

We resolved all the issues mentioned above, namely:

Resolved issues (raised by @kevincheng96):

Warning banner at the top should contain Audit info

It now looks like this:

Comma separators to make numbers easier to read

It now looks like this:

Add exchange rate

It now looks like this:

Fix info toggle

Resolved issues (raised by @torreyatcitty)

Update tech to switch wallet to correct network

It now looks like this:

Fix error when trying to swap

Next steps: final review & deployment to IPFS

@kevincheng96 mentioned to deploy the UI to IPFS. We propose to do that as the final step in this PR, right before it gets merged.

@kevincheng96 @torreyatcitty Can we kindly ask you for another round of reviews and if everything is now good from your side (except for deploying to IPFS)? Once you confirm, we will deploy to IPFS and ask you to merge this PR.

Note: Thank you @torreyatcitty for your hint on the CSS tooltip. We ended up using it.

kevincheng96 · 2023-08-04T01:44:11Z

@kevincheng96 @torreyatcitty Can we kindly ask you for another round of reviews and if everything is now good from your side (except for deploying to IPFS)?

The fixes in the UI look good to me.

torreyatcitty · 2023-08-04T20:20:23Z

Cool, confirmed the fixes look good to me also.

mazurroman · 2023-08-07T13:48:32Z

@torreyatcitty @kevincheng96 thank you for your reviews, glad to hear it's all good!

We will now proceed to deploying to IPFS so that this PR can be merged and the Collateral Swap Extension is launched as BETA.

Will also plan some marketing around this to ensure people see it and try it out, hopefully with help from the Compound marketing team (@ajb413 for visibility)

sraver · 2023-09-14T06:48:46Z

src/Extensions.ts

+      trx: [
+        {
+          contract: '$operator',
+          abi: 'swapCollateral((address,uint256),(address,uint256),((uint8,bytes32,bytes32),(uint8,bytes32,bytes32)),(address,address,bytes),address)',
+          params: '*'
+        }
+      ]


@kevincheng96 @hayesgm @mykelp I have added the required permission for the app to execute the transaction. Could you verify that it looks fine? I am most interested in the contract: '$operator' line, since I'm not sure what the "$operator" placeholder is replaced with.

the trx object does look ok to me. $operator is meant to signal your smart contract and you do look to have it specified correctly. What error are you currently getting?

@torreyatcitty thanks, with this changes it should work fine then.

Currently the issue is that the deployed version didn't include the trx permissions, so it's failing at the time of actually executing the transaction.

On the embedded sandbox it works fine, with or without the permissions, but once deployed it gets blocked without them.

Being this correct, I think next step would be to re-deploy so it takes the right permissions on the prod app?

ah i see, yes the current version on prod does not have the trx permission. Will share an updated test build with the newer permission.

Add Collataral Swap Extension

87e5fdd

sraver requested review from hayesgm and mykelp as code owners June 16, 2023 13:04

sraver marked this pull request as draft June 17, 2023 08:43

sraver added 2 commits June 21, 2023 10:28

Add Arbitrum market

dac4eb1

Update source link

e129994

sraver marked this pull request as ready for review June 22, 2023 12:23

ajb413 requested a review from kevincheng96 July 14, 2023 17:22

ajb413 approved these changes Jul 14, 2023

View reviewed changes

kevincheng96 reviewed Jul 14, 2023

View reviewed changes

kevincheng96 changed the title ~~Add Collataral Swap Extension~~ Add Collateral Swap Extension Jul 14, 2023

kevincheng96 reviewed Jul 14, 2023

View reviewed changes

Update source link to IPFS

331e6c7

sraver force-pushed the collateral-swap-extension branch from c406dfd to cfa074b Compare September 7, 2023 07:25

Update source link to IPFS

55d5413

sraver force-pushed the collateral-swap-extension branch from cfa074b to 55d5413 Compare September 7, 2023 08:55

add transaction permissions

3a959eb

sraver commented Sep 14, 2023

View reviewed changes

barabanovro added 3 commits September 28, 2023 18:00

Add logs to checkSendTransaction function

379291f

Fix market token symbols

b41555a

Add contract addresses to supportedMarkets

c72df4f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Collateral Swap Extension #29

Add Collateral Swap Extension #29

sraver commented Jun 16, 2023 •

edited

Loading

kevincheng96 left a comment •

edited

Loading

torreyatcitty commented Jul 14, 2023

kevincheng96 Jul 14, 2023

mazurroman Jul 18, 2023 •

edited

Loading

kevincheng96 Jul 18, 2023

mazurroman commented Jul 18, 2023 •

edited

Loading

kevincheng96 commented Jul 18, 2023

torreyatcitty commented Jul 18, 2023

mazurroman commented Jul 25, 2023

kevincheng96 commented Aug 4, 2023

torreyatcitty commented Aug 4, 2023

mazurroman commented Aug 7, 2023 •

edited

Loading

sraver Sep 14, 2023

torreyatcitty Sep 20, 2023

sraver Sep 21, 2023

torreyatcitty Sep 22, 2023

Add Collateral Swap Extension #29

Are you sure you want to change the base?

Add Collateral Swap Extension #29

Conversation

sraver commented Jun 16, 2023 • edited Loading

Context

Sandbox

Curation and Safety

Links to accompanying code

kevincheng96 left a comment • edited Loading

Choose a reason for hiding this comment

torreyatcitty commented Jul 14, 2023

kevincheng96 Jul 14, 2023

Choose a reason for hiding this comment

mazurroman Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

kevincheng96 Jul 18, 2023

Choose a reason for hiding this comment

mazurroman commented Jul 18, 2023 • edited Loading

kevincheng96 commented Jul 18, 2023

torreyatcitty commented Jul 18, 2023

mazurroman commented Jul 25, 2023

Resolved issues (raised by @kevincheng96):

Resolved issues (raised by @torreyatcitty)

Next steps: final review & deployment to IPFS

kevincheng96 commented Aug 4, 2023

torreyatcitty commented Aug 4, 2023

mazurroman commented Aug 7, 2023 • edited Loading

sraver Sep 14, 2023

Choose a reason for hiding this comment

torreyatcitty Sep 20, 2023

Choose a reason for hiding this comment

sraver Sep 21, 2023

Choose a reason for hiding this comment

torreyatcitty Sep 22, 2023

Choose a reason for hiding this comment

sraver commented Jun 16, 2023 •

edited

Loading

kevincheng96 left a comment •

edited

Loading

mazurroman Jul 18, 2023 •

edited

Loading

mazurroman commented Jul 18, 2023 •

edited

Loading

mazurroman commented Aug 7, 2023 •

edited

Loading