feat: add blockmeta block number provider #37

0xyaco · 2024-09-10T14:06:12Z

🤖 Linear

Closes GRT-131

Description

Implements the provider for consuming block meta service by using HTTP POST requests with JSON content.

The blockmeta.BlockByTime service has two operations:

BlockByTime/At which gives you the block that's been mined at a specific timestamp
BlockByTime/Before which gives you the most recent block that's been mined before a specific timestamp

Given that BlockByTime/Before is not inclusive, some extra logic that coordinates the two operations need to be added to get the relevant block for EBO operation.

linear · 2024-09-10T14:06:15Z

GRT-131 Non-EVM block number service

jahabeebs · 2024-09-10T15:04:45Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+
+            return blockNumberAt;
+        } catch (err) {
+            const isAxios404 = isAxiosError(err) && err.status === 404;


considering this is an axios error are we sure it shouldn't be err.response.status?

That's an interesting question. I cannot find any doc describing the error properties, although checking its source code it looks like the err.status is basically the same as err.response?.status.

I'll change to the more explicit property access though, as you suggested 👌 Feels like it removes any ambiguous interpretation!

jahabeebs · 2024-09-10T15:07:40Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+     *
+     * @param response an AxiosResponse of a request to BlockByTime endpoint
+     * @param isoTimestamp the timestamp that was sent in the request
+     * @returns


What are we returning?

jahabeebs · 2024-09-10T15:08:24Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+     * @returns
+     */
+    private parseBlockByTimeResponse(response: AxiosResponse, isoTimestamp: string) {
+        const { data } = response;


we should specify what we're returning in the function def right?

0xnigir1 · 2024-09-11T18:20:40Z

packages/blocknumber/src/providers/blockNumberProviderFactory.ts

-                return new EvmBlockNumberProvider(client, DEFAULT_PROVIDER_CONFIG, logger);
+                return new EvmBlockNumberProvider(evmClient, DEFAULT_PROVIDER_CONFIG, logger);
+
+            case EBO_SUPPORTED_CHAINS_CONFIG.solana.namespace:


Solana 👀 going shopping for those 200GB ram for running validators xd

0xnigir1 · 2024-09-11T18:23:33Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+        private readonly options: BlockmetaClientConfig,
+        private readonly logger: ILogger,
+    ) {
+        const { baseUrl, bearerToken } = options;


is this a long lived Bearer Token like an API Key?

Yeah, people have to get this token through the The Graph webapp which is kinda weird as they have no documented endpoint to automate it.

But now that you mention, we should definitely notify when this token is going to expire, even if it's an extremely long-lived token (1 year).

0xnigir1 · 2024-09-11T18:25:29Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+            const isAxios404 = isAxiosError(err) && err.response?.status === 404;
+            const isUndefinedBlockNumber = err instanceof UndefinedBlockNumber;
+
+            if (!isAxios404 && !isUndefinedBlockNumber) throw err;


would we want to also handle Unauthorized errors?

thinking on this, we could use some ep on constructor (like an echo get or smth simple) to check that credentials are working or else throw on constructor, wdyt?

That's a nice idea. I'm generally reluctant about async-ing constructors but I think that by using a static method we could do something like:

// BlockmetaJsonBlockNumberProvider.ts async static initialize(config, ...) { const provider = new BlockmetaJsonBlockNumberProvider(); const successfulConnection = await provider.testConnection(); if (successfulConnection) return provider; else throw new Error("BANG"); }

Wdyt?

There is a pattern on our best practices :)

https://dev.to/somedood/the-proper-way-to-write-async-constructors-in-javascript-1o8c

Lol we are aligned without even trying

0xnigir1 · 2024-09-11T18:35:28Z

packages/blocknumber/src/providers/blockNumberProviderFactory.ts

+        evmClient: PublicClient<FallbackTransport<HttpTransport[]>>,
+        blockmetaConfig: { baseUrl: URL; servicePath: string; bearerToken: string },


im thinking if this should be a config object like:

{ evmClient?: PublicClient<FallbackTransport<HttpTransport[]>>, blockmetaConfig?: { baseUrl: URL; servicePath: string; bearerToken: string } }: ProviderConfig

and here validate that the proper client/config is defined depending on the switch case. passing an evmClient when instantiating a Solana's one is not necessary actually

Yeah it's kinda too "functional-y" as it is right now, this factory could probably be a instance initialized with the config you mention and then by defining the new BlockNumberProviderFactory().build() instance method, there's no need to pass the config as it's already inside the factory instance.

0xkenj1 · 2024-09-11T19:32:54Z

packages/blocknumber/src/providers/blockNumberProviderFactory.ts

-                return new EvmBlockNumberProvider(client, DEFAULT_PROVIDER_CONFIG, logger);
+                return new EvmBlockNumberProvider(evmClient, DEFAULT_PROVIDER_CONFIG, logger);
+
+            case EBO_SUPPORTED_CHAINS_CONFIG.solana.namespace:


0xkenj1 · 2024-09-11T19:34:00Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+        private readonly options: BlockmetaClientConfig,
+        private readonly logger: ILogger,
+    ) {
+        const { baseUrl, bearerToken } = options;


Config or options ? 🤣

0xkenj1 · 2024-09-11T19:35:22Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+
+    /** @inheritdoc */
+    async getEpochBlockNumber(timestamp: Timestamp): Promise<bigint> {
+        if (timestamp > Number.MAX_SAFE_INTEGER || timestamp < Number.MIN_SAFE_INTEGER)


Suggested change

if (timestamp > Number.MAX_SAFE_INTEGER || timestamp < Number.MIN_SAFE_INTEGER)

if (timestamp > Number.MAX_SAFE_INTEGER || timestamp < 0)

Number.MIN_SAFE_INTEGER = -9007199254740991

Just for fun I've requested the BlockByTime/After after '{"time": "1950-01-01T00:00:00.000Z"}' and got:

"num":"20737620", "time":"1915-04-21T01:10:00.999Z"

There's probably a weird overflow bug in substreams lol should we report it? Doesn't seem too big of an issue as no sane person would try to fetch blocks before computers even existed

Hmmm, maybe we should just tell the graph team on slack

0xkenj1 · 2024-09-11T19:38:36Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+            const isAxios404 = isAxiosError(err) && err.response?.status === 404;
+            const isUndefinedBlockNumber = err instanceof UndefinedBlockNumber;
+
+            if (!isAxios404 && !isUndefinedBlockNumber) throw err;


There is a pattern on our best practices :)

https://dev.to/somedood/the-proper-way-to-write-async-constructors-in-javascript-1o8c

0xkenj1 · 2024-09-11T19:41:04Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+     */
+    private async getBlockNumberAt(isoTimestamp: string): Promise<bigint> {
+        const { servicePath } = this.options;
+


should we validate isoTimestamp here?

Ended up passing around Date instances and each method is in charge of calling the toISOString() method on it. No need for validating now 🪄

0xkenj1 · 2024-09-11T19:41:08Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+     */
+    private async getBlockNumberBefore(isoTimestamp: string): Promise<bigint> {
+        const { servicePath } = this.options;
+


0xkenj1 · 2024-09-11T19:42:24Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+     * @param isoTimestamp the timestamp that was sent in the request
+     * @returns the block number inside a BlockByTime service response
+     */
+    private parseBlockByTimeResponse(response: AxiosResponse, isoTimestamp: string): bigint {


Suggested change

private parseBlockByTimeResponse(response: AxiosResponse, isoTimestamp: string): bigint {

private parseBlockByTimeResponse(data: unknown, isoTimestamp: string): bigint {

Opinions ? @0xnigir1

i wouldn't use unknown because it's an AxiosResponse object, what we should validate is response.data, probablyAxiosRespose<unknown> might be better (if im not wrong it's generic)

AxiosResponse<unknown> works for me!

what i wanted to point out here was that the signature shouldn't rely on Axios types, just receives the data

0xkenj1 · 2024-09-11T19:42:37Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+     */
+    private parseBlockByTimeResponse(response: AxiosResponse, isoTimestamp: string): bigint {
+        const { data } = response;
+        // TODO: validate with zod instead


jahabeebs

some suggestions but don't want to block 👍🏻

jahabeebs · 2024-09-13T13:48:17Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+import { InvalidTokenError, jwtDecode } from "jwt-decode";
+
+import { BlockmetaConnectionFailed } from "../exceptions/blockmetaConnectionFailed.js";
+import { UndefinedBlockNumber } from "../exceptions/undefinedBlockNumber.js";


I guess we should import these from index.js but we can clean up later

Yep, got a task already for that! 👌

jahabeebs · 2024-09-13T13:52:09Z

packages/blocknumber/src/providers/blockmetaJsonBlockNumberProvider.ts

+import { BlockNumberProvider } from "./blockNumberProvider.js";
+
+type BlockByTimeResponse = {
+    num: string;


maybe this should be blockNum so it's a bit more descriptive--I had to see where it's being used to understand the type

This is based on the blockmeta BlockByTime service response, sadly we cannot change it as the keys are defined by the third-party service.

feat: add blockmeta block number provider

da24531

Merge remote-tracking branch 'origin/dev' into feat/non-evm-blocknumber

719df23

0xyaco requested review from 0xkenj1, jahabeebs and 0xnigir1 September 10, 2024 14:11

jahabeebs reviewed Sep 10, 2024

View reviewed changes

0xyaco added 2 commits September 10, 2024 12:38

refactor: use explicit blockmeta response status

e2d952f

docs: add return type and desc to parseBlockByTimeResponse

4819329

0xyaco requested a review from jahabeebs September 10, 2024 15:42

jahabeebs previously approved these changes Sep 10, 2024

View reviewed changes

0xnigir1 reviewed Sep 11, 2024

View reviewed changes

0xkenj1 reviewed Sep 11, 2024

View reviewed changes

0xyaco added 3 commits September 12, 2024 18:33

feat: validate connection for blockmeta service

2b793cb

feat: validate jwt expiration time

c56901f

fix: pr comments

80ee81f

0xyaco dismissed jahabeebs’s stale review via 80ee81f September 12, 2024 22:58

Merge remote-tracking branch 'origin/dev' into feat/non-evm-blocknumber

80e05bf

0xyaco requested review from 0xkenj1, 0xnigir1 and jahabeebs September 12, 2024 23:02

jahabeebs approved these changes Sep 13, 2024

View reviewed changes

jahabeebs self-requested a review September 13, 2024 14:17

jahabeebs approved these changes Sep 13, 2024

View reviewed changes

0xkenj1 approved these changes Sep 13, 2024

View reviewed changes

0xyaco merged commit 3d7c923 into dev Sep 13, 2024
5 checks passed

0xyaco deleted the feat/non-evm-blocknumber branch September 13, 2024 14:52

		evmClient: PublicClient<FallbackTransport<HttpTransport[]>>,
		blockmetaConfig: { baseUrl: URL; servicePath: string; bearerToken: string },

	if (timestamp > Number.MAX_SAFE_INTEGER \|\| timestamp < Number.MIN_SAFE_INTEGER)
	if (timestamp > Number.MAX_SAFE_INTEGER \|\| timestamp < 0)

	private parseBlockByTimeResponse(response: AxiosResponse, isoTimestamp: string): bigint {
	private parseBlockByTimeResponse(data: unknown, isoTimestamp: string): bigint {

feat: add blockmeta block number provider #37

feat: add blockmeta block number provider #37

Conversation

0xyaco commented Sep 10, 2024 • edited Loading

🤖 Linear

Description

linear bot commented Sep 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jahabeebs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xyaco commented Sep 10, 2024 •

edited

Loading