-
Notifications
You must be signed in to change notification settings - Fork 301
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
Implements pancakeswap #1322
Conversation
|
There was a problem hiding this 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" |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
export const ENV_ADDRESS_PROVIDER = 'ADDRESS_PROVIDER' | ||
export const ENV_BLOCKCHAIN_NETWORK = 'BLOCKCHAIN_NETWORK' |
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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.
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' |
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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
process.env.CACHE_ENABLED = 'false' |
Hey @lucaswxp, are you interested in getting this PR updated, or should we close this PR? |
Closing this PR for now, but feel free to re-open once it's ready for re-review! |
Closes #824
Description
......
Steps to Test
This will update snapshot, be careful. Some times timeouts may occur due to network slowdown, just try again.
Quality Assurance
<ADAPTER_PACKAGE>/schemas/env.json
and<ADAPTER_PACKAGE>/README.md
infra-k8s
configuration file.feature/x
,chore/x
,release/x
,hotfix/x
,fix/x
) or is created from Clubhouse/Shortcut