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

feat: switch FOXy from web3 to ethers and JsonRpcBatchProvider #3508

Merged
merged 29 commits into from
Apr 25, 2023

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Dec 8, 2022

Description

  • Fully plumb through web
  • getXYZGas -> getXYZFees/Estimate vernacular in diffed methods
  • Scrutinize derp API and see if there's anything to improve to ease bringing it to web soonish
  • shouldUseEip1559Fees

This PR:

  • Switches from web3.js to ethers.js in investor-foxy, with revamped consumption to accommodate for it
  • bumps ethers to a unified 5.7.2 version across packages to ease the bundle size of having multiple transient/dependencies for ethers
  • uses the JsonRpcBatchProvider so that FOXy Ethereum JSON-RPC calls are batched
  • adds a 5% gas limit buffer to ensure Txs go through - debugged through Tenderly and both our node and ethers tend to slightly underestimate the gas limit - we were somehow lucky with web3.js because they use different calculations, but our estimations don't cut it anymore
  • handles EIP-1559 gas fees spec when supported by wallet
  • improves HDWallet/chain-adapters types through investor-foxy and web to be narrowed down to ETHWallet / EVMBaseAdapter
  • cleans up investor-foxy:
    • moves from ethers gasPrice and gasLimit estimations to unchained estimations, which should be more accurate
    • uses fees vernacular vs gas vernacular in fees estimation methods, which now return a FeeDataEstimate
    • removes buildTxToSign and use chain-adapters buildCustomTx instead
    • removes getGasPriceAndNonce, which isn't needed anymore now that we use chain-adapters so nonce will be correctly gotten there, and gas price is now useless because we use fees estimation

While at it, replaces all instances of JsonRpcProvider in web/packages to be JsonRpcBatchProvider, which will provide implicit batching whenever possible.

Notice

  • Have you followed the guidelines in our Contributing guide?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Pull Request Type

  • 🐛 Bug fix (Non-breaking Change: Fixes an issue)
  • 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

N/A

Risk

FOXy might be borked - tested deposit approval and deposit Txs through MM, and ran both instant and delayed withdraw ones through Tenderly, which were happy. Was unable to test claims

Testing

  • Fully regression test FOXy

Engineering

  • Some requests from investor-foxy will be implicitly batched, e.g getRebaseHistory:

image

Operations

  • ☝🏽

Screenshots (if applicable)

image

image

image

Copy link
Contributor Author

gomesalexandre commented Dec 8, 2022

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@gomesalexandre gomesalexandre force-pushed the feat_accomodate_new_investor_foxy_types branch from 9951649 to 660bba8 Compare January 3, 2023 17:57
@gomesalexandre gomesalexandre force-pushed the feat_accomodate_new_investor_foxy_types branch from 6e55bec to 7c55569 Compare April 23, 2023 17:40
@gomesalexandre gomesalexandre changed the title feat: accomodate for new investor-foxy ethers types feat: switch FOXy from web3 to ethers and JsonRpcBatchProvider Apr 23, 2023
@gomesalexandre gomesalexandre marked this pull request as ready for review April 24, 2023 11:15
@gomesalexandre gomesalexandre requested a review from a team as a code owner April 24, 2023 11:15
@gomesalexandre gomesalexandre force-pushed the feat_accomodate_new_investor_foxy_types branch from bd0187b to ed53a49 Compare April 25, 2023 17:26
@gomesalexandre gomesalexandre changed the base branch from develop to eip1559-fees April 25, 2023 17:48
@gomesalexandre gomesalexandre mentioned this pull request Apr 25, 2023
3 tasks
kaladinlight
kaladinlight previously approved these changes Apr 25, 2023
Base automatically changed from eip1559-fees to develop April 25, 2023 18:33
@gomesalexandre gomesalexandre merged commit 9d797f6 into develop Apr 25, 2023
@gomesalexandre gomesalexandre deleted the feat_accomodate_new_investor_foxy_types branch April 25, 2023 20:18
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