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

Implements pancakeswap #1322

Closed
wants to merge 3 commits into from
Closed

Conversation

lucaswxp
Copy link

Closes #824

Description

  • Integrates pancaswap dex contract
  • Inserted presetTokens overrides for Binance Smart Chain addresses (source: Coinmarketcap API)
  • Tests rewritten to fit BSC tokens
    ......

Steps to Test

This will update snapshot, be careful. Some times timeouts may occur due to network slowdown, just try again.

RPC_URL=https://bsc-dataseed1.binance.org:443 RECORD=1 yarn jest --updateSnapshot -- packages/sources/pancakeswap

Quality Assurance

  • [ X ] If a new adapter was made, or an existing one was modified so that its environment variables have changed, update the relevant <ADAPTER_PACKAGE>/schemas/env.json and <ADAPTER_PACKAGE>/README.md
  • If a new adapter was made, or an existing one was modified so that its environment variables have changed, update the relevant infra-k8s configuration file.
  • The branch naming follows git flow (feature/x, chore/x, release/x, hotfix/x, fix/x) or is created from Clubhouse/Shortcut
  • [ X ] This is related to a maximum of one Clubhouse/Shortcut story or GitHub issue
  • [ X ] Types are safe (avoid TypeScript/TSLint features like any and disable, instead use more specific types)

@changeset-bot
Copy link

changeset-bot bot commented Dec 21, 2021

⚠️ No Changeset found

Latest commit: 949c5f3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@boxhock boxhock self-requested a review January 3, 2022 15:39
@boxhock boxhock self-assigned this Jan 3, 2022
Copy link
Contributor

@boxhock boxhock left a comment

Choose a reason for hiding this comment

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

Thank you for your PR! 😄

Left some comments below:

"PASTRYPUNKS": "0x1BF571b3d5485109e59d002e9765104e4E238BfD",
"PTA": "0x3843f234b35a311e195608d32283a68284b3c44d",
"POR": "0x9000Cac49C3841926Baac5b2E13c87D43e51B6a4",
"PLG": "0xDAd3ABce6Fb87FA0355203b692723A7EE8aa9D63"
Copy link
Contributor

Choose a reason for hiding this comment

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

How did you compile this list?

I'm a bit hesitant approving this PR when I cannot verify that there aren't any incorrect mappings here. Potentially we can reduce this list to the most common tickers.

@@ -0,0 +1,69 @@
# Chainlink External Adapter for Curve.fi
Copy link
Contributor

Choose a reason for hiding this comment

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

This README needs to be updated

Comment on lines +8 to +9
export const ENV_ADDRESS_PROVIDER = 'ADDRESS_PROVIDER'
export const ENV_BLOCKCHAIN_NETWORK = 'BLOCKCHAIN_NETWORK'
Copy link
Contributor

Choose a reason for hiding this comment

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

These need to be included in README and the schema file

export const ENV_ADDRESS_PROVIDER = 'ADDRESS_PROVIDER'
export const ENV_BLOCKCHAIN_NETWORK = 'BLOCKCHAIN_NETWORK'

export const DEFAULT_BLOCKCHAIN_NETWORK = 'bsc'
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the full name. Same goes for preset token addresses.

Suggested change
export const DEFAULT_BLOCKCHAIN_NETWORK = 'bsc'
export const DEFAULT_BLOCKCHAIN_NETWORK = 'binance-smart-chain'


export const DEFAULT_BLOCKCHAIN_NETWORK = 'bsc'
export const DEFAULT_ENDPOINT = 'crypto'
export const ROUTER_CONTRACT = '0x10ED43C718714eb63d5aA57B78B54704E256024E'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this configurable in any way?

beforeAll(async () => {
server = await startServer()
req = request(`localhost:${(server.address() as AddressInfo).port}`)
process.env.CACHE_ENABLED = 'false'
Copy link
Contributor

Choose a reason for hiding this comment

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

We should aim to have all new adapters work without this set

Suggested change
process.env.CACHE_ENABLED = 'false'

@boxhock
Copy link
Contributor

boxhock commented Feb 21, 2022

Hey @lucaswxp, are you interested in getting this PR updated, or should we close this PR?

@boxhock
Copy link
Contributor

boxhock commented Feb 23, 2022

Closing this PR for now, but feel free to re-open once it's ready for re-review!

@boxhock boxhock closed this Feb 23, 2022
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.

Pancakeswap: use DEX on-chain data
2 participants