Entropy tester #2762
Entropy tester #2762
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 8 Skipped Deployments
|
apps/entropy-tester/.eslintrc.js
Outdated
| "@typescript-eslint/no-explicit-any": "off", | ||
| "@typescript-eslint/no-non-null-assertion": "off", | ||
| }, | ||
| }; |
There was a problem hiding this comment.
A few notes:
.eslintrc.jsis a deprecated config format- This is a pretty loose config that won't catch a ton of issues, including many issues that make the type system actually reliable and trustworthy.
The easiest fix is to replace this file with a file eslint.config.js that exports my canned config, e.g. something like this. Note you'll need to set "type": "module" in your package.json to use this as I expect esm for all packages I support.
apps/entropy-tester/package.json
Outdated
| "main": "lib/index.js", | ||
| "types": "lib/index.d.ts", |
There was a problem hiding this comment.
Note that in modern js packages, it is recommended to use subpath exports in addition to main and types at the root, as eventually subpath exports will be the only supported mechanism.
apps/entropy-tester/package.json
Outdated
| "scripts": { | ||
| "build": "tsc", | ||
| "fix:format": "prettier --write \"src/**/*.ts\"", | ||
| "fix:lint": "eslint src/ --fix --max-warnings 0", | ||
| "test:format": "prettier --check \"src/**/*.ts\"", | ||
| "test:lint": "eslint src/ --max-warnings 0", | ||
| "start": "node lib/index.js", | ||
| "dev": "ts-node src/index.ts", | ||
| "prepublishOnly": "pnpm run build && pnpm run test:lint", | ||
| "preversion": "pnpm run test:lint", | ||
| "version": "pnpm run test:format && pnpm run test:lint && git add -A src" | ||
| }, |
There was a problem hiding this comment.
I suggest modeling your scripts after another app, e.g. https://github.com/pyth-network/pyth-crosschain/blob/main/apps/entropy-explorer/package.json#L9-L21 -- many of the scripts here don't make any sense for a package that isn't published like this and you're missing things like having prettier format all valid file types
apps/entropy-tester/package.json
Outdated
| "eslint": "^8.13.0", | ||
| "jest": "^29.7.0", |
There was a problem hiding this comment.
In general you should use catalog: dependencies for anything that's listed in the workspace catalog. Doing so will enable me to maintain the versions as I upgrade shared monorepo dependencies.
At a minimum I would ask that you do this for code checking tools like eslint, jest, prettier, etc.
apps/entropy-tester/package.json
Outdated
| "keywords": [], | ||
| "author": "", | ||
| "license": "Apache-2.0", | ||
| "packageManager": "[email protected]" |
There was a problem hiding this comment.
This field is not necessary on a workspace package, it should only be specified at the root.
apps/entropy-tester/src/index.ts
Outdated
| `Multiple contracts found for chain ${config["chain-id"]}, check contract manager store.`, | ||
| ); | ||
| } | ||
| loadedConfigs.push({ contract: contracts[0], interval }); |
There was a problem hiding this comment.
Doing:
const foo = [];
for (const bar of bars) {
foo.push(getSomeValueFrom(bar));
}is just a procedural equivalent to:
const foo = bars.map(bar => getSomeValueFrom(bar))I generally recommend getting in the habit of the declarative map over the procedural loop
|
|
||
| // eslint-disable-next-line no-constant-condition | ||
| while (true) { | ||
| await new Promise((resolve) => setTimeout(resolve, 2000)); |
There was a problem hiding this comment.
Don't do this, but for your entertainment, if you want some very esoteric node facts, you can actually do this:
import { promisify } from "util";
await promisify(setTimeout)(2000);You wouldn't expect it to work due to the argument order of setTimeout, but this works because of some absolutely wild Node internals where the promisify function actually can return a completely separate implementation which is defined in the Node lib internals.
apps/entropy-tester/src/index.ts
Outdated
| } | ||
| const privateKey = toPrivateKey( | ||
| fs | ||
| .readFileSync(argv["private-key"], "utf-8") |
There was a problem hiding this comment.
I suggest using fs/promises and making this async/await. readFileSync is largely unnecessary now that async/await works, and using it at all is kind of a bad habit because it really only serves to mask the async nature of what you're doing, and in some cases it can unintentionally cause major issues because code gets deferred that doesn't need to be.
The only case I recommend using readFileSync or other such sync functions is if you absolutely must add code like this in a place where the code is not designed for async, and converting it would be a huge pain. But here, that's not the case, you're already in an async function.
apps/entropy-tester/src/index.ts
Outdated
| "Path to the private key to sign the transactions with. Should be hex encoded", | ||
| }, | ||
| }, | ||
| handler: async (argv: any) => { |
There was a problem hiding this comment.
We should never use any; yargs will properly typecheck the arguments so the annotation shouldn't even be needed and adding it just cripples the type system. any is a cancer and it will spread and cause the type checker to silently stop verifying a lot more of your code than you probably intend.
If you must, use unknown, but never use any.
| }, | ||
| "include": ["src"], | ||
| "exclude": ["node_modules", "**/__tests__/*"] | ||
| } |
There was a problem hiding this comment.
I suggest using my canned tsconfig as a base here, as that will enable a lot of much stricter configs which make the type system much more reliable / trustworthy. You can use it like this if you decide to do so
apps/entropy-tester/src/index.ts
Outdated
| async function loadConfig(configPath: string): Promise<LoadedConfig[]> { | ||
| const configs = (await import(configPath, { | ||
| with: { type: "json" }, | ||
| })) as { default: ParsedConfig[] }; |
There was a problem hiding this comment.
For all the same reasons you use serde to deserialize in Rust and you don't just unsafe cast external data, we should also avoid typecasting in typescript and should instead validate the schema using zod: https://v3.zod.dev/ (note we still use v3 everywhere in the monorepo but you're welcome to try v4 if you want). This would look basically like this:
const configSchema = z.array(z.strictObject({
"chain-id": z.string(),
interval: z.string(),
}));
// ...
const configs = configSchema.parse(
(await import(configPath, { with: { type: 'json' } })).default
);
apps/entropy-tester/src/index.ts
Outdated
| if (contracts.length === 0 || !contracts[0]) { | ||
| throw new Error( | ||
| `Can not find the contract for chain ${config["chain-id"]}, check contract manager store.`, | ||
| ); | ||
| } | ||
| if (contracts.length > 1) { | ||
| throw new Error( | ||
| `Multiple contracts found for chain ${config["chain-id"]}, check contract manager store.`, | ||
| ); | ||
| } | ||
| return { contract: contracts[0], interval }; |
There was a problem hiding this comment.
Marginally better here is to store contracts[0] in a variable and reuse that:
const firstContract = contracts[0];
if (contracts.length === 0 || !firstContract) {
/* ... */
} else if (contracts.length > 1) {
/* ... */
} else {
return { contract: firstContract, interval }
}The reason this is preferable is that typescript can guarantee statically with this code that firstContract is not undefined in the return branch, but it cannot guarantee the same with contracts[0] due to how typescript narrowing is designed.
apps/entropy-tester/src/index.ts
Outdated
| }; | ||
| }; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Same comment -- don't typecast, instead parse with zod. Typecasting is unsafe and it breaks all the type safety the type checker attempts to provide.
apps/entropy-tester/src/index.ts
Outdated
| describe: "run the tester until manually stopped", | ||
| builder: RUN_OPTIONS, | ||
| handler: async ( | ||
| argv: ArgumentsCamelCase<InferredOptionTypes<typeof RUN_OPTIONS>>, |
There was a problem hiding this comment.
Hmm yargs should be able to infer this type automatically, are you not finding that to be the case?
apps/entropy-tester/tsconfig.json
Outdated
| @@ -0,0 +1,5 @@ | |||
| { | |||
| "extends": "@cprussin/tsconfig/nextjs.json", | |||
| "include": ["svg.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"], | |||
There was a problem hiding this comment.
You should remove the first and last option here give you aren't in a next app so there's no .next folder at all, and you have no svgs so the first one doesn't make sense
Summary
Utility app to regularly check entropy contracts by making a request and waiting for callback.
Things left to do:
Rationale
Additional reliability of fortuna
How has this been tested?