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

Subgraphs (ApiVersion<0.0.5) with CallHandlers have non-deterministic handling of failed transactions #4130

Closed
sduchesneau opened this issue Nov 3, 2022 · 1 comment · Fixed by #4149

Comments

@sduchesneau
Copy link
Contributor

This is a Bug

When processing old blocks (Final(Arc<LightEthereumBlock>)),

  1. the fn triggers_in_block gets the call data from fn blocks_with_triggers
    (https://github.com/graphprotocol/graph-node/blob/master/chain/ethereum/src/chain.rs#L491)
  2. this fn block_with_triggers filters out calls from failed transactions ONLY when the subgraph apiVersion is above 0_0_5 (https://github.com/graphprotocol/graph-node/blob/master/chain/ethereum/src/ethereum_adapter.rs#L1460-L1467)

The result is that old blocks on subgraphs with apiVersion<0.0.5 are triggering callHandlers on failed transactions. (this is a wanted compatibility behavior.)

however

When processing blocks close to head (NonFinal(EthereumBlockWithCalls))

  1. the fn triggers_in_block gets the data from fn parse_call_triggers (https://github.com/graphprotocol/graph-node/blob/master/chain/ethereum/src/chain.rs#L529)
  2. this parse_call_triggers calls block.transaction_for_call_succeeded to filter out calls from failed transactions without checking the apiVersion (https://github.com/graphprotocol/graph-node/blob/master/chain/ethereum/src/ethereum_adapter.rs#L1570 -> https://github.com/graphprotocol/graph-node/blob/master/graph/src/components/ethereum/types.rs#L70)

this means

that the current subgraphs running with ApiVersion < 0.0.5 with CallHandlers are NOT triggering on failed transactions when they happen live, but they ARE triggering on historical blocks.

It may imply that some dapps are running with these subgraphs as a data source and are getting the ""sane"" behavior of skipping failed transactions by ""mistake"", so fixing this bug may not be a good idea. Deprecating service of those callHandler-driven pre-0.0.5 subgraphs sooner rather than later seems like a better idea.

@sduchesneau
Copy link
Contributor Author

Note: firehose implementation treats every block as non-final, to benefit from having the full EthereumBlockWithCalls object, containing more data.

Therefore, the transaction_for_call_succeeded function is always called, filtering out calls on failed transactions when running in firehose mode.

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 a pull request may close this issue.

1 participant