Pull liquidation info from the new subgraph #1204

noisekit · 2022-08-01T07:33:33Z

POC

works on optimism-mainnet
works on optimism-kovan
TODO: periodical checks on timer
TODO: support liquidation subgraphs on mainnet

NOTE: I don't think we should put all the subgraphs into a single repo, maybe monorepo? So we can treat each of them as a separate package.
Right now I just ran all the queries directly from staking, but we could collocate those queries with the subgraph code itself as an option...

vercel · 2022-08-01T07:33:35Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
staking	✅ Ready (Inspect)	Visit Preview	Sep 2, 2022 at 0:20AM (UTC)

fritzschoff · 2022-08-01T07:35:27Z

components/BannerManager/Liquidation/useLiquidationOptimismSubgraph.ts

+	if (!isSuccess) {
+		return 0;
+	}
+	// @ts-ignore I give up


Spent half of today trying to type this useQuery thing 🤦‍♂️

Made a PR showing how you can type it:
#1205

fritzschoff · 2022-08-01T07:38:47Z

components/BannerManager/Liquidation/useLiquidation.ts

+	const deadline =
+		optimismDeadline > 0
+			? optimismDeadline
+			: Number(liquidationDeadlineForAccount.toString()) * 1000;


if liquidationDeadlineForAccount is wei. I guess you can directly do liq.toNumber() * 1000

This was a copy of previous code, but yeah makes sense to avoid this

0xjocke · 2022-08-01T13:52:14Z

components/BannerManager/Liquidation/useLiquidationOptimismSubgraph.ts

+function getEndpoint(networkName: String) {
+	switch (networkName) {
+		case 'kovan-ovm':
+			return 'https://api.thegraph.com/subgraphs/name/noisekit/liquidator-optimism-kovan';


It would be good to add this subgraph to https://github.com/synthetixio/synthetix-subgraph/ instead.

We should probably also remove the old liquidation graph: https://github.com/Synthetixio/synthetix-subgraph/blob/main/subgraphs/liquidations.js

If it's added to that repo we can then pull/refresh the subgraph main.json file in the queries lib, and it will generate typed hooks for us .

It's a bit more work. But also good for you to get to know that repo. And convenient when upgrades to the liquidation contract happens, good to have one place to remember to update

The problem with existing repo is that it is not really set up well for testing and other things. The graph generates a different structure now and fitting it with the old way does not make much sense to me. I would rather keep it either in a separate repo or have another monorepo for subgraphs.

Also when we do a deploy from that repo it deployes the subgraphs to all supported network (which very sson does not include kovan). And the hooks will use the correct network url in the hooks automatically from context.

And the hooks will use the correct network url in the hooks automatically from context.

That's the part I would like to avoid. We already depend too much on abstractions that cannot be untied right now, trying to run all queries without relying on any of the existing libs and context that includes all the contracts.

That's kind of the point of this POC

The graph generates a different structure now and fitting it with the old way does not make much sense to me

You mean the liquidation graph? I think the current liquidation graph in unused.

And the hooks will use the correct network url in the hooks automatically from context.

That's the part I would like to avoid. We already depend too much on abstractions that cannot be untied right now, trying to run all queries without relying on any of the existing libs and context that includes all the contracts.

That's kind of the point of this POC

Yea I guess to would be nice to decouple it a little. But having hooks generated make sense to me, since with graphql you can ask for what ever parameters you need either way.

Also, I guess decoupling it from the queries lib isn't really related to having the subgraph definition live in synthetix-subgraphs? That repo is essentially a monorepo for all our subgraphs already? And it has some smart things in the deployment script. Using the "Grafting" feature, deploying to all supported synthetix networks, deploying the mainnet graph to the decentralised network. Not saying it's perfect put there's quite a few useful features there.

There are few useful features, yes, but we need to talk about it properly. They are heavily outdated now and I don't want to limit myself to an old version of the graph tooling. At the same time I cannot afford upgrading it to the modern standard now either. The only practical solution was to have subgraph created separately.

It is a lot of context to learn already and increasing it with all the extra tooling written with old graph in mind - would be prohibitively hard.

BIG NOTE here: this pr is a draft, it is not meant to be merged like this anyway. I am simply testing that things work and was not expecting any reviews yet at all (tho really appreciate the input and help, will make it easier to talk about it later)

another thing is that we cannot just do full automated switching of networks as we have differences between contracts sometimes, and those contracts are not compatible. But that's whole another topic. I do not believe that full automation in a way we have it now in synthetix-subgraph is fully usable and covers all cases anymore. Contracts changed a lot, the graph tooling changed too.

Yea know it's a draft. Spark some good convos either way. And I do agree with your points.. I guess what's annoying is that we have some quite tricky bugs related to synth supply and SNXHolder that are more important than the liquidation subgraph. So the main benefit of you starting to look into the synthetix-subgraphs repo would be to help debugging those..

With that said, maybe the easiest way to resolve those bugs (in handleTransferSynth) is to start with a fresh subgraph...

more important than the liquidation subgraph

Yep 100%, this is a learning exercise first not a full on feature work. Would be great if it can be helpful in a more feature-practical sense, but this was not a main goal

With that said, maybe the easiest way to resolve those bugs (in handleTransferSynth) is to start with a fresh subgraph...

Haha, literally was my plan I mentioned today over standup ;) I planned to give a quick try and if it does not help - dig into existing one harder. Will need to learn a lot of smart contract context for it either way, might as well keep it simple at first to reduce the learning surface.

0xjocke · 2022-08-01T14:13:56Z

NOTE: I don't think we should put all the subgraphs into a single repo, maybe monorepo? So we can treat each of them as a separate package.
Right now I just ran all the queries directly from staking, but we could collocate those queries with the subgraph code itself as an option...

I missed this note before adding my other comments. I do think the benefits I described above is worth having it in synthetix-subgraph.. But happy to have a chat about it

0xjocke · 2022-08-01T15:16:44Z

TODO: periodical checks on timer

A final note, react query have support for this :)

		{
			enabled: Boolean(walletAddress && network?.name),
			refetchInterval: 3000,
			...queryOptions,
		}

Ts example of hook

Pull liquidation info from the new subgraph

3fdb08b

noisekit requested a review from a team August 1, 2022 07:33

noisekit self-assigned this Aug 1, 2022

fritzschoff reviewed Aug 1, 2022

View reviewed changes

0xjocke reviewed Aug 1, 2022

View reviewed changes

Example of how to type react query hooks

2680926

Merge pull request #1205 from Synthetixio/react-query-ts

b15d415

Ts example of hook

vercel bot deployed to Preview August 2, 2022 00:20 View deployment

Pull liquidation info from the new subgraph #1204

Are you sure you want to change the base?

Pull liquidation info from the new subgraph #1204

Uh oh!

Conversation

noisekit commented Aug 1, 2022

Uh oh!

vercel bot commented Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

noisekit Aug 1, 2022 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xjocke Aug 1, 2022 • 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!

noisekit Aug 1, 2022 • 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!

0xjocke Aug 1, 2022 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xjocke Aug 1, 2022 • 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!

noisekit Aug 1, 2022 • 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!

0xjocke commented Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xjocke commented Aug 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vercel bot commented Aug 1, 2022 •

edited

Loading

noisekit Aug 1, 2022 •

edited

Loading

0xjocke Aug 1, 2022 •

edited

Loading

noisekit Aug 1, 2022 •

edited

Loading

0xjocke Aug 1, 2022 •

edited

Loading

0xjocke Aug 1, 2022 •

edited

Loading

noisekit Aug 1, 2022 •

edited

Loading

0xjocke commented Aug 1, 2022 •

edited

Loading