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

Feature/fix firehose dynamic data source #4075

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

sduchesneau
Copy link
Contributor

@sduchesneau sduchesneau commented Oct 18, 2022

This PR leverages the feature of firehose-ethereum to fetch a single block (https://github.com/streamingfast/proto/blob/develop/sf/firehose/v2/firehose.proto#L13-L40) at current cursor in its full version.
It includes a bump in the firehose protocol from sf.firehose.v1.Stream/Blocks to sf.firehose.v2.Stream/Blocks

Firehose endpoints now have a minimal version requirement to because of the upgrade to sf.firehose.v2.Stream/Blocks:

When a dynamic data source is added on a block, that block must be fetched back from firehose before we apply the new source to it, because the block that we have in hands is incomplete (transformed by the firehose filter to only contain interesting transactions).

@leoyvens
Copy link
Collaborator

leoyvens commented Oct 19, 2022

Does this reproduce with static filters on? In principle the static filter should retrieve all the triggers that could potentially apply to a template, regardless of the dynamic data sources that are actually instantiated.

Edit: turns out we don't always want to have static filters on with firehose, and will do it adaptively as introduced in #4008

@sduchesneau sduchesneau force-pushed the feature/fix-firehose-dynamic-data-source branch 2 times, most recently from c53a20d to 29c277d Compare October 20, 2022 13:50
@sduchesneau
Copy link
Contributor Author

Does this reproduce with static filters on?
Edit: turns out we don't always want to have static filters on with firehose, and will do it adaptively as introduced in #4008

Exactly, this problem happens only without static filters, but since the performance of static filters is very bad on some use cases, the adaptive approach is much more appropriate, so both ways must work without determinism issues

@sduchesneau sduchesneau force-pushed the feature/fix-firehose-dynamic-data-source branch from 29c277d to 1667e12 Compare October 24, 2022 12:57
* This firehose bump requires all firehose endpoints to be upgraded to v2
* the Dynamic DataSource fix requires the ethereum firehose endpoint to expose
  the new sf.firehose.v2.Fetch.Block method, used to refetch block before apply new filters
* added BlockRefetcher interface for mocking in ethereum integration tests
@sduchesneau sduchesneau force-pushed the feature/fix-firehose-dynamic-data-source branch from ecbf583 to b622682 Compare October 25, 2022 19:37
@sduchesneau sduchesneau requested a review from leoyvens October 25, 2022 19:37
@sduchesneau
Copy link
Contributor Author

(note: current unit test failing happen on master)

@sduchesneau sduchesneau marked this pull request as ready for review November 3, 2022 18:46
@leoyvens
Copy link
Collaborator

leoyvens commented Nov 8, 2022

When we mix rpcs and firehoses in a config, it would be good to avoid calling the firehose endpoint for a refetch on a subgraph that's using an rpc blockstream. But we don't have to deal with that right now.

@leoyvens leoyvens merged commit e139890 into master Nov 8, 2022
@leoyvens leoyvens deleted the feature/fix-firehose-dynamic-data-source branch November 8, 2022 16:16
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.

3 participants