-
Notifications
You must be signed in to change notification settings - Fork 9
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: Web3 RPC Handler (npm package) #1
feat: Web3 RPC Handler (npm package) #1
Conversation
dont hold back @molecula451 |
.github/workflows/build.yml
Outdated
@@ -28,8 +28,8 @@ jobs: | |||
yarn | |||
yarn build | |||
# env: # Set environment variables for the build | |||
# SUPABASE_URL: "https://wfzpewmlyiozupulbuur.supabase.co" | |||
# SUPABASE_ANON_KEY: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6IndmenBld21seWlvenVwdWxidXVyIiwicm9sZSI6ImFub24iLCJpYXQiOjE2OTU2NzQzMzksImV4cCI6MjAxMTI1MDMzOX0.SKIL3Q0NOBaMehH0ekFspwgcu3afp3Dl9EDzPqs1nKs" | |||
# SUPABASE_URL: "https://wfzpewmlyiozupulbuur.supabase.co" |
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 guess it would be best not to publish these
build/esbuild-build.ts
Outdated
]; | ||
import chainlist from "../lib/chainlist/constants/extraRpcs"; | ||
|
||
const typescriptEntries = ["rpc-handler.ts", "constants.ts", "handler.ts", "services/RPCService.ts", "services/StorageService.ts"]; |
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'm surprised it passed kebab case test?
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.
ts-template doesn't have the kebab workflow in it by default, pay.ubq does tho. Just copy paste it over?
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.
ts-template doesn't have the kebab workflow in it by default, pay.ubq does tho. Just copy paste it over?
bring it over https://github.com/ubiquity/web3-rpc-racer/actions/runs/8123062862/job/22205701380?pr=1
dist/cjs/constants.d.ts
Outdated
Anvil = 31337, | ||
} | ||
export declare enum Tokens { | ||
DAI = "0x6b175474e89094c44da98b954eedeac495271d0f", |
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.
It would make more sense to associate tokens per network in case they aren't matching
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.
Makes sense, should I add any more tokens in btw?
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.
Not for now but I suppose this should be passed in as constructor arguments instead so that the application can handle this?
src/services/StorageService.ts
Outdated
export class StorageService { | ||
static getLatencies(env: string): Record<string | number, number> { | ||
if (env === "browser") { | ||
return JSON.parse(localStorage.getItem("rpcLatencies") || "{}"); |
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 don't think local storage modification should be within this module.
Instead the results should be stored on the object. The end user should manually handle local storage if it's necessary I think.
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.
So it's both, I felt that would be better. The invoking logic doesn't need to do anything besides initialize and the rest is taken care of, it saves having to repeat code across multiple UI's and for node envs the state is still saved onto the object
what would you suggest, that I remove this and have each UI handle it's own storage?
I'd need to expose the runtimeRPCSs and latencies, then the UI would pull those from the object after init and then save them into local. Anytime getFastestProvider is called the UI will pull from local, pass them into the handler then save the output to local again?
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.
You can consider decoupling the logic if you don't want each UI to handle it redundantly. Just have the localStorage logic in a separate method.
However I would argue that it may not be necessary to use localStorage at all. If we do a Promise.race once upon page load every time that's probably not bad and will accommodate for changing rpc conditions on a sporadic basis.
Since all the RPC endpoints fire off in parallel we shouldn't have any bottle necks initializing like this.
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.
Like a boolean passed into init enabling local storage handling automatically then methods that just set/get local storage?
The only problem I think is, with every page refresh the class is re-initialized and because the class state of latencies is <= 1 then it'll always race all RPCs. On init I pull the latencies and cache refresh count from localStorage and with passing the refresh arg in on init the UI is able to maintain a cache, so I think local is needed unless I'm not seeing something
We already initialize like you say essence just with the cached rpcs (unless refreshing the list), I can't think how we'd persist state across page visits without using local
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.
Yes I just think its not really necessary and adds complexity to making this polymorphic, or also being able to run in node.
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.
@Keyrxng Pls add a proper readme file which describes:
- what the package does
- how to install it
- how to use it
- how to run tests
idk what the official name will be but I just went with rpc-handler |
build still fails @Keyrxng |
Added the kebab scripts (forgot to when I added the workflow). Deploying to cloudflare should be removed as there is nothing to deploy i.e no static folder which fails the run and I think Knip is still not fully set up yet, is that right? |
yes you can skip/ignore knip |
Is the run failing maybe because the workflow isn't on the branch it's being merged into? I have no idea why it would fail when it's a 1:1 copy of the stuff on the pay.ubq repo |
you mean the build.yml? |
The kebab case workflow, the debug run was a big help |
yeah we are on a permissions issue: cc @gitcoindev |
This is destructive and renames files so lets not use it.
You are importing the unbuilt version, import from /dist/cjs it should work |
Trying to run this code:
Getting this error:
What am I doing wrong? |
Thanks, I'll try |
extraRpcs it's part of chainlist submodule make sure you git recursively clone or init the module |
I've installed git submodules via Trying to get RPCs for BNB chain via:
Getting this error:
What am I doing wrong? |
README.md
Outdated
- Re-test the cached RPCs | ||
|
||
```typescript | ||
const provider = handler.getFastestRpcProvider(); |
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.
const provider = handler.getFastestRpcProvider(); | |
const provider = await handler.getFastestRpcProvider(); |
Your screenshot made me realize two things:
I've tested multiple times with fresh vsc instance, yarn, build, test and script and everything is good for me, it should be for you too
|
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 you build the project with
yarn build
then this lineimport { HandlerConstructorConfig } from "./dist/cjs/src/handler";
returns theModule '"./dist/cjs/src/handler"' has no exported member 'HandlerConstructorConfig'
error. But if you build withyarn build:cjs
there is no such error. Pls make sure thatyarn build
,yarn build:cjs
andyarn build:esm
are working the same way. - Getting this error:
rndquu@Users-MacBook-Pro web3-rpc-racer % npx tsx my.ts
node:internal/modules/cjs/loader:1144
const err = new Error(message);
^
Error: Cannot find module './dist/cjs/src/rpc-handler'
Require stack:
- /Users/rndquu/Public/projects/backend/web3-rpc-racer/my.ts
at Module._resolveFilename (node:internal/modules/cjs/loader:1144:15)
at a._resolveFilename (/Users/rndquu/Public/projects/backend/web3-rpc-racer/node_modules/tsx/dist/cjs/index.cjs:1:1729)
at Module._load (node:internal/modules/cjs/loader:985:27)
at Module.require (node:internal/modules/cjs/loader:1235:19)
at require (node:internal/modules/helpers:176:18)
at <anonymous> (/Users/rndquu/Public/projects/backend/web3-rpc-racer/my.ts:2:28)
at Object.<anonymous> (/Users/rndquu/Public/projects/backend/web3-rpc-racer/my.ts:16:6)
at Module._compile (node:internal/modules/cjs/loader:1376:14)
at Object.S (/Users/rndquu/Public/projects/backend/web3-rpc-racer/node_modules/tsx/dist/cjs/index.cjs:1:1292)
at Module.load (node:internal/modules/cjs/loader:1207:32) {
code: 'MODULE_NOT_FOUND',
requireStack: [ '/Users/rndquu/Public/projects/backend/web3-rpc-racer/my.ts' ]
}
Node.js v20.11.1
How to reproduce:
- Checkout to the latest commit from this PR
yarn install
git submodule update --init --recursive
yarn build:cjs
npx tsx my.ts
import { HandlerConstructorConfig } from "./dist/cjs/src/handler";
import { RPCHandler } from "./dist/cjs/src/rpc-handler";
const config: HandlerConstructorConfig = {
networkId: 100,
autoStorage: false,
cacheRefreshCycles: 5,
};
async function main() {
const handler = new RPCHandler(config);
const provider = await handler.getFastestRpcProvider();
console.log(provider);
}
main();
Your build dir should look like this after running
Vid below is me doing a fresh I do not understand why you are still getting this error @rndquu 0310.mp4 |
Just make a GitHub action to prove its not env related? |
@Keyrxng I haven't touched anything in the code since yesterday but today the build is performed without errors Check this script: import { HandlerConstructorConfig } from "../dist/cjs/src/handler";
import { RPCHandler } from "../dist/cjs/src/rpc-handler";
const config: HandlerConstructorConfig = {
networkId: 314,
autoStorage: false,
};
async function main() {
const handler = new RPCHandler(config);
const provider = await handler.getFastestRpcProvider();
console.log(provider);
}
main(); It returns:
Although there are available RPCs for the Filecoin network: Why is it happening? |
Happy days we are on the same page with it now tho good stuff It's failing because I have the request timeout at 500ms, only three rpcs are returned for 314 that have We want to keep the timeout low as in the sense that UBQ use it but it might be an idea for me to implement some sort of fallback where if none succeed then try again without the timeout? As we use 100 and 1 and they have lots of RPCS for both those chains the timeout should be kept in as I've seen calls take ^3s, so the timeout keeps things clean when there are lots of endpoints to test but in the case of there being very few endpoints or few good quality endpoints maybe it should be handled? Our use-case and the needs that UBQ have for it are covered anything else is more of a public service than anything I think is it not? :)) Let me know how to handle it and I'll push it today $ npx tsx ./tests/script-test.ts
chainID 314: filecoin
chain: Filecoin
rpcss: [
'https://api.node.glif.io',
'https://node.filutils.com/rpc/v1',
'https://api.chain.love/rpc/v1'
]
Trace: r {
_isProvider: true, Provider available
Block number: 3728033
{
'https://node.filutils.com/rpc/v1_314': 789.8341999999999,
'https://api.node.glif.io_314': 1249.8516,
'https://api.chain.love/rpc/v1_314': 1176.7801
} |
You're right, ubiquity case is covered, all these settings (timeout, |
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.
Works fine
I was going to suggest just that, add timeout etc into config and leave it to the user |
Tested, working good, it looks like https://gnosis-pokt.nodies.app it's making the race at this time of writing, the README can be improved further with all the stuff that was tested here, as it it only available as a NPM installation on the current readme |
Code looks good and well structured, the package it's working well, there are maybe things to improve that will only become handful as the package it's use, mergin this, good stuff @Keyrxng |
d003d97
into
ubiquity:web3-rpc-racer-npm-branch
* feat: web3 rpc handler npm package * feat: web3 rpc handler npm package * chore: configurable, add kebab, remove dist * chore: readme * chore: add workflow scripts * chore: remove build workflow * Delete .github/workflows/scripts/kebabalize.sh This is destructive and renames files so lets not use it. * chore: improved race, tests and readme, removed ubq rpcs * chore: script permission * chore: type the constants remove bad else * fix: type out path to src * chore: type the promises, add ts config and jest workflow * chore: final touch ups * chore: export all from index * fix: readme * Update package.json * Update README.md --------- Co-authored-by: Keyrxng <106303466+Keyrxng@users.noreply.github.com> Co-authored-by: アレクサンダー.eth <4975670+pavlovcik@users.noreply.github.com>
Resolves #3
My package is hosted at
yarn @keyrxng/web3-rpc-racer
which I'm using in the video where I had left off the now closed pr.All covered bar one
The wrapper method I haven't implemented as the spec has described it. The optimalRPC is outdated from call to call so
getFastestRpcProvider()
will retest eitherallNetworkUrls
or just the successful cached rpcs, returning an updated provider which the invoking logic would handle making the call. This way the requested provider is always the quickest other than when callinggetProvider()
which will return the provider as-is.the runtime rpcs are dropped as they fail for whatever reason so there is no need for rotation plus you always receive back the lowest latency provider.
Usage
just shows the bad rpcs being excluded until refresh
cache-refresh-every-5.mp4