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

Allow a data source to create another in the same block it was created #1185

Merged
merged 6 commits into from
Sep 19, 2019

Conversation

leoyvens
Copy link
Collaborator

@leoyvens leoyvens commented Sep 13, 2019

Resolves #1105, this involved some refactoring to instance_manager::process_block.

This was tested by uncommenting this line on this subgraph https://github.com/graphprotocol/aragon-subgraph/blob/sistemico/canary/canary/src/mappings/apm.ts#L42. Previously it would print the expected error, now it spawns the data source and processes its events on the same block.

@leoyvens leoyvens requested a review from a team September 13, 2019 16:23
let block_with_calls = EthereumBlockWithCalls {
ethereum_block: block.deref().clone(),
calls: calls.clone(),
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This code is wrong afaict, if the new data source contains call handlers for new addresses the call won't be contained here. But this is the way it was before so I will open an issue for fixing this.

Copy link

Choose a reason for hiding this comment

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

Looked into this and found that the calls will not be included in the block in the case where the data sources at the beginning of the block did not require them. The calls will be complete when they are included though.

@leoyvens leoyvens force-pushed the leo/create-data-source-on-creation-block branch from 605b4f1 to 256cc4a Compare September 19, 2019 14:03
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM, let's just fix the issue with the calls not being included if the data sources at the beginning of the block did not require them

let block_with_calls = EthereumBlockWithCalls {
ethereum_block: block.deref().clone(),
calls: calls.clone(),
};
Copy link

Choose a reason for hiding this comment

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

Looked into this and found that the calls will not be included in the block in the case where the data sources at the beginning of the block did not require them. The calls will be complete when they are included though.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Talked with @leoyvens, will merge now and fix the issue with calls not being populated during reprocessing separately

@leoyvens leoyvens merged commit dad5b96 into master Sep 19, 2019
@leoyvens
Copy link
Collaborator Author

Thanks for the review!

@leoyvens leoyvens deleted the leo/create-data-source-on-creation-block branch September 19, 2019 18:34
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.

Allow a data source to create another in the same block it was created
1 participant