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

Event-polling across consecutive blocks is broken #4119

Open
therealjmj opened this issue Jun 2, 2023 · 6 comments
Open

Event-polling across consecutive blocks is broken #4119

therealjmj opened this issue Jun 2, 2023 · 6 comments
Assignees
Labels
fixed/complete This Bug is fixed or Enhancement is complete and published. on-deck This Enhancement or Bug is currently being worked on. v6 Issues regarding v6

Comments

@therealjmj
Copy link

therealjmj commented Jun 2, 2023

Ethers Version

6.4.0

Search Terms

No response

Describe the Problem

It's patched in my local version and working to scan my "Shield" event which occurs across 2 consecutive blocks during automated tests. The event was completely lost - it took me about 6 hours today to track it down.

I believe this is a logical off-by-one bug - the fix is below. Hope it helps :)

Code Snippet

inside PollingEventSubscriber.#poll:

-        filter.fromBlock = this.#blockNumber + 1;
+        filter.fromBlock = this.#blockNumber; // Updated to fix a bug in polling scans for the same filtered events across consecutive blocks (ie. during automated tests).

Contract ABI

No response

Errors

No response

Environment

No response

Environment (Other)

No response

@therealjmj therealjmj added investigate Under investigation and may be a bug. v6 Issues regarding v6 labels Jun 2, 2023
@ricmoo ricmoo added the on-deck This Enhancement or Bug is currently being worked on. label Jun 2, 2023
@ricmoo
Copy link
Member

ricmoo commented Jun 2, 2023

Thanks! I look into it shortly. There was a change in v5 to v6 how the last scanned block was indexed (lastScanned vs nextToScan).

Thanks for your research! :)

@thisisyou33-publishing-networks
Copy link

@ricmoo
Copy link
Member

ricmoo commented Jun 6, 2023

What network are you on?

I've traced through the logic and tried a few experiments, and I think the current logic makes sense. There was a small problem on networks which are "fast" (like BNB, Polygon and Arbitrum) that I've fixed locally.

But trying to track down what might have been the cause of your issue...

@ricmoo
Copy link
Member

ricmoo commented Jun 6, 2023

I made a change that might also help with this, if it was caused by networks with non-consistent events. Can you try out v6.4.2?

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed investigate Under investigation and may be a bug. labels Jun 6, 2023
@therealjmj
Copy link
Author

therealjmj commented Jun 8, 2023

This was on Ethereum.
Not Fixed: v6.4.2 has the same issue, my change is still necessary to get tests to pass correctly.

If you're interested in trying it yourself, in context, see:

https://github.com/Railgun-Community/cookbook

To test:

  1. Isolate the beefy fork test (add only to describe.only(... in FORK-beefy-vault-recipes)
  2. Run anvil locally: ./run-anvil Ethereum https://your/eth/rpc/url
  3. Run tests: env NETWORK_NAME=Ethereum yarn test-fork (note, it takes about 40 sec to run)

If you try without the patch, the tests will fail with incorrect values. This is because the proper "Shield" event was not synced (the block gets skipped by the poller).

Note that the current version of ethers is set to 6.4.0, and the above code patch is in the patches folder.

@ricmoo
Copy link
Member

ricmoo commented Jun 8, 2023

But that patch will duplicate events from blocks, since you are fetching logs for [ x, x + y ] and then [x + y, x + y + z ]. Since x + y was processed twice, and logs in that block would be repeated.

Can you add a console.log inside there to print the ranges being fetched, and include the number of logs fetched?

Woodpile37 pushed a commit to Woodpile37/ethers.js that referenced this issue Jan 14, 2024
Woodpile37 pushed a commit to Woodpile37/ethers.js that referenced this issue Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed/complete This Bug is fixed or Enhancement is complete and published. on-deck This Enhancement or Bug is currently being worked on. v6 Issues regarding v6
Projects
None yet
Development

No branches or pull requests

3 participants