Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Parity doesn't return removed flag on filter changes #6880

Closed
LogvinovLeon opened this issue Oct 24, 2017 · 9 comments
Closed

Parity doesn't return removed flag on filter changes #6880

LogvinovLeon opened this issue Oct 24, 2017 · 9 comments
Assignees
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M6-rpcapi 📣 RPC API. P5-sometimesoon 🌲 Issue is worth doing soon.
Milestone

Comments

@LogvinovLeon
Copy link

I'm running:

  • Parity version: 1.8.1
  • Operating system: Linux
  • And installed: docker images

I've installed a log filter and try to get filter changes.
Standard defines, that every emited log shouls have a removed field, but the logs I'm getting miss this field.

        {
            "address": "0x71d271f8b14adef568f8f28f1587ce7271ac4ca5",
            "blockHash": "0xe8ed671b455103a24c9c0efc1204416687a2ba2af00c397c350c011202f4d75b",
            "blockNumber": "0x43772e",
            "data": "0x000000000000000000000000db790e1d4a5d7f71db60e55aa7b3af7cf9bbabf60000000000000000000000002013201070451a038d99cbbecedb879b7bdb73e8",
            "logIndex": "0x20",
            "topics": [
                "0x09e48df7857bd0c1e0d31bb8a85d42cf1874817895f171c917f6ee2cea73ec20"
            ],
            "transactionHash": "0x3ab0317c18848368581da767ce0518cac675630f722d1ef8dc385762d5c07c67",
            "transactionIndex": "0x35",
            "transactionLogIndex": "0x0",
            "type": "mined"
        }
@Office-Julia Office-Julia added the Z0-unconfirmed 🤔 Issue might be valid, but it’s not yet known. label Oct 28, 2017
@5chdn 5chdn added F2-bug 🐞 The client fails to follow expected behavior. M6-rpcapi 📣 RPC API. P5-sometimesoon 🌲 Issue is worth doing soon. and removed Z0-unconfirmed 🤔 Issue might be valid, but it’s not yet known. labels Nov 13, 2017
@5chdn 5chdn added this to the 1.10 milestone Nov 13, 2017
@5chdn 5chdn modified the milestones: 1.10, 1.9 Dec 6, 2017
@debris
Copy link
Collaborator

debris commented Dec 29, 2017

it's NOT a bug.

it's caused by insidious change in the specification on 18 Jul 2016 followed by the commit in go-ethereum in Dec 2016.

@debris debris added A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. and removed F2-bug 🐞 The client fails to follow expected behavior. labels Dec 29, 2017
@debris debris modified the milestones: 1.9, 1.10 Dec 29, 2017
@tomusdrw
Copy link
Collaborator

it's NOT a bug

That said, we are aiming for compatibility between clients, so it is important to implement that soonish.

@debris I don't agree that it's insubstantial, since we currently don't inform about removed log events at all - it requires quite significant change in filters logic to support that.

@debris debris changed the title Parity doesn't return removed flag on filter canges Parity doesn't return removed flag on filter changes Dec 29, 2017
@5chdn 5chdn removed the A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). label Jan 3, 2018
@5chdn
Copy link
Contributor

5chdn commented Jan 3, 2018

Note, A-labels are for pull-requests and Q-labels for issues.

@5chdn 5chdn modified the milestones: 1.10, 1.11 Jan 23, 2018
@5chdn 5chdn modified the milestones: 1.11, 1.12 Mar 1, 2018
@sorpaas
Copy link
Collaborator

sorpaas commented Apr 23, 2018

Duplicate of #7994

(Edit: looks like not the same issue.)

@sorpaas sorpaas marked this as a duplicate and then as not a duplicate of #7994 Apr 23, 2018
@5chdn 5chdn modified the milestones: 1.12, 1.11 Apr 23, 2018
@sorpaas
Copy link
Collaborator

sorpaas commented Jun 5, 2018

@folsen @debris @tomusdrw

Right now we have type field in log, which will be set to "removed" if a log is removed in reorg. This is equivalent to, as in this issue, having a removed field set to true. If the goal for resolving the issue is to keep compatibility, then maybe we can support both type and removed?

@folsen
Copy link
Contributor

folsen commented Jun 5, 2018

@sorpaas makes sense to me

@tomusdrw
Copy link
Collaborator

tomusdrw commented Jun 5, 2018

@sorpaas yup, although I believe that filters never report removed log entries at all, so the fix is only for pubsub implementation. So #8796 doesn't fully address this issue I believe.

@sorpaas
Copy link
Collaborator

sorpaas commented Jun 5, 2018

@tomusdrw I think that one is fixed in #8463. So on the filter level removed logs should be returned as normal on master. :)

@tomusdrw
Copy link
Collaborator

tomusdrw commented Jun 5, 2018

I think #8463 fixes the re-org messages, but only for pubsub. The regular RPC filters never had anything like that implemented (not even sure if it was ever speced), have a look here: https://github.com/paritytech/parity/blob/master/rpc/src/v1/impls/eth_filter.rs#L168-L206

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M6-rpcapi 📣 RPC API. P5-sometimesoon 🌲 Issue is worth doing soon.
Projects
None yet
Development

No branches or pull requests

7 participants