fix: don't presume lack of removed means true

[InoMurko]

📋 Add associated issues, tickets, docs URL here.

Overview

Describe what your Pull Request is about in a few sentences.

Changes

Describe your changes and implementation choices. More details make PRs easier to review.

• Change 1
• Change 2
• ...

Testing

Describe how to test your new feature/bug fix and if possible, a step by step guide on how to demo this.

[coveralls]

Coverage decreased (-57.03%) to 33.233% when pulling 136f841 on ganache_filter_logs into 518c04e on master.

[chrishunt]

Do we have any more context on why this function exists? It seemed to have fixed the issue we were having in our acceptance specs, but maybe it would be a good idea to add a test to this PR to show why the change was necessary. Maybe we can remove this filter completely?

[InoMurko]

Client specification:

- `removed`: `TAG` - `true` when the log was removed, due to a chain reorganization. `false` if its a valid log.

https://github.com/ethereum/wiki/wiki/JSON-RPC/_compare/677ad3a74a992a544c20e0485e43a39665ea187e...c248e86f1719f57f8546114eda88ae11dd6a84a5
also, rpc specs
https://github.com/ethereum/go-ethereum/wiki/RPC-PUB-SUB#logs

In case of a chain reorganization previous sent logs that are on the old chain will be resend with the removed property set to true. Logs from transactions that ended up in the new chain are emitted. Therefore a subscription can emit logs for the same transaction multiple times.

Searching across the internet, what I get is
the logs get removed set to true if they are part of the chain that is retracted (i.e. becomes non-canon now)
openethereum/parity-ethereum#4187
this is quite interesting:
openethereum/parity-ethereum#6880

So I think our change is valid. Ganache is not a fully compliant Ethereum client,so hiccups here and there are to be expected!
For example:
ethers-io/ethers.js#385 (comment)

The problem is Ganache has historically not been a very reliable simulation of what a blockchain does. I have not used it recently, but it has had a lot of bugs and non-standard behaviour. I believe it is getting better though.

[pdobacz]

I think this totally makes sense. It might have been a simple typo, that never actually surfaced, because removed was always there for geth/parity. removed: false naturally feels like a "defaulter" option, esp. when you're doing getLogs and not getFilterChanges.

LGTM