Fix topic_list parameter implementation (matching and validation) to align with JSON-RPC spec #118
Conversation
Validators disallow topics lists such as: [TOPICA, [TOPICB, TOPICC], TOPICD] which are both common and valid
dylanjw
force-pushed
the
fix-topic-validators
branch
from
c6faf53 to
967965a
Compare
dylanjw
changed the title
dylanjw
force-pushed
the
fix-topic-validators
branch
from
4ed8e53 to
48a097f
Compare
Topics with nested arrays should be interpreted as positional options. The match function was updated to reflect the topic argument spec
dylanjw
force-pushed
the
fix-topic-validators
branch
from
48a097f to
c382a16
Compare
dylanjw
force-pushed
the
fix-topic-validators
branch
from
c382a16 to
0392b0c
Compare
| (TOPICS_B_C, FILTER_MATCH_ONLY_A, False), | ||
| (TOPICS_B_A_C, FILTER_MATCH_ONLY_A, False), | ||
| (TOPICS_B_C_A, FILTER_MATCH_ONLY_A, False), | ||
| (TOPICS_EMPTY, FILTER_MATCH_ONE_OR_MORE, False), |
There was a problem hiding this comment.
This one im not sure on. Intuitively I would think (None,) should match all non-anonymous events excluding anonymous ones, but that is assuming.
There was a problem hiding this comment.
Tests results from ethereum/web3.py#985 show that clients diverge in how they treat None values. go-ethereum interprets None as matching any thing, and parity will match anything even the absence of a value.
I think it is up to us to decide what makes the most sense.
What should happen in these cases?
| Topics | Match params | Parity match | Go-ethereum match | eth-tester match |
|---|---|---|---|---|
| [] | [None,] | Yes | No | |
| [TOPICA] | [None, None] | Yes | No |
There was a problem hiding this comment.
The parity version makes more sense to me. You can already get the geth like functionally by removing the None.
There was a problem hiding this comment.
Parity doesn't allow things like matching any one, or any two, like go-ethereum does. go-ethereum essentially treats None as a place specific wildcard. On the other hand None feels like a bad choice for a wildcard symbol, and is more intuitive that it provide no actual matching function.
There was a problem hiding this comment.
Ah, right, I got them switched up. The geth one makes more sense to me, too. 👍
| (TOPICS_ONLY_A, FILTER_MATCH_ANY_TWO, False), | ||
| (TOPICS_ONLY_B, FILTER_MATCH_ANY_TWO, False), | ||
| (TOPICS_ONLY_C, FILTER_MATCH_ANY_TWO, False), | ||
| (TOPICS_EMPTY, FILTER_MATCH_TWO_OR_MORE, False), |
There was a problem hiding this comment.
Same assumption here. That a None value matches any topic that exists at that position, but not an absent topic.
What was wrong?
Validators disallow topics lists such as:
[TOPICA, [TOPICB, TOPICC], TOPICD]
EDIT: fixes #119
How was it fixed?
The topic validators were extended to check the validity of single items along with flat topic arrays.
Cute Animal Picture