Skip to content
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

fix: blocknumber binsearch #68

Merged
merged 9 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions packages/blocknumber/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"dependencies": {
"@ebo-agent/shared": "workspace:*",
"axios": "1.7.7",
"bignumber.js": "9.1.2",
"jwt-decode": "4.0.0",
"viem": "2.17.10"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { EvmBlockNumberProvider } from "./evmBlockNumberProvider.js";

const DEFAULT_PROVIDER_CONFIG = {
blocksLookback: 10_000n,
deltaMultiplier: 2n,
deltaMultiplier: 2,
};

export class BlockNumberProviderFactory {
Expand Down
118 changes: 95 additions & 23 deletions packages/blocknumber/src/providers/evmBlockNumberProvider.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ILogger, UnixTimestamp } from "@ebo-agent/shared";
import { Block, FallbackTransport, HttpTransport, PublicClient } from "viem";
import { BigNumber } from "bignumber.js";
import { Block, BlockNotFoundError, FallbackTransport, HttpTransport, PublicClient } from "viem";

import {
InvalidTimestamp,
Expand All @@ -12,7 +13,7 @@ import {
import { BlockNumberProvider } from "./blockNumberProvider.js";

const BINARY_SEARCH_BLOCKS_LOOKBACK = 10_000n;
const BINARY_SEARCH_DELTA_MULTIPLIER = 2n;
const BINARY_SEARCH_DELTA_MULTIPLIER = 2;

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

Expand All @@ -26,7 +27,7 @@ interface SearchConfig {
* Multiplier to apply to the step, used while scanning blocks backwards, to find a
* lower bound block.
*/
deltaMultiplier: bigint;
deltaMultiplier: number;
}

export class EvmBlockNumberProvider implements BlockNumberProvider {
Expand All @@ -45,7 +46,7 @@ export class EvmBlockNumberProvider implements BlockNumberProvider {
*/
constructor(
client: PublicClient<FallbackTransport<HttpTransport[]>>,
searchConfig: { blocksLookback?: bigint; deltaMultiplier?: bigint },
searchConfig: { blocksLookback?: bigint; deltaMultiplier?: number },
private logger: ILogger,
) {
this.client = client;
Expand Down Expand Up @@ -129,17 +130,24 @@ export class EvmBlockNumberProvider implements BlockNumberProvider {
private async calculateLowerBoundBlock(timestamp: UnixTimestamp, lastBlock: BlockWithNumber) {
const { blocksLookback, deltaMultiplier } = this.searchConfig;

const estimatedBlockTime = await this.estimateBlockTime(lastBlock, blocksLookback);
const timestampDelta = lastBlock.timestamp - timestamp;
let candidateBlockNumber = lastBlock.number - timestampDelta / estimatedBlockTime;
const estimatedBlockTimeBN = await this.estimateBlockTime(lastBlock, blocksLookback);
const timestampDeltaBN = new BigNumber((lastBlock.timestamp - timestamp).toString());

const baseStep = (lastBlock.number - candidateBlockNumber) * deltaMultiplier;
let candidateBlockNumberBN = new BigNumber(lastBlock.number.toString())
.dividedBy(timestampDeltaBN.dividedBy(estimatedBlockTimeBN))
.integerValue();
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this okay? original code is: let candidateBlockNumber = lastBlock.number - timestampDelta / estimatedBlockTime; and you're doing two divisions (idk if im missing smth on the math)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh boy what a nice catch 🎯


const baseStepBN = new BigNumber(lastBlock.number.toString())
.minus(candidateBlockNumberBN)
.multipliedBy(deltaMultiplier);

this.logger.info("Calculating lower bound for binary search...");

let searchCount = 0n;
while (candidateBlockNumber >= 0) {
const candidate = await this.client.getBlock({ blockNumber: candidateBlockNumber });
let searchCount = 0;
while (candidateBlockNumberBN.isGreaterThanOrEqualTo(0)) {
const candidate = await this.client.getBlock({
blockNumber: BigInt(candidateBlockNumberBN.toString()),
});

if (candidate.timestamp < timestamp) {
this.logger.info(`Estimated lower bound at block ${candidate.number}.`);
Expand All @@ -148,7 +156,10 @@ export class EvmBlockNumberProvider implements BlockNumberProvider {
}

searchCount++;
candidateBlockNumber = lastBlock.number - baseStep * 2n ** searchCount;

candidateBlockNumberBN = new BigNumber(lastBlock.number.toString()).minus(
baseStepBN.multipliedBy(2 ** searchCount),
);
}

const firstBlock = await this.client.getBlock({ blockNumber: 0n });
Expand All @@ -171,10 +182,12 @@ export class EvmBlockNumberProvider implements BlockNumberProvider {
this.logger.info("Estimating block time...");

const pastBlock = await this.client.getBlock({
blockNumber: lastBlock.number - BigInt(blocksLookback),
blockNumber: lastBlock.number - blocksLookback,
});

const estimatedBlockTime = (lastBlock.timestamp - pastBlock.timestamp) / blocksLookback;
const estimatedBlockTime = new BigNumber(
(lastBlock.timestamp - pastBlock.timestamp).toString(),
).dividedBy(blocksLookback.toString());

this.logger.info(`Estimated block time: ${estimatedBlockTime}.`);

Expand All @@ -186,8 +199,7 @@ export class EvmBlockNumberProvider implements BlockNumberProvider {
*
* @param timestamp timestamp to find the block for
* @param between blocks search space
* @throws {UnsupportedBlockTimestamps} when two consecutive blocks with the same timestamp are found
* during the search. These chains are not supported at the moment.
* @throws {UnsupportedBlockTimestamps} throw if a block has a smaller timestamp than a previous block.
* @throws {TimestampNotFound} when the search is finished and no block includes the searched timestamp
* @returns the block number
*/
Expand All @@ -206,25 +218,34 @@ export class EvmBlockNumberProvider implements BlockNumberProvider {
currentBlockNumber = (high + low) / 2n;

const currentBlock = await this.client.getBlock({ blockNumber: currentBlockNumber });
const nextBlock = await this.client.getBlock({ blockNumber: currentBlockNumber + 1n });
const nextBlock = await this.getNextBlockWithDifferentTimestamp(currentBlock);

this.logger.debug(
`Analyzing block number #${currentBlock.number} with timestamp ${currentBlock.timestamp}`,
);

// We do not support blocks with equal timestamps (nor non linear or non sequential chains).
// We could support same timestamps blocks by defining a criteria based on block height
// apart from their timestamps.
if (nextBlock.timestamp <= currentBlock.timestamp)
// If no next block with a different timestamp is defined to ensure that the
// searched timestamp is between two blocks, it won't be possible to answer.
//
// As an example, if the latest block has timestamp 1 and we are looking for timestamp 10,
// the next block could have timestamp 2.
if (!nextBlock) throw new TimestampNotFound(timestamp);

// Non linear or non sequential chains are not supported.
if (nextBlock.timestamp < currentBlock.timestamp)
throw new UnsupportedBlockTimestamps(timestamp);

const isCurrentBlockBeforeOrAtTimestamp = currentBlock.timestamp <= timestamp;
const isNextBlockAfterTimestamp = nextBlock.timestamp > timestamp;
const blockContainsTimestamp =
currentBlock.timestamp <= timestamp && nextBlock.timestamp > timestamp;
isCurrentBlockBeforeOrAtTimestamp && isNextBlockAfterTimestamp;

if (blockContainsTimestamp) {
this.logger.debug(`Block #${currentBlock.number} contains timestamp.`);

return currentBlock.number;
const result = await this.searchFirstBlockWithEqualTimestamp(currentBlock);

return result.number;
} else if (currentBlock.timestamp <= timestamp) {
low = currentBlockNumber + 1n;
} else {
Expand All @@ -234,4 +255,55 @@ export class EvmBlockNumberProvider implements BlockNumberProvider {

throw new TimestampNotFound(timestamp);
}

/**
* Get the next block searched moving sequentially and forward which has a different
* timestamp from `block`'s timestamp.
*
* @param block a `Block` with a number and a timestamp.
* @returns a `Block` with a different timestamp, or `null` if no block with different timestamp was found.
*/
private async getNextBlockWithDifferentTimestamp(
block: BlockWithNumber,
): Promise<BlockWithNumber | null> {
let nextBlock: BlockWithNumber = block;

try {
while (nextBlock.timestamp === block.timestamp) {
nextBlock = await this.client.getBlock({ blockNumber: nextBlock.number + 1n });
}

return nextBlock;
} catch (err) {
if (err instanceof BlockNotFoundError) {
// This covers the case where the search surpasses the latest block
// and no more blocks are found by block number.
return null;
} else {
throw err;
}
}
}

/**
* Search the block with the lowest height that has the same timestamp as `block`.
*
* @param block the block to use in the search
* @returns a block with the same timestamp as `block` and with the lowest height.
*/
private async searchFirstBlockWithEqualTimestamp(
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe call this getFirstBlockWithEqualTimestamp or the other function searchNextBlockWithDifferentTimestamp, so you keep a naming convention for search/get functions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice suggestion, I think I'll go with search; feels like a more appropriate naming for O(n) lookups.

get feels like more appropriate for O(1) functions, wdty?

block: BlockWithNumber,
): Promise<BlockWithNumber> {
let prevBlock: BlockWithNumber = block;
let candidateBlock: BlockWithNumber = block;

do {
if (prevBlock.number === 0n) return prevBlock;

candidateBlock = prevBlock;
prevBlock = await this.client.getBlock({ blockNumber: prevBlock.number - 1n });
} while (prevBlock.timestamp === block.timestamp);

return candidateBlock;
}
}
26 changes: 16 additions & 10 deletions packages/blocknumber/test/providers/evmBlockNumberProvider.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { ILogger, UnixTimestamp } from "@ebo-agent/shared";
import { Block, createPublicClient, GetBlockParameters, http } from "viem";
import { Block, BlockNotFoundError, createPublicClient, GetBlockParameters, http } from "viem";
import { mainnet } from "viem/chains";
import { describe, expect, it, vi } from "vitest";

import {
InvalidTimestamp,
LastBlockEpoch,
UnsupportedBlockNumber,
UnsupportedBlockTimestamps,
} from "../../src/exceptions/index.js";
import { EvmBlockNumberProvider } from "../../src/providers/evmBlockNumberProvider.js";

Expand Down Expand Up @@ -80,11 +79,12 @@ describe("EvmBlockNumberProvider", () => {
);
});

it("fails when finding multiple blocks with the same timestamp", () => {
it("returns the first one when finding multiple blocks with the same timestamp", async () => {
const timestamp = BigInt(Date.UTC(2024, 1, 1, 0, 0, 0, 0)) as UnixTimestamp;
const afterTimestamp = BigInt(Date.UTC(2024, 1, 2, 0, 0, 0, 0));
const prevTimestamp = timestamp - 1n;
const afterTimestamp = timestamp + 1n;
const rpcProvider = mockRpcProviderBlocks([
{ number: 0n, timestamp: timestamp },
{ number: 0n, timestamp: prevTimestamp },
{ number: 1n, timestamp: timestamp },
{ number: 2n, timestamp: timestamp },
{ number: 3n, timestamp: timestamp },
Expand All @@ -93,9 +93,9 @@ describe("EvmBlockNumberProvider", () => {

evmProvider = new EvmBlockNumberProvider(rpcProvider, searchConfig, logger);

expect(evmProvider.getEpochBlockNumber(timestamp)).rejects.toBeInstanceOf(
UnsupportedBlockTimestamps,
);
const result = await evmProvider.getEpochBlockNumber(timestamp);

expect(result).toEqual(1n);
});

it("fails when finding a block with no number", () => {
Expand Down Expand Up @@ -158,11 +158,17 @@ function mockRpcProviderBlocks(blocks: Pick<Block, "timestamp" | "number">[]) {
.fn()
.mockImplementation((args?: GetBlockParameters<false, "finalized"> | undefined) => {
if (args?.blockTag == "finalized") {
return Promise.resolve(blocks[blocks.length - 1]);
const block = blocks[blocks.length - 1];

return Promise.resolve(block);
} else if (args?.blockNumber !== undefined) {
const blockNumber = Number(args.blockNumber);
const block = blocks[blockNumber];

if (block === undefined)
throw new BlockNotFoundError({ blockNumber: BigInt(blockNumber) });

return Promise.resolve(blocks[blockNumber]);
return Promise.resolve(block);
}

throw new Error("Unhandled getBlock mock case");
Expand Down
11 changes: 11 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.