[WEB-1138-183] Feat: add multicall to getNormalizedValueUsdc #183
Conversation
yearn-linear-gh-sync
bot
changed the title
|
Linear reply from Fox Crypto: |
test/oracle.test.ts
Outdated
| let tokenAddress: string; | ||
| beforeAll(async () => { | ||
| const chainId = 1; | ||
| const rpcUrl = ""; |
There was a problem hiding this comment.
does using process.env.WEB3_PROVIDER work here?
There was a problem hiding this comment.
Definitely. I'll add it.
src/services/oracle.ts
Outdated
| * @param overrides | ||
| * @returns Usdc exchange rate (6 decimals) | ||
| */ | ||
| async getNormalizedValuesUsdc(tokens: Address[], amounts: Integer[], overrides: CallOverrides = {}): Promise<Usdc[]> { |
There was a problem hiding this comment.
This is great, and is a useful addition. I think the issue is referring to using an on-chain multicall contract so calls to the web3 provider can be reduced if you wanted to have a go at implementing that and using it here?
There was a problem hiding this comment.
Sure. I'll give it a shot and post updates.
There was a problem hiding this comment.
I'm working on integrating https://github.com/indexed-finance/multicall and currently working out some issues in how the library handles inputs. Is this what you had in mind?
There was a problem hiding this comment.
@jstashh multicall should work now with the indexed finance multicall lib, still need to figure out how to use the CallOverrides, getting the following error
types/values length mismatch (count={"types":2,"values":3}, value={"types":[{"name":null,"type":"address","indexed":null,"components":null,"arrayLength":null,"arrayChildren":null,"baseType":"address","_isParamType":true},{"name":null,"type":"uint256","indexed":null,"compon
ents":null,"arrayLength":null,"arrayChildren":null,"baseType":"uint256","_isParamType":true}],"values":["0x0bc529c00C6401aEF6D220BE8C6Ea1667F6Ad93e","1000000000000000000",{}]}, code=INVALID_ARGUMENT, version=abi/5.5.0)
once this is resolved, I can lint/prettify and add more comprehensive tests.
Also, not sure why I can't send the this.contract.abi directly to the multicall as it's an interface. ContractInterface vs Interface. I've resorted to directly creating an interface out of the exported abi. I don't think this is the correct way but it works for now. Can update based on feedback.
| async getNormalizedValuesUsdc(tokens: Address[], amounts: Integer[], overrides: CallOverrides = {}): Promise<any> { | ||
| const multi = new MultiCall(this.ctx.provider.read); | ||
| const inputs = []; | ||
| for (var i = 0, size = tokens.length; i < size; i++) { |
There was a problem hiding this comment.
nit: use let instead of var.
Just for readability you could do i < tokens.length, there is no need to set size before it since you are not going to be modifying tokens
There was a problem hiding this comment.
I can add these changes. Just picking up Typescript so I appreciate the pointers. I think this general approach will be scrapped in favor of adding a multicall method to the associated lens contract.
| [amount, amount] | ||
| ); | ||
| console.log(normValues); | ||
| expect(+normValues[0]).toBeGreaterThan(0); |
There was a problem hiding this comment.
this is a hack to convert strings to intergers, just to be consistent and more reliable we should do parseInt(normvalues[0], 10), so we always specify the base into which the string should be converted to
| @@ -39,15 +39,18 @@ | |||
| "eslint-plugin-simple-import-sort": "6", | |||
| "events": "3.3.0", | |||
| "size-limit": "7.0.4", | |||
| "ts-node": "^10.4.0", | |||
There was a problem hiding this comment.
should we try to keep dependencies versions fixed @jstashh ?
I know it is a requirement for yearn-finance-v3 repo
|
Closing since this will be implemented on the lens contracts instead |
Addressing #57 (comment), Support multicall getNormalizedValueUsdc for optimization
Quick solution that allows the return of normalized usdc values for a list of tokens and amounts.
.getNormalizedValueUsdc(...)for single calls.getNormalizedValuesUsdc(...)for batch callsthe calls are made offchain so if the intention was to use a multicall-like contract this will need to be revised. Just posting a quick solution for feedback.