Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

feat: get logs from a fork network directly #3692

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

adjisb
Copy link
Contributor

@adjisb adjisb commented Sep 20, 2022

No description provided.

@davidmurdoch
Copy link
Member

davidmurdoch commented Sep 28, 2022

This looks like it's not quiet ready for review as you've introduced some new TODOs

@adjisb
Copy link
Contributor Author

adjisb commented Sep 28, 2022

Ok, I will remove the TODOs, they are related to some bugs in the block-manager.ts and can be fixed in other PRs.

@@ -31,46 +32,80 @@ export default class BlockLogManager extends Manager<BlockLogs> {
async getLogs(filter: FilterArgs): Promise<Ethereum.Logs> {
const blockchain = this.#blockchain;
if ("blockHash" in filter) {
// TODO: revert back to getNumberFromHash (when they add support for fallback/forks)
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 depends on a fix on block-manager.

pendingLogsPromises.push(this.get(Quantity.toBuffer(i)));
}
}
// TODO: Use block-manager earliest when fixed (currently it doesn't support fallback/fork correctly)
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 depends on a fix on block-manager.

@@ -31,46 +32,80 @@ export default class BlockLogManager extends Manager<BlockLogs> {
async getLogs(filter: FilterArgs): Promise<Ethereum.Logs> {
const blockchain = this.#blockchain;
if ("blockHash" in filter) {
// TODO: revert back to getNumberFromHash (when they add support for fallback/forks)
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 depends on a fix on block-manager.

@davidmurdoch
Copy link
Member

Ok, I will remove the TODOs, they are related to some bugs in the block-manager.ts and can be fixed in other PRs.

What are the bugs?

@adjisb
Copy link
Contributor Author

adjisb commented Sep 28, 2022

getNumberFromHash is called from api.eth_getBlockTransactionCountByHash but it doesn't support forks, so I used getByHash.
BlockManager.earliest also doesn't support forks or at least doesn't point to the earliest block of the forked network.
I tried to minimize the changes outside blocklog-manager.ts

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants