yearn-sdk icon indicating copy to clipboard operation
yearn-sdk copied to clipboard

Add Wido Zaps with support for more vaults

Open mazurroman opened this issue 2 years ago • 4 comments

Description

Add Zap support for more vaults on Yearn.

Wido brings Zap support for:

  • yvCurve-ibAUD-USDC
  • yvCurve-ibGBP-USDC
  • yvCurve-ibCHF-USDC
  • yvCurve-ibJPY-USDC
  • yvCurve-ibKRW-USDC
  • yvCurve-ibBTC

With Zaps for more vaults to come later.

Related Issue

PR in yearn-finance-v3 https://github.com/yearn/yearn-finance-v3/pull/755

Motivation and Context

Zaps let users deposit any token into a vault instead of just the underlying token. That improves UX and increases deposits and TVL.

Data shows that Zaps contribute to 40% of deposits on Yearn. Having Zap support for all vaults will grow Yearn's overall TVL.

Report: https://twitter.com/widolabs/status/1574762471314915334

How Has This Been Tested?

It was tested locally with the front end running the yalc'ed SDK.

Left to do

  • [x] [Bug] Some zaps show expected slippage of 100.0% because priceUSDC is missing in supported tokens
  • [x] Supported Vaults — make an API call instead of hardcoding
  • [x] Unit tests
  • [x] Show “Approve” not “Sign” on Wido Zaps

Screenshots (if appropriate):

image

mazurroman avatar Sep 30 '22 12:09 mazurroman

PR looks good overall :)

We should add a mechanism to dynamically switch precedence between different zap providers. So users of SDK can define the precedence of their choice if a vault is supported on multiple services. An idea could be setting a configuration on the SDK context, so this can be set when the SDK gets instantiated, and have a default if its not provided. Also that same mechanism would be useful to turn on/off a zap service provider (this can be done in separate PR though) for added privacy use cases.

something like:

export interface ContextValue {
  ...
  zaps?: {
    zapInWith: ZapInWith[];
    zapOutWith: ZapOutWith[];
  };
  ...
}

Where the first element in the array takes precedence over the next.

wdyt?

xgambitox avatar Oct 05 '22 20:10 xgambitox

PR looks good overall :)

We should add a mechanism to dynamically switch precedence between different zap providers. So users of SDK can define the precedence of their choice if a vault is supported on multiple services. An idea could be setting a configuration on the SDK context, so this can be set when the SDK gets instantiated, and have a default if its not provided. Also that same mechanism would be useful to turn on/off a zap service provider (this can be done in separate PR though) for added privacy use cases.

something like:

export interface ContextValue {
  ...
  zaps?: {
    zapInWith: ZapInWith[];
    zapOutWith: ZapOutWith[];
  };
  ...
}

Where the first element in the array takes precedence over the next.

wdyt?

Makes sense. Can you take a look at c82cc69ca8570f3e86596933bfe74c652c35659e ?

kernelwhisperer avatar Oct 07 '22 07:10 kernelwhisperer

With regards to deposit vs _deposit:

I audited _deposit and _withdraw and they correctly call the correct zap functions. E.g.: getDepositContractAddress and populateDepositTransaction are called, both of which are zap aware.

If we were to deprecate/remove deposit and withdraw we could also remove the functions zapInApprovalState, zapInApprovalTransaction, zapOutApprovalState and zapOutApprovalTransaction in both wido.ts and portals.ts, as they would not be used.

kernelwhisperer avatar Oct 07 '22 12:10 kernelwhisperer

With regards to deposit vs _deposit:

I audited _deposit and _withdraw and they correctly call the correct zap functions. E.g.: getDepositContractAddress and populateDepositTransaction are called, both of which are zap aware.

If we were to deprecate/remove deposit and withdraw we could also remove the functions zapInApprovalState, zapInApprovalTransaction, zapOutApprovalState and zapOutApprovalTransaction in both wido.ts and portals.ts, as they would not be used.

Ok cool then, lets leave them for now since its done and code looks good, and once we get out of alpha for this version I will deprecate and remove those functions. Thanks for looking into it!

xgambitox avatar Oct 08 '22 00:10 xgambitox