feat: search block by timestamp with binsearch #11

0xyaco · 2024-07-23T21:47:02Z

🤖 Linear

Closes GRT-39

Description

Implements the EvmProvider.getEpochBlockNumber using a binary search with an optimization that takes into account that the current epoch begins near the last block.

linear · 2024-07-23T21:47:04Z

GRT-38 Implement BlocknumberService

Follow Figma diagrams: https://www.figma.com/board/BbciqJb5spg35ZglTsRRBb/Offchain?node-id=233-2491&t=fNurQJG1q77rduKS-4to implement BlocknumberService.

AC:

BlocknumberService successfully implemented
CAIP-2 standard is used for blockchains ids (https://github.com/ChainAgnostic/CAIPs/blob/main/CAIPs/caip-2.md)
BlocknumberService should have the capability to distinguish between an EVM or nonEVM chain to use the corresponding provider to fetch blocknumbers in the future (EVMProvider, BlockmetaProvider)
Unit tests

0xkenj1 · 2024-07-24T16:43:55Z

package.json

-    "packageManager": "[email protected]+sha512.140036830124618d624a2187b50d04289d5a087f326c9edfc0ccd733d76c4f52c3a313d4fc148794a2a9d81553016004e6742e8cf850670268a7387fc220c903"
+    "packageManager": "[email protected]+sha512.140036830124618d624a2187b50d04289d5a087f326c9edfc0ccd733d76c4f52c3a313d4fc148794a2a9d81553016004e6742e8cf850670268a7387fc220c903",
+    "dependencies": {
+        "winston": "^3.13.1"


0xkenj1 · 2024-07-24T16:45:02Z

packages/blocknumber/src/providers/evmProvider.ts

+
+type BlockWithNumber = Omit<Block, "number"> & { number: bigint };
+
+export class EvmProvider implements BlockNumberProvider {


Lets rename EvmProvider here, as we talked some minutes ago

0xkenj1 · 2024-07-24T16:46:57Z

packages/blocknumber/src/providers/evmProvider.ts

+import logger from "../utils/logger.js";
+import { BlockNumberProvider } from "./blockNumberProvider.js";
+
+const BINARY_SEARCH_DELTA_MULTIPLIER = 2n;


Maybe a we can add this as default parameter, wdyt? Or in the constructor, to avoid modifiying BlockNumberProvider interface

Now that we've defined the way to go with the upcoming BlockNumber service, I agree this should be in the default value for some constructor parameter.

0xkenj1 · 2024-07-24T16:48:07Z

packages/blocknumber/src/providers/evmProvider.ts

Natspec left for all of the methods

0xkenj1 · 2024-07-24T16:52:17Z

packages/blocknumber/src/providers/evmProvider.ts

+        this.client = client;
+    }
+
+    async getEpochBlockNumber(


We can add some comments on this method to make sure it is understandable for anyone who jumps into the code

0xkenj1 · 2024-07-24T17:15:25Z

packages/blocknumber/src/providers/evmProvider.ts

+            `Working with latest block (number: ${upperBoundBlock.number}, timestamp: ${upperBoundBlock.timestamp})...`,
+        );
+
+        if (_timestamp >= upperBoundBlock.timestamp) return upperBoundBlock.number;


I would rather throw an error here, its a very edge case, i would prefer not to assume finalization just in case. We can discuss this offline

0xkenj1 · 2024-07-24T17:16:51Z

packages/blocknumber/src/providers/evmProvider.ts

+
+    private async calculateLowerBoundBlock(
+        timestamp: bigint,
+        lastBlock: BlockWithNumber,


lets call this upperBoundBlock as it is on the caller function

0xkenj1 · 2024-07-24T17:31:33Z

packages/blocknumber/src/providers/evmProvider.ts

+    ): Promise<bigint> {
+        const _timestamp = BigInt(timestamp);
+        const upperBoundBlock = await this.client.getBlock({ blockTag: "finalized" });
+


we can also fetch the block 0 and cache it to make sure that we are using a valid timestamp before making any computation.

Oooh nice, I like this idea

0xkenj1 · 2024-07-24T17:32:37Z

packages/blocknumber/src/providers/evmProvider.ts

+    ) {
+        let currentBlockNumber: bigint;
+        let { fromBlock: low, toBlock: high } = between;
+


add validation here if(from > toblock) throw ...

0xkenj1 · 2024-07-24T17:33:43Z

packages/blocknumber/src/utils/logger.ts

Lets move this logger to a new package called common or shared. This will allow us to reuse it accross all our codebase.

0xkenj1

:)

0xkenj1 · 2024-07-24T17:40:57Z

packages/blocknumber/test/providers/evmProvider.spec.ts

+import { UnsupportedBlockTimestamps } from "../../src/exceptions/unsupportedBlockTimestamps.js";
+import { EvmProvider } from "../../src/providers/evmProvider.js";
+
+describe("EvmProvider", () => {


Rename here

0xkenj1 · 2024-07-24T17:42:13Z

packages/blocknumber/test/providers/evmProvider.spec.ts

+import { EvmProvider } from "../../src/providers/evmProvider.js";
+
+describe("EvmProvider", () => {
+    describe("getEpochBlockNumber", () => {


should we also test private methods ? let me know your thoughts on that

I'm debating myself over this, as within this class we have a pretty complex behavior and we might want to go a little deeper with testing.

Generally I don't feel comfortable testing private methods, as this is (generally) a signal that we want to extract some behavior in another class with public methods or that the modelling/abstraction is lacking in some aspect.

In this case, I've managed to cover a nice bunch of edge cases without knowing any implementation specific details, except for the fails when finding multiple blocks with the same timestamp case, where knowing the implementation was needed to cover the actual situation of finding multiple blocks with the same timestamp (eg. binsearch would not detect same timestamp blocks in this situation [t1, (t1*, t2)], I need to specify a pretty tailored scenario [tx, ty, (tz*, tz), tw] for the algorithm to detect two blocks with the same timestamp by looking at the element in the middle).

With this said, I feel like we are okay-ish as we are now, unless we can come up with more complex test cases that are binsearch related. In that case, I'd definitely consider testing the main "binsearch" private methods or extract them to a BinSearch service or something like that with some public and testable methods.

I'll think a little bit more about it; keep suggestions coming!

0xkenj1 · 2024-07-25T18:34:20Z

packages/blocknumber/src/providers/evmBlockNumberProvider.ts

+    /**
+     * Fetches and caches the first block. Cached block will be returned if the cache is hit.
+     *
+     * @returns the chain's first block
+     */
+    private async getFirstBlock(): Promise<Block> {
+        if (this.firstBlock !== null) return this.firstBlock;
+
+        this.firstBlock = await this.client.getBlock({ blockNumber: 0n });
+
+        return this.firstBlock;
+    }
+


NIceeeeee 💯

0xkenj1 · 2024-07-25T18:42:25Z

packages/blocknumber/src/providers/evmBlockNumberProvider.ts

+
+        const firstBlock = await this.getFirstBlock();
+
+        if (_timestamp < firstBlock.timestamp) throw new TimestampNotFound(_timestamp);


I would throw an InvalidTimestamp error, given that in this case, we will never find the block for the timestamp.

I think it's a different case than TimestampNotFound, since on this one we didn't find it but we can in the future.

Correct me if i am wrong pls

0xkenj1 · 2024-07-25T18:43:43Z

packages/blocknumber/test/providers/evmBlockNumberProvider.spec.ts

+
+            expect(evmProvider.getEpochBlockNumber(timestamp)).rejects.toBeDefined();
+        });
+    });


Should we consider the case in which any of the external calls fail? We may want to be sure that the methods are throwing. Let me know wdyt :)

linear · 2024-07-25T19:33:41Z

GRT-39 Implement EVMProvider

Follow figma diagram: https://www.figma.com/board/BbciqJb5spg35ZglTsRRBb/Offchain?node-id=233-2491&t=fNurQJG1q77rduKS-4

AC:

getEpochBlocknumber() successfully implemented
Unit tests

0xkenj1

Amazing job !! 👏

0xnigir1

WOW this is so cool 👏

0xyaco added 11 commits July 18, 2024 18:18

build: separate tsconfig for build stage

06ef8af

feat: set up blocknumber module tests

9802a89

chore: set up blocknumber module linting

546d76e

Merge remote-tracking branch 'origin/dev' into chore/blocknumber-setup

5b57a2d

style: remove total-typescript on non-base tsconfig

f291b45

build: set up build script

0e8fc5a

fix: blocknumber module package config

baa144a

fix: dummy class import module

9965c35

chore: add coverage script and tasks

6215697

chore: remove dummy classes

2748ca2

feat: implement caip-2 compliant chain ids

ab869a4

0xyaco force-pushed the feat/evm-block-search branch 2 times, most recently from 03f487a to b567846 Compare July 23, 2024 21:55

0xyaco added 3 commits July 24, 2024 11:55

refactor: create exceptions/ folder

212960a

Merge remote-tracking branch 'origin/dev' into feat/bn-service

21f025b

feat: search block by timestamp with binsearch

ecd91e9

0xkenj1 reviewed Jul 24, 2024

View reviewed changes

0xkenj1 requested changes Jul 24, 2024

View reviewed changes

Base automatically changed from feat/bn-service to dev July 24, 2024 19:19

0xyaco added 3 commits July 24, 2024 16:23

fix: fixed winston dependency version

4a38c03

refactor: rename evmProvider to evmBlockNumberProvider

711216d

refactor: client and search params in evm provider constructor

c7a9035

0xyaco force-pushed the feat/evm-block-search branch from b567846 to c7a9035 Compare July 25, 2024 16:37

0xyaco added 2 commits July 25, 2024 13:41

Merge remote-tracking branch 'origin/dev' into feat/evm-block-search

18d537a

fix: pnpm dependecies mismatch

5ad1369

0xyaco marked this pull request as ready for review July 25, 2024 16:45

0xkenj1 self-requested a review July 25, 2024 18:29

0xkenj1 requested changes Jul 25, 2024

View reviewed changes

fix: throw InvalidTimestamp for timestamps prior first block

6875aea

0xkenj1 approved these changes Jul 25, 2024

View reviewed changes

0xkenj1 requested a review from 0xnigir1 July 25, 2024 19:45

0xnigir1 approved these changes Jul 26, 2024

View reviewed changes

0xyaco merged commit faffdfd into dev Jul 26, 2024
5 checks passed

0xyaco deleted the feat/evm-block-search branch July 26, 2024 18:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: search block by timestamp with binsearch #11

feat: search block by timestamp with binsearch #11

0xyaco commented Jul 23, 2024 •

edited

Loading

linear bot commented Jul 23, 2024

0xkenj1 Jul 24, 2024

0xkenj1 Jul 24, 2024

0xkenj1 Jul 24, 2024

0xyaco Jul 24, 2024

0xkenj1 Jul 24, 2024

0xkenj1 Jul 24, 2024

0xyaco Jul 24, 2024

0xkenj1 Jul 24, 2024

0xkenj1 Jul 24, 2024

0xkenj1 Jul 24, 2024

0xyaco Jul 24, 2024

0xkenj1 Jul 24, 2024

0xkenj1 Jul 24, 2024

0xkenj1 left a comment

0xkenj1 Jul 24, 2024

0xkenj1 Jul 24, 2024

0xyaco Jul 24, 2024

0xkenj1 Jul 25, 2024

0xkenj1 Jul 25, 2024

0xkenj1 Jul 25, 2024

linear bot commented Jul 25, 2024

0xkenj1 left a comment

0xnigir1 left a comment


		type BlockWithNumber = Omit<Block, "number"> & { number: bigint };

		export class EvmProvider implements BlockNumberProvider {


		const firstBlock = await this.getFirstBlock();

		if (_timestamp < firstBlock.timestamp) throw new TimestampNotFound(_timestamp);

feat: search block by timestamp with binsearch #11

feat: search block by timestamp with binsearch #11

Conversation

0xyaco commented Jul 23, 2024 • edited Loading

🤖 Linear

Description

linear bot commented Jul 23, 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

0xkenj1 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linear bot commented Jul 25, 2024

0xkenj1 left a comment

Choose a reason for hiding this comment

0xnigir1 left a comment

Choose a reason for hiding this comment

0xyaco commented Jul 23, 2024 •

edited

Loading