-
Notifications
You must be signed in to change notification settings - Fork 26
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
Migrate findAccountsByPublicKey endpoint to QueryAPI #685
Migrate findAccountsByPublicKey endpoint to QueryAPI #685
Conversation
|
||
const { | ||
BRIDGE_TOKEN_FACTORY_ACCOUNT_ID = 'factory.bridge.near', | ||
GRAPHQL_URL = 'https://near-queryapi.api.pagoda.co/v1/graphql', |
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.
Does this handle testnet
queries as well?
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.
Ah that's a good point, it does not. We may need to parameterise this in ECS, and use ExplorerDB in testnet, QueryAPI doesn't support testnet yet.
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.
thanks for the heads up, I changed to have a fallback for non-mainnet environments or if no results from graphql
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.
Do you actually want a fallback for mainnet queries that return no results from graphql? If the DBs are actually going to be unavailable after April 30th then I imagine you'd want to know sooner than later that there's a disparity in what QueryAPI is returning. Also saves you the effort of needing to come back in < 2 weeks to remove the fallback 🙂
Are the testnet indexer DBs also going to be unavailable after April 30th? It's sounding like there will be an indeterminate gap in coverage for testnet. I don't know how severe the real impact of this would be, it's probably safe to assume there isn't a lot of account recovery, staking, etc. coming from testnet environments. It would almost certainly break parity for wallets supporting testnet but it might not be a dealbreaker in the short term.
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.
Good questions, yes we want to keep the fallback, April 30th is the desired deadline, but there are other endpoints that needs to be migrated before we turn off the DB instance.
Testnet indexer DBs will stay, there is no deadline or workstream yet to deprecate them.
src/middleware/indexer.js
Outdated
query: ` | ||
query access_keys_v1_by_public_key { | ||
dataplatform_near_access_keys_v1_access_keys_v1( | ||
where: {public_key: {_eq: \"${publicKey}\"}} |
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.
Doesn't look like the "
need to be escaped here
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.
thanks, removed
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.
LGTM from my side :)
Closes #684
Related to near/queryapi#325
The response continues the same using QueryAPI
QueryAPI response:
Explorer DB record: