-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
FallbackProvider should spawn quorum/weight
runners, not quorum
runners
#4298
Labels
enhancement
New feature or improvement.
fixed/complete
This Bug is fixed or Enhancement is complete and published.
v6
Issues regarding v6
Comments
staltz
added
investigate
Under investigation and may be a bug.
v6
Issues regarding v6
labels
Aug 7, 2023
ricmoo
added
enhancement
New feature or improvement.
on-deck
This Enhancement or Bug is currently being worked on.
next-patch
Issues scheduled for the next arch release.
and removed
investigate
Under investigation and may be a bug.
labels
Aug 8, 2023
Looking at this, I agree. Working on this now. :) |
ricmoo
added a commit
that referenced
this issue
Nov 24, 2023
Fixed in 6.9.0. Thanks! :) |
ricmoo
added
fixed/complete
This Bug is fixed or Enhancement is complete and published.
and removed
on-deck
This Enhancement or Bug is currently being worked on.
minor-bump
Planned for the next minor version bump.
next-patch
Issues scheduled for the next arch release.
labels
Nov 27, 2023
Woodpile37
pushed a commit
to Woodpile37/ethers.js
that referenced
this issue
Jan 14, 2024
Woodpile37
pushed a commit
to Woodpile37/ethers.js
that referenced
this issue
Jan 14, 2024
Woodpile37
pushed a commit
to Woodpile37/ethers.js
that referenced
this issue
Jan 14, 2024
rrw-zilliqa
added a commit
to Zilliqa/ethers.js
that referenced
this issue
Apr 29, 2024
* docs: fixed typo in jsdocs for Wallet.createRandom (ethers-io#4461) * admin: added diff scripts for build page * admin: updated dist files * Added safe and finalized provider events (ethers-io#3921). * tests: bumped Node versions for testing (ethers-io#4451) * admin: style fix (ethers-io#4356) * More robust FallbackProvider broadcast (ethers-io#4186, ethers-io#4297, ethers-io#4442). * Account for provider config weight when kicking off a request in FallbackProvider (ethers-io#4298). * Fixed ParamType formatting causing bad tuple full and minimal ABI output (ethers-io#4329, ethers-io#4479). * Added Base network to AlchemyProvider (ethers-io#4384). * Add auto-detected static network support to providers and allow customizing socket provider options (ethers-io#4199, ethers-io#4418, ethers-io#4441). * Use provider-specified suggested priority fee when available, otherwise fallback onto existing logic of 1 gwei (ethers-io#4463). * admin: updated dist files * admin: update changelog after build-clean * docs: Fixed some grammar in getting-started (ethers-io#4486, ethers-io#4487, ethers-io#4488) * Fix uncatchable issue when sending transactions over JSON-RPC and provide some retry-recovery for missing v (ethers-io#4513). * admin: update dist files * Fix Base58 padding for string representation of binary data (ethers-io#4527). * admin: updated dist files * Limit decoded result imflation ratio from ABI-encoded data (ethers-io#4537). * admin: updated dist files * Better debugging output on fetch errors. * docs: added StaticJsonRpcProvider to migration docs * Fixed typo in Error string (ethers-io#4539). * Fix EIP-712 type aliases for uint and int (ethers-io#4541). * Added additional sepolia testnets. * Updated third-party provider network URLs (ethers-io#4542). * admin: updated dist files * Fixed normalization and abstracted EIP-712 Array parsing (ethers-io#4541). * admin: updated dist files * tests: added testing for correct thrid-party URLs * Updated thrid-part provider URLs for QuickNode. * tests: rename test suite to follow naming convention * admin: updated dist files * Normalize EIP-712 types before computing the payload (ethers-io#4541). * tests: add tests for EIP-712 payload aliases * admin: updated dist files --------- Co-authored-by: Richard Moore <[email protected]>
This was referenced May 22, 2024
This was referenced Jun 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
New feature or improvement.
fixed/complete
This Bug is fixed or Enhancement is complete and published.
v6
Issues regarding v6
Ethers Version
6.6.2
Search Terms
FallbackProvider
Describe the Problem
For
eth_getLogs
requests, we would like to send requests sequentially to providers, in such a way that if thei-th
request is successful, then the(i+1)-th
request is NOT sent.Our understanding is that (according to our chat last week) "it is supposed to call parallel up to quorum. So if the quorum is 2 and each provider has a weight of 1, there should be two in-flight at a time. After stallTimeout the next gets kicked off, to account for the overly common case of backends not responding and also not hanging up."
Assuming all our providers have weight 2 and the quorum is 2:
Expected: each provider ("runner") is attempted one at a time, proceeding to the next one only if the request failed.
Actual: 2 runners are simultaneously attempted.
The culprit seems to be this line of code:
ethers.js/src.ts/providers/provider-fallback.ts
Lines 732 to 734 in c5cb7cd
Notice how
this.quorum
runners are started, independently of the weight of each provider runner. I believe that we should instead count the total-so-far weight of runners started and stop the for-loop if enough has been reached for the quorum.Code Snippet
No response
Contract ABI
No response
Errors
No response
Environment
node.js (v12 or newer), Browser (Chrome, Safari, etc), React Native/Expo/JavaScriptCore
Environment (Other)
No response
The text was updated successfully, but these errors were encountered: