Skip to content

fix: added batch call + delete useless on-chain call#75

Merged
hzhu merged 3 commits into0xProject:mainfrom
lucaspluchon:optimization
Mar 21, 2025
Merged

fix: added batch call + delete useless on-chain call#75
hzhu merged 3 commits into0xProject:mainfrom
lucaspluchon:optimization

Conversation

@lucaspluchon
Copy link
Copy Markdown
Contributor

@lucaspluchon lucaspluchon commented Feb 6, 2025

Improve efficiency and reliability 🏎️

Replaced await publicClient.getChainId() with direct access to publicClient.chain.id for a synchronous, faster retrieval of the current chain ID.

Batched network requests for traceCall, getTransaction, and getTransactionReceipt using Promise.all to reduce latency and improve performance. This avoids an unnecessary async call to fetch chain ID when the value is already available. Running all RPC calls in parallel speeds up parseSwap, especially under high-load or latency-sensitive conditions.

@hzhu hzhu self-requested a review February 11, 2025 17:34
Copy link
Copy Markdown
Member

@hzhu hzhu left a comment

Choose a reason for hiding this comment

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

PR looks good! Just a couple of tweaks before merging:

  • This seems more suited for a patch release rather than a minor. Could you update the commit message to fix: added batch call + delete unused on-chain call?
  • Also, could you remove .idea?

@lucaspluchon lucaspluchon changed the title feat: added batch call + delete useless on-chain call fix: added batch call + delete useless on-chain call Feb 18, 2025
@hzhu
Copy link
Copy Markdown
Member

hzhu commented Feb 21, 2025

@lucaspluchon I noticed that some tests are failing:

  • parse a swap on BNB Chain (BNB for USDT) with smart contract wallet
  • parse a swap on BNB Chain (USDT for BNB) with smart contract wallet
  • throws an error for unsupported chains

I'll look into these when I have time, but do you know why they’re failing? I should add testing instructions to the README, but for now, you can add an Alchemy API key (with debug_trace support) and run npm test. If you’re unable to, I can do it when I have time.

Thanks again for the PR!

@hzhu
Copy link
Copy Markdown
Member

hzhu commented Mar 21, 2025

This PR is good. It's failing for a different reason: #81

@hzhu hzhu merged commit 544dc8c into 0xProject:main Mar 21, 2025
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.

2 participants