-
Notifications
You must be signed in to change notification settings - Fork 123
[CON-659] General audit #145
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| setBalance(assets) | ||
| console.log(assets) | ||
| setBalance(result) |
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.
Incorrect response property access breaks token balance functionality
The response destructuring was changed from extracting assets to extracting result, which breaks the wallet token balance feature. The QuickNode qn_getWalletTokenBalance API returns a response containing an assets array property with the token balances. When using ethers.js provider.send(), the JSON-RPC result is returned directly, so the code should extract assets from data, not result. This change causes balance to be set to undefined, preventing tokens from being displayed.
mikejhale
left a comment
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.
@0xsergen Made a few minor suggestions.
CONTRIBUTING.md
Outdated
| - **Title** from the first `# Heading` in your README.md | ||
| - **Description** from `package.json`'s `description` field | ||
|
|
||
| Optional: Install the pre-commit hook to update automatically on each commit: |
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 mention the pre-commit hook earlier in Setup, but here it's mentioned again as optional.
README.md
Outdated
| cd qn-guide-examples/<example-folder> | ||
| # Install dependencies for the chosen runtime (e.g., npm install, yarn, pip, go mod download) | ||
| cp .env.example .env # if provided | ||
| # Update .env with your Quicknode endpoint keys and any required secrets |
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.
I think this should be
Update .env with your Quicknode endpoint URLs and any required secrets
README.md
Outdated
|
|
||
| ## Community & Support | ||
|
|
||
| - Join the Quicknode Discord for questions and collaboration: https://discord.gg/quicknode |
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.
I would make "Quicknode Discord" the link vs a raw URL
README.md
Outdated
|
|
||
| - Join the Quicknode Discord for questions and collaboration: https://discord.gg/quicknode | ||
| - Repo-specific bugs or requests: open a GitHub issue so we can track it. | ||
| - Need help with Quicknode products? Reach out via support: https://support.quicknode.com/ |
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.
Same here, link "Reach out via support"
| const params = [{ | ||
| wallet: wallet, | ||
| } | ||
| }] |
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.
Incorrect API parameter format breaking NFT Explorer
The API calls to Quicknode methods are now passing parameters as an array wrapped around an object instead of passing the object directly. The reference scripts in scripts/getWalletTokenBalance.js and similar files show these methods expect provider.send(method, {...params}) not provider.send(method, [{...params}]). This breaks all three pages since the API will reject the malformed parameters.
Additional Locations (2)
mikejhale
left a comment
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.
Looks good to me!
Note
Rebrand and docs overhaul
README.mdwith badges, usage guidance, and auto-generated project directory; adds comprehensiveCONTRIBUTING.md; updatesCLAUDE.mdNew/updated examples and guides
Streams/ai-bot-discordand updates Streams setup docs; multiple minor README fixes across AI, EVM MCP, Solana, Base, Bitcoin, Console APINFT Collection Explorer improvements
alt), and UI styling; updatestsconfig.jsontoreact-jsxRemovals and misc tweaks
enhanced-apis/get-receipts-by-addressexampleWritten by Cursor Bugbot for commit d156d04. This will update automatically on new commits. Configure here.