Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions apps/entropy-tester/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module.exports = {
root: true,
parser: "@typescript-eslint/parser",
plugins: ["@typescript-eslint"],
extends: ["eslint:recommended", "plugin:@typescript-eslint/recommended"],
rules: {
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-non-null-assertion": "off",
},
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few notes:

  1. .eslintrc.js is a deprecated config format
  2. 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.

1 change: 1 addition & 0 deletions apps/entropy-tester/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
lib
6 changes: 6 additions & 0 deletions apps/entropy-tester/config.sample.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
- chain-id: berachain_mainnet
interval: 3h
- chain-id: apechain_mainnet
interval: 6h
- chain-id: blast
interval: 10m
58 changes: 58 additions & 0 deletions apps/entropy-tester/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
{
"name": "@pythnetwork/entropy-tester",
"version": "1.0.0",
"description": "Utility to test entropy provider callbacks",
"main": "lib/index.js",
"types": "lib/index.d.ts",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"files": [
"lib/**/*"
],
"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"
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"repository": {
"type": "git",
"url": "git+https://github.com/pyth-network/pyth-crosschain.git",
"directory": "apps/entropy-tester"
},
"bin": {
"pyth-entropy-tester": "./lib/index.js"
},
"devDependencies": {
"@types/ethereum-protocol": "^1.0.2",
"@types/express": "^4.17.21",
"@types/jest": "^27.4.1",
"@types/yargs": "^17.0.10",
"@typescript-eslint/eslint-plugin": "^6.0.0",
"@typescript-eslint/parser": "^6.0.0",
"eslint": "^8.13.0",
"jest": "^29.7.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"pino-pretty": "^11.2.1",
"prettier": "catalog:",
"ts-jest": "^29.1.1",
"ts-node": "catalog:",
"typescript": "catalog:"
},
"dependencies": {
"@pythnetwork/contract-manager": "workspace:*",
"joi": "^17.6.0",
"pino": "^9.2.0",
"prom-client": "^15.1.0",
"viem": "^2.19.4",
"yaml": "^2.1.1",
"yargs": "^17.5.1"
},
"keywords": [],
"author": "",
"license": "Apache-2.0",
"packageManager": "[email protected]"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is not necessary on a workspace package, it should only be specified at the root.

}
168 changes: 168 additions & 0 deletions apps/entropy-tester/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
#!/usr/bin/env node
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General advice but I usually separate scripts from implementations, similar to how you'd do in rust too. here's a good example package where I've done this: https://github.com/dourolabs/fogo-web/tree/main/packages/snapshot. You'll notice there's a simple javascript (not typescript) bin entry point that just imports the cli from a lib file, a cli.ts which configures yargs, and everything else is defined in standard library files.

Structuring things like this enables a few nice benefits:

  1. By having the entry point be written as javascript, you avoid some issues where e.g. package managers will expect the bin entry point to exist at install time, not at build time
  2. You get a nice lib for the implementations that you can reuse if you ever need it
  3. The code is generally easier to structure, read, and predict

import yargs from "yargs";
import { hideBin } from "yargs/helpers";
import YAML from "yaml";
import fs from "fs";
import pino, { Logger } from "pino";
import { DefaultStore } from "@pythnetwork/contract-manager/node/store";
import { EvmEntropyContract } from "@pythnetwork/contract-manager/core/contracts/evm";
import {
PrivateKey,
toPrivateKey,
} from "@pythnetwork/contract-manager/core/base";

type DurationSeconds = number;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newtypes don't do anything in typescript and in fact add cognitive overhead. For instance, this typechecks:

type DurationSeconds = number;
type SheepCountedDuringSleep = number;

const addOneSecond = (value: DurationSeconds) => value + 1;

const main = () => {
  const sheep: SheepCountedDuringSleep = 20;

  // ....

  const oneSecondLater = addOneSecond(sheep);
};

Because typescript does not actually treat these as newtypes and instead treat these as interchangeable aliases, they tend to lead to confusion and cognitive overhead, and situations where type aliases for compatible types are mixed and the code is hard to understand or predict.

For this reason, I generally advise against this pattern in typescript. Better to make the code obvious than to use aliases that can get abused and just add mental leaps.

Note there are tricks to making newtypes work the way you might expect in Typescript, however they aren't first-class native language features, and I'm dubious that they're worth the effort.

type LoadedConfig = {
contract: EvmEntropyContract;
interval: DurationSeconds;
};

function timeToSeconds(timeStr: string): number {
const match = timeStr.match(/^(\d+)([hms])$/i);
if (!match)
throw new Error("Invalid format. Use formats like '6h', '15m', or '30s'.");

const value = parseInt(match[1], 10);
const unit = match[2].toLowerCase();

switch (unit) {
case "h":
return value * 3600;
case "m":
return value * 60;
case "s":
return value;
default:
throw new Error("Unsupported time unit.");
}
}

function loadConfig(configPath: string): LoadedConfig[] {
const configs = YAML.parse(fs.readFileSync(configPath, "utf-8"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can use json, I recommend it -- I know that yaml is nicer, but json enables using built in loading mechanisms like async import etc and you will generally find life easier in typescript if you just stick to json.

Even better would be to use something like https://github.com/cosmiconfig/cosmiconfig, which will support json OR yaml or even js files for config, and generally makes configuration work the way that people expect of most node tools.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is going be read in runtime. Probably k8s will mount this file as a volume into the docker container. Does the argument still hold?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, async import is specifically made for runtime imports, and cosmiconfig is the most common way to configure node stuff, so I'd still do it. I don't think it's a big deal, but it's a good habit to get into and a better default for future code -- sticking to json or cosmiconfig (there's a few other similar config libs out there) will generally make your life easier when dealing with configuring node stuff.

const loadedConfigs = [];
for (const config of configs) {
const interval = timeToSeconds(config["interval"]);
const contracts = Object.values(DefaultStore.entropy_contracts).filter(
(contract) => contract.chain.getId() == config["chain-id"],
);
if (contracts.length === 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.`,
);
}
loadedConfigs.push({ contract: contracts[0], interval });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
return loadedConfigs;
}

async function testLatency(
contract: EvmEntropyContract,
privateKey: PrivateKey,
logger: Logger,
) {
const provider = await contract.getDefaultProvider();
const userRandomNumber = contract.generateUserRandomNumber();
const requestResponse = await contract.requestRandomness(
userRandomNumber,
provider,
privateKey,
true, // with callback
);
// Read the sequence number for the request from the transaction events.
const sequenceNumber = parseInt(
requestResponse.events.RequestedWithCallback.returnValues.sequenceNumber,
);
logger.info(
{ sequenceNumber, txHash: requestResponse.transactionHash },
`Request submitted`,
);

const startTime = Date.now();

// eslint-disable-next-line no-constant-condition
while (true) {
await new Promise((resolve) => setTimeout(resolve, 2000));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const request = await contract.getRequest(provider, sequenceNumber);
logger.debug(request);

if (parseInt(request.sequenceNumber) === 0) {
// 0 means the request is cleared
const endTime = Date.now();
logger.info(
{ sequenceNumber, latency: endTime - startTime },
`Successful callback`,
);
break;
}
if (Date.now() - startTime > 60000) {
logger.error(
{ sequenceNumber },
"Timeout: 60s passed without the callback being called",
);
break;
}
}
}

yargs(hideBin(process.argv))
.parserConfiguration({
"parse-numbers": false,
})
.command({
command: "run",
describe: "run the tester until manually stopped",
builder: {
validate: {
description: "Only validate the configs and exit",
type: "boolean",
default: false,
required: false,
},
config: {
description: "Yaml config file",
type: "string",
required: true,
},
"private-key": {
type: "string",
required: true,
description:
"Path to the private key to sign the transactions with. Should be hex encoded",
},
},
handler: async (argv: any) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

const logger = pino();
const configs = loadConfig(argv.config);
if (argv.validate) {
logger.info("Config validated");
return;
}
const privateKey = toPrivateKey(
fs
.readFileSync(argv["private-key"], "utf-8")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.replace("0x", "")
.trimEnd(),
);
logger.info("Running");
const promises = configs.map(async ({ contract, interval }) => {
const child = logger.child({ chain: contract.chain.getId() });
// eslint-disable-next-line no-constant-condition
while (true) {
try {
await testLatency(contract, privateKey, child);
} catch (e) {
child.error(e, "Error testing latency");
}
await new Promise((resolve) => setTimeout(resolve, interval * 1000));
}
});
await Promise.all(promises);
},
})
.demandCommand()
.help().argv;
15 changes: 15 additions & 0 deletions apps/entropy-tester/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"target": "esnext",
"module": "nodenext",
"declaration": true,
"rootDir": "src/",
"outDir": "./lib",
"strict": true,
"esModuleInterop": true,
"resolveJsonModule": true
},
"include": ["src"],
"exclude": ["node_modules", "**/__tests__/*"]
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Loading
Loading