-
Notifications
You must be signed in to change notification settings - Fork 43
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: Dynamic rpc handler #183
Conversation
September Update
sync to our ts-template
Sub 1s loading UI
chore: remove blacklist
Seems to be working well Gnosis permit for Anvil
|
Please rebase to a later commit. It makes it more difficult than it needs to be when we see commits that are already merged in. |
|
||
const PERMIT2_ADDRESS = "0x000000000022D473030F116dDEE9F6B43aC78BA3"; // same on all chains |
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.
if it's going to be removed i think we can leave it as a comment or add it to the readme for others dev to be aware
I'm going to close this PR and open a fresh one
|
what is your repo's link |
https://github.com/Keyrxng/web3-rpc-racer/tree/master It'll be my first NPM package, esm and cjs builds for fe and be. Everything needed exported properly ( at least locally (i don't see why tht would change pulling it from npm but yet to test that)) |
but looks crude, at least at first sight, you are planning to add more changes on PR opening or is ready to just PR and review? If it's a npm package i think a good move it's first upload to npm, as npm can also be remove, for then to test as a package itself |
That's the plan once I test pulling from npm if everything works then refactor where needed It's commonplace to have to import from the specific build dirs isn't it? So running jest I'm importing diretly from the cjs module and in the UI importing directly from the esm one
Exactly, so my PR to the UI code will be removing the logic and adding my test package which can be replaced when the UBQ one is hosted? or the pr will prove it works and the ubq can be published and I'll swap them before merge |
I think you are in route, yes, perhaps first local testing with npm link Context: https://chat.openai.com/share/92e44b12-cb21-42a1-a2cc-b3deb5c0909f but if you going to close this PR then open an issue where you could add more context |
everything working with local yarn link my friend yes Pav changed the original spec of the issue, this is the correct place for it but there's no repo to push the code for the npm module to |
so you're plan it's to PR a fresh ubq-repo for the npm package to then add more changes + submit to the npm repositories? |
I'll test npm before I open the new PR but if you could create a repo for me to fork and port the code into that would be sweet |
send the PR against this branch only https://github.com/ubiquity/web3-rpc-racer/tree/web3-rpc-racer-npm-branch and open an issue only to add more context
closing now |
Resolves ubiquity/rpc-handler#3
cancelToken
feature of axios to cancel any calls upwards of 500ms (this does log a(cancelled)
failure reason but that can be a hit and a miss with one rpc being really quick from one call to the next) and the original flickering issue the "blacklist" resolved has been resolved included in this prsync-erc-permits
pr and validated everything is workingIn the RPC video I'm using mainnet as an example as that has far more failed calls
claiming.mp4
rpc_dynamic_handling.mp4