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 historical capability #525

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Conversation

alfulinku
Copy link

@alfulinku alfulinku commented Aug 24, 2023

Hello Balancer team,

second attempt at adding historical capability to the SDK (see my other PR I made a few hours ago: balancer/balancer-sor#413 that I closed now), this time without modifying anything on the SOR part.

I figured that the "GraphQLArgs" already had the blockNumber available so I used it to fetch the pools from TheGraph at the block I wanted and used the parameter in "getOnChainBalances" to instantiate the multicaller with the override "blockTag".

This seem to be working but sadly I don't find valid results and I don't know why. You can try with the new example I wrote in examples/swaps/historicalSwaps.ts

when running it here's what is outputted:

1 WETH is 1641.539134 USDC using the most recent data
1 WETH was 2054.066786 USDC at block 17000000

While the current value is correct, the one at block 17M is not, should be around 1864 .

What's crazy to me is that my other PR on the SOR does almost the same thing and finds the correct results. I'm still investigating.

@alfulinku
Copy link
Author

alfulinku commented Aug 25, 2023

Ok i've made a few tests with the historical mode and found a few difference with the "onChainData.ts" here and the one on the SOR repository (where the historical price is valid). The main difference is that here we have this call added to the multicall for the ComposableStable pool type:

multiPool.call(
          `${pool.id}.actualSupply`,
          pool.address,
          'getActualSupply'
      );

It is not done this way in the onChainData.ts implementation on the SOR repository (in the SOR repository, the same calls are made for these types: 'Stable', 'MetaStable', 'StablePhantom' and 'ComposableStable', here in the SDK repository, we fetch actualSupply for ComposableStable)

When I run the code as is for the block 17 000 000, the swaps that are done are:

image

I don't understand that part precisely:
image

When I run the code without this

multiPool.call(
          `${pool.id}.actualSupply`,
          pool.address,
          'getActualSupply'
      );

I get a lot of warnings like
[WARN] ComposableStable missing Actual Supply: 0x2ba7aa2213fa2c909cd9e46fed5a0059542b36b00000000000000000000003a3

But the value I have is valid (it's the ETH price at the block 17 000 000) and the swaps are simpler, on three different pools
image

And the result is 1 WETH = 1861 USDC, which is the true result for the block 17 000 000.

I'm kinda stuck here because I don't know where the problem might be with the actualSupply of the ComposableStable pools.

Anyone could give me a hand?

@johngrantuk
Copy link
Member

Hey. I might be missing something in your explanation but we are using actualSupply in SOR, here?

Also, just to make you aware, we're going to be moving to a new onchain method using multicall3 soon. See this PR.

@alfulinku
Copy link
Author

Hey. I might be missing something in your explanation but we are using actualSupply in SOR, here?

Hey, thanks for answering. You're right I missed this previous code block because I focused on this one:
image

Well I'm back to step 0 then, trying to find why I'm not having the same results with the SOR and the SDK.

Also, just to make you aware, we're going to be moving to a new onchain method using multicall3 soon. See this PR.

OK I will wait for this PR to be merged to retry and hopefully find valid historical results.

@gmbronco
Copy link
Contributor

gmbronco commented Sep 1, 2023

Hey @alfulinku thanks for looking into this. fyi, we just merged the PR with multicall3.

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.

3 participants