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.

Sign up for GitHub

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

Jump to bottom

Adaptation of Subgraph for Saucerswap v2 #41

Merged
merged 9 commits into from
Apr 18, 2024
Merged

Adaptation of Subgraph for Saucerswap v2 #41

merged 9 commits into from
Apr 18, 2024

Conversation

AlfredoG87
Copy link
Contributor

Description:
Changed the original for and adapted it to support saucerswap v2 contracts and ecosystem

Related issue(s):

Fixes #30

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@AlfredoG87 AlfredoG87 requested a review from Nana-EC March 1, 2024 04:00
@AlfredoG87 AlfredoG87 self-assigned this Mar 1, 2024
@AlfredoG87 AlfredoG87 added the Feature Enhancement Enhancing an existing feature driven by business requirements. Typically backwards compatible. label Mar 1, 2024
@AlfredoG87 AlfredoG87 added this to the 0.2.0 milestone Mar 1, 2024
@AlfredoG87 AlfredoG87 force-pushed the port-subgraph-saucerswapv2 branch from f136d9c to d6d983c Compare March 28, 2024 20:27
Nana-EC
Nana-EC reviewed Mar 28, 2024
View reviewed changes
Copy link

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good.
Some comments

subgraphs/saucerswap/v2/package.json Outdated
"create": "graph create saucerswap-v2 --node http://127.0.0.1:8020",
"deploy-local": "graph deploy saucerswap-v2 --debug --ipfs http://localhost:5001 --node http://127.0.0.1:8020",
"deploy-stg": "graph deploy saucerswap-v2 --debug --ipfs https://api.thegraph.com/ipfs/ --node http://127.0.0.1:8020",
"remove": "graph remove saucerswap-v2 --node http://127.0.0.1:8020"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: should we have a watch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not consider it, will investigate and get back 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What the --watch flag does, it that the graph-cli will keep watching the files, and when detecting a change, it will automatically redeploy to the server.

I don't think we need it, since is useful only during intensive development and test phases, but is not recommended for production environment.

subgraphs/saucerswap/v2/src/mappings/position-manager.ts Outdated
import { convertTokenToDecimal, loadTransaction } from '../utils'

const excludedBlocks: BigInt[] = [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: do we still need to exclude and if so we should say why these blocks are being excluded.
Could the missing blocks ever be reindexed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try after the improvements we've done to see if we don't need this anymore. 👍

Copy link
Contributor Author

@AlfredoG87 AlfredoG87 Apr 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did took another look at this and was able to find the culprit of some blocks failing, the issue was fixed on this PR of the relay:
hashgraph/hedera-json-rpc-relay#2333

It was deployed to mainnet and I have re-indexed the subgraph and now is working without the need to hardcode "faulty" blocks.

✅ Fixed!

subgraphs/saucerswap/v2/src/mappings/position-manager.ts Show resolved Hide resolved
subgraphs/saucerswap/v2/src/utils/constants.ts Outdated
@@ -3,7 +3,7 @@ import { BigInt, BigDecimal, Address } from '@graphprotocol/graph-ts'
import { Factory as FactoryContract } from '../types/templates/Pool/Factory'

export const ADDRESS_ZERO = '0x0000000000000000000000000000000000000000'
export const FACTORY_ADDRESS = '0x1F98431c8aD98523631AE4a59f267346ea31F984'
export const FACTORY_ADDRESS = '0x00000000000000000000000000000000003c3951'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: which env is this for and is it reliably going to be that address?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for mainnet, but I will provide a way to parametrized it based on .env configuration and using envsub to substitute those fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Completed

subgraphs/saucerswap/v2/src/utils/pricing.ts Outdated
@@ -4,42 +4,28 @@ import { Bundle, Pool, Token } from './../types/schema'
import { BigDecimal, BigInt } from '@graphprotocol/graph-ts'
import { exponentToBigDecimal, safeDiv } from '../utils/index'

const WETH_ADDRESS = '0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2'
const USDC_WETH_03_POOL = '0x8ad599c3a0ff1de082011efddc58f1908eb6e6d8'
const WHBAR_ADDRESS = '0x0000000000000000000000000000000000163b5a'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this is mainnet?
Do we need to support testnet or how might we configure this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes is mainnet, will work on .env file that provides this values based on network chosen

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done the needed changes so based on the deployed network: mainnet or testnet it takes some or other values, all is done by .env on the subgraph yaml creation using a template, and on constants.ts file based on the network the constants are populated with their respective networks, effectively making this subgraph multi-network.

✅ Completed!

subgraphs/saucerswap/v2/src/utils/pricing.ts Outdated
if (usdcPool !== null) {
return usdcPool.token0Price
return usdcPool.token0Price //? maybe is token1?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: were you able to confirm?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I were able to confirm, is not, is token0 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will remove the comment 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ removed!

subgraphs/saucerswap/v2/src/utils/tick.ts
if (feeTier.equals(BigInt.fromI32(10000))) {
return BigInt.fromI32(200)
}
if (feeTier.equals(BigInt.fromI32(3000))) {
return BigInt.fromI32(60)
}
if (feeTier.equals(BigInt.fromI32(1500))) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: why the need for the extra tier?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a change that saucerswap confirmed they introduced.

@AlfredoG87 AlfredoG87 requested review from a team and beeradb as code owners April 14, 2024 21:47
@AlfredoG87
Changed the original for and adapted it to support saucerswap v2 cont…
bef7094 
…racts and ecosystem

Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87
Added missing files and improved some other files
29f72a1 
Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87
fix format of files
3d01bdb 
Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87
improve performance by initializing only once the excluded blocks and…
039d272 
… improving on the check condition.

Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87
removing excluded blocks since fixed the revert eth_call issue not be…
4117536 
…ing recognized by thegraph node.

Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87
Improvements of network parametrization and removed error handling th…
c63a236 
…at is no longer needed

Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87
Adding support for both mainnet and testnet with parametrized deploym…
57ad1f4 
…ent.

also added .env example for both mainnet and testnet

Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87
Updated the documentation instructions and improved env examples
348da8b 
Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87
removed not needed comment anymore
89c826c 
Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87 AlfredoG87 force-pushed the port-subgraph-saucerswapv2 branch from 0ea4887 to 89c826c Compare April 18, 2024 21:09
@AlfredoG87 AlfredoG87 merged commit 12545a8 into main Apr 18, 2024
5 checks passed
@AlfredoG87 AlfredoG87 deleted the port-subgraph-saucerswapv2 branch April 18, 2024 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nana-EC Nana-EC Nana-EC approved these changes

@beeradb beeradb Awaiting requested review from beeradb beeradb is a code owner

@david-bakin-sl david-bakin-sl Awaiting requested review from david-bakin-sl david-bakin-sl is a code owner automatically assigned from hashgraph/hedera-smart-contracts

Assignees

@AlfredoG87 AlfredoG87

Labels
Feature Enhancement Enhancing an existing feature driven by business requirements. Typically backwards compatible.
Projects
None yet
Milestone
0.2.0
Development

Successfully merging this pull request may close these issues.

Include Subgraph for SaucerSwap
2 participants
@AlfredoG87 @Nana-EC