eth_getLogs should not show removed events in some cases

[rvagg]

TheGraph seems to have an expectation that eth_getLogs doesn't return reverted events, at least past a particular point (it recommends running against a finalised chain for this reason I believe). So, it's running into trouble with a response like this for an eth_getLogs with { "fromBlock":"0x3E2831", "toBlock":"0x3E2831" } on a node that experienced a reorg at epoch 4073521 which resulted in reverted logs:

JSON log output

{
    "id": 74,
    "jsonrpc": "2.0",
    "result": [
        {
            "address": "0xf7c27a7c5b33933880ce8f4054c4d340020181b2",
            "data": "0x000000000000000000000000000000000000000000000000000000000000002a000000000000000000000000000000000000000000000000000000000000000046494c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000066f5f5800000000000000000000000000000000000000000000000007068fb1598aa0000000000000000000000000000000000000000000000000000000000000000251c",
            "topics": [
                "0x8a73e656799f8fbd4ea08e9420225176f98f85ef9a2a11a5a011580b65c8f145",
                "0x00000000000000000000000010a15f515e68ce7bc5884cc6a7254846d1a910fb"
            ],
            "removed": true,
            "logIndex": "0x1e",
            "transactionIndex": "0x40",
            "transactionHash": "0x8e188f03e039a9f5a245cc1175434da357792eed0614776816a9b15e2e1e8774",
            "blockHash": "0x9bdcfa3dcf978b56901e94e2b6ac6e220d27f957e7113d614e522a47e78496cb",
            "blockNumber": "0x3e2831"
        },
        {
            "address": "0xf7c27a7c5b33933880ce8f4054c4d340020181b2",
            "data": "0x000000000000000000000000000000000000000000000000000000000000002a000000000000000000000000000000000000000000000000000000000000000046494c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000066f5f5800000000000000000000000000000000000000000000000007068fb1598aa0000000000000000000000000000000000000000000000000000000000000000251c",
            "topics": [
                "0x8a73e656799f8fbd4ea08e9420225176f98f85ef9a2a11a5a011580b65c8f145",
                "0x00000000000000000000000010a15f515e68ce7bc5884cc6a7254846d1a910fb"
            ],
            "removed": false,
            "logIndex": "0x1e",
            "transactionIndex": "0x41",
            "transactionHash": "0x8e188f03e039a9f5a245cc1175434da357792eed0614776816a9b15e2e1e8774",
            "blockHash": "0x94b6432ae5fa4183633d1f4a40de651e09492b5d1fa6bbb707db340899e81814",
            "blockNumber": "0x3e2831"
        },
        {
            "address": "0xf7c27a7c5b33933880ce8f4054c4d340020181b2",
            "data": "0x000000000000000000000000000000000000000000000000000000000000002a000000000000000000000000000000000000000000000000000000000000000046494c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000066f5f5800000000000000000000000000000000000000000000000007068fb1598aa0000000000000000000000000000000000000000000000000000000000000000251c",
            "topics": [
                "0x8a73e656799f8fbd4ea08e9420225176f98f85ef9a2a11a5a011580b65c8f145",
                "0x00000000000000000000000010a15f515e68ce7bc5884cc6a7254846d1a910fb"
            ],
            "removed": false,
            "logIndex": "0x0",
            "transactionIndex": "0x41",
            "transactionHash": "0x8e188f03e039a9f5a245cc1175434da357792eed0614776816a9b15e2e1e8774",
            "blockHash": "0x94b6432ae5fa4183633d1f4a40de651e09492b5d1fa6bbb707db340899e81814",
            "blockNumber": "0x3e2831"
        }
    ]
}

The first event has "removed": true and the remainder have the expected "removed": false. It then sees 2 different block hashes: 0x9bdcfa3dcf978b56901e94e2b6ac6e220d27f957e7113d614e522a47e78496cb and the canonical 0x94b6432ae5fa4183633d1f4a40de651e09492b5d1fa6bbb707db340899e81814 and it seems to try reconciling or at least following them somehow and gets confused.

I think a thorough client should be filtering these out, regardless, but we are likely bumping up against an Ethereum expectation that we are not properly handling. It's a bit tricky to figure out the precise go-ethereum behaviour from the code (I guess I could spend more time digging), but my guess (slightly educated by the code) is that when you ask for a block by a hash then it fetches precisely those, otherwise when you fetch by a block number (or range of numbers) it gets the canonical hashes for those ranges and works with that. i.e. it does a translation from block number (including the specials, "finalized", "safe", etc.) to block hash and works with that. The block hash in this case is the currently known canonical hash for that block number, regardless of how far back in history it is.

Given that, I think the expected behaviour is:

• If you request a specific "blockHash":"0x..." and that tipset is reverted, return those events and they will all be "removed":true.
• In all other cases, you should only ever return "removed":false events, either by filtering them at the source or filtering before returning to the user.

Additionally worth checking:

• eth_getBlockReceipts should be OK I think because we re-execute tipsets to extract the events instead of using the index
• eth_*filter APIs are likely impacted because they are essentially the same, need to check those to make sure what action, if any needs to be taken.
• eth_subscribe is a live view, I think it's expected that these flip-flop for re-orgs
• GetActorEventsRaw and SubscribeActorEventsRaw should probably get the same treatment, the logic holds for them too I think, if you ask for an epoch then only give the canonical chain tipset for that epoch.

Ref: graphprotocol/graph-node#5661

[rvagg]

This is the kind of thing that makes me think that it's dealing with specific block hashes: https://github.com/ethereum/go-ethereum/blob/16bf471151a2e3c65499e0e9ae913e31d634826e/eth/filters/filter.go#L232-L236

That's for all pathways where it's gathering logs other than by specific block hash. For each block number, it fetches the blocks from the backend as well as the expected "header" information for that block, then filters and merges that data to form the final log output.

For specific block hashes it uses HeaderByHash, gets the results for that header and merges the output: https://github.com/ethereum/go-ethereum/blob/master/eth/filters/filter.go#L274-L277

[rvagg]

Further notes on this:

• Exclude reverted events in the new events APIs #11770 has some history of discussion about this I don't want to lose, so linking it from here. Specifically the ideas around actor events APIs that can do to/from tipsets and have the API walk the path between and give you appropriate reversions as you move from canonical to non-canonical, vice versa, or even if all those tipsets are non-canonical. ChainGetPath being the important API for this.
• @eshon pointed to this discussion which is related: https://ethereum-magicians.org/t/towards-a-stateless-node-api/1458 - basically the same api we were discussing in Exclude reverted events in the new events APIs #11770 but for eth_getLogs where your fromBlock and toBlock can be block hashes and then it'd walk between and do the right thing with reverts. We currently don't support that and I don't believe geth does either. But we could implement that and if we do we need to pay attention to this problem.