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

optimize start block #666

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

Conversation

NIXBNT
Copy link
Collaborator

@NIXBNT NIXBNT commented May 22, 2024

No description provided.

@NIXBNT NIXBNT requested a review from barakman May 22, 2024 08:13
@@ -1293,7 +1293,7 @@ def get_start_block(
elif mgr.tenderly_fork_id:
return safe_int(max(block["last_updated_block"] for block in mgr.pool_data)) - reorg_delay, mgr.w3_tenderly.eth.block_number
else:
return safe_int(max(block["last_updated_block"] for block in mgr.pool_data)) - reorg_delay, None
return max(safe_int(max(block["last_updated_block"] for block in mgr.pool_data)) , last_block) - reorg_delay, None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it that by definition last_block >= last_updated_block for all for blocks in mgr.pool_data?
In which case, your code is equivalent to simply returning last_block.

From looking into how this function is used, it seems that the returned value is then passed on to function get_latest_events, which means that if the value of last_block is passed to it, then most likely no events will be fetched (or perhaps only the events in the very last block).

So I suspect this this fix achieves undesired behavior (perhaps it reduces the time spent on fetching blocks, but on the same time, it may be skipping many required events).

Base automatically changed from remove-sei-and-tri-complete to develop May 22, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants