Skip to content

Commit 79bede4

Browse files
RomarQRomarQ
committed
fix: handle case when a log has no topics
Fixes eth_getLogs RPC behavior by filtering out logs with no topics when a filter is provided
1 parent 29be929 commit 79bede4

File tree

3 files changed

+120
-77
lines changed

3 files changed

+120
-77
lines changed

‎client/rpc-core/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ serde = { workspace = true }
2020
serde_json = { workspace = true }
2121

2222
# Substrate
23-
sp-core = { workspace = true }
23+
sp-core = { workspace = true, features = ["default"] }
2424
sp-crypto-hashing = { workspace = true, features = ["default"] }
2525

2626
[features]

‎client/rpc-core/src/types/filter.rs‎

Lines changed: 116 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,13 @@ pub struct Filter {
129129
/// Address
130130
pub address: Option<FilterAddress>,
131131
/// Topics
132-
#[serde(default, skip_serializing_if = "Vec::is_empty")]
133-
pub topics: Topics,
132+
pub topics: Option<Topics>,
133+
}
134+
135+
impl Filter {
136+
pub fn topics(&self) -> Topics {
137+
self.topics.clone().unwrap_or_default()
138+
}
134139
}
135140

136141
/// Helper for Filter matching.
@@ -191,30 +196,6 @@ impl FilteredParams {
191196
false
192197
}
193198

194-
/// Replace None values - aka wildcards - for the log input value in that position.
195-
pub fn replace(&self, topics: &[H256], input_topics: Topics) -> Vec<Vec<H256>> {
196-
let mut out: Vec<Vec<H256>> = Vec::new();
197-
for (idx, topic) in topics.iter().enumerate() {
198-
if let Some(t) = input_topics.get(idx) {
199-
match t {
200-
VariadicValue::Single(value) => {
201-
out.push(vec![*value]);
202-
}
203-
VariadicValue::Multiple(value) => {
204-
out.push(value.clone());
205-
}
206-
_ => {
207-
out.push(vec![*topic]);
208-
}
209-
};
210-
} else {
211-
out.push(vec![*topic]);
212-
}
213-
}
214-
215-
out
216-
}
217-
218199
pub fn filter_block_range(&self, block_number: u64) -> bool {
219200
let mut out = true;
220201
if let Some(BlockNumberOrHash::Num(from)) = self.filter.from_block {
@@ -269,10 +250,30 @@ impl FilteredParams {
269250
}
270251

271252
pub fn filter_topics(&self, topics: &[H256]) -> bool {
272-
let replaced = self.replace(topics, self.filter.topics.clone());
273-
for (idx, topic) in topics.iter().enumerate() {
274-
if !replaced[idx].contains(topic) {
275-
return false;
253+
for (idx, topic_filter) in self.filter.topics().iter().enumerate() {
254+
let log_topic = topics.get(idx);
255+
256+
match (log_topic, topic_filter) {
257+
// Wildcard matches anything
258+
(_, VariadicValue::Null) => {}
259+
260+
// Single value must match exactly
261+
(Some(actual), VariadicValue::Single(expected)) if actual != expected => {
262+
return false;
263+
}
264+
265+
// Multiple values must contain the actual topic
266+
(Some(actual), VariadicValue::Multiple(options)) if !options.contains(actual) => {
267+
return false;
268+
}
269+
270+
// No topic provided for non-wildcard filter
271+
(None, VariadicValue::Single(_) | VariadicValue::Multiple(_)) => {
272+
return false;
273+
}
274+
275+
// All other combinations are valid
276+
_ => {}
276277
}
277278
}
278279

@@ -392,15 +393,17 @@ mod tests {
392393
to_block: None,
393394
block_hash: None,
394395
address: None,
395-
topics: vec![
396-
VariadicValue::Single(topic1),
397-
VariadicValue::Multiple(vec![topic2, topic3]),
398-
]
399-
.try_into()
400-
.expect("qed"),
396+
topics: Some(
397+
vec![
398+
VariadicValue::Single(topic1),
399+
VariadicValue::Multiple(vec![topic2, topic3]),
400+
]
401+
.try_into()
402+
.expect("qed"),
403+
),
401404
};
402405

403-
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics);
406+
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics());
404407
assert!(FilteredParams::topics_in_bloom(
405408
block_bloom(),
406409
&topics_bloom
@@ -422,14 +425,16 @@ mod tests {
422425
to_block: None,
423426
block_hash: None,
424427
address: None,
425-
topics: vec![
426-
VariadicValue::Single(topic1),
427-
VariadicValue::Multiple(vec![topic2, topic3]),
428-
]
429-
.try_into()
430-
.expect("qed"),
428+
topics: Some(
429+
vec![
430+
VariadicValue::Single(topic1),
431+
VariadicValue::Multiple(vec![topic2, topic3]),
432+
]
433+
.try_into()
434+
.expect("qed"),
435+
),
431436
};
432-
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics);
437+
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics());
433438
assert!(!FilteredParams::topics_in_bloom(
434439
block_bloom(),
435440
&topics_bloom
@@ -444,7 +449,7 @@ mod tests {
444449
address: None,
445450
topics: Default::default(),
446451
};
447-
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics);
452+
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics());
448453
assert!(FilteredParams::topics_in_bloom(
449454
block_bloom(),
450455
&topics_bloom
@@ -467,15 +472,17 @@ mod tests {
467472
to_block: None,
468473
block_hash: None,
469474
address: Some(VariadicValue::Single(test_address)),
470-
topics: vec![
471-
VariadicValue::Single(topic1),
472-
VariadicValue::Multiple(vec![topic2, topic3]),
473-
]
474-
.try_into()
475-
.expect("qed"),
475+
topics: Some(
476+
vec![
477+
VariadicValue::Single(topic1),
478+
VariadicValue::Multiple(vec![topic2, topic3]),
479+
]
480+
.try_into()
481+
.expect("qed"),
482+
),
476483
};
477484
let address_bloom = FilteredParams::address_bloom_filter(&filter.address);
478-
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics);
485+
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics());
479486
let matches = FilteredParams::address_in_bloom(block_bloom(), &address_bloom)
480487
&& FilteredParams::topics_in_bloom(block_bloom(), &topics_bloom);
481488
assert!(matches);
@@ -497,15 +504,17 @@ mod tests {
497504
to_block: None,
498505
block_hash: None,
499506
address: Some(VariadicValue::Single(test_address)),
500-
topics: vec![
501-
VariadicValue::Single(topic1),
502-
VariadicValue::Multiple(vec![topic2, topic3]),
503-
]
504-
.try_into()
505-
.expect("qed"),
507+
topics: Some(
508+
vec![
509+
VariadicValue::Single(topic1),
510+
VariadicValue::Multiple(vec![topic2, topic3]),
511+
]
512+
.try_into()
513+
.expect("qed"),
514+
),
506515
};
507516
let address_bloom = FilteredParams::address_bloom_filter(&filter.address);
508-
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics);
517+
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics());
509518
let matches = FilteredParams::address_in_bloom(block_bloom(), &address_bloom)
510519
&& FilteredParams::topics_in_bloom(block_bloom(), &topics_bloom);
511520
assert!(!matches);
@@ -523,14 +532,16 @@ mod tests {
523532
to_block: None,
524533
block_hash: None,
525534
address: None,
526-
topics: vec![
527-
VariadicValue::Null,
528-
VariadicValue::Multiple(vec![topic2, topic3]),
529-
]
530-
.try_into()
531-
.expect("qed"),
535+
topics: Some(
536+
vec![
537+
VariadicValue::Null,
538+
VariadicValue::Multiple(vec![topic2, topic3]),
539+
]
540+
.try_into()
541+
.expect("qed"),
542+
),
532543
};
533-
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics);
544+
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics());
534545
assert!(FilteredParams::topics_in_bloom(
535546
block_bloom(),
536547
&topics_bloom
@@ -549,17 +560,49 @@ mod tests {
549560
to_block: None,
550561
block_hash: None,
551562
address: None,
552-
topics: vec![
553-
VariadicValue::Null,
554-
VariadicValue::Multiple(vec![topic2, topic3]),
555-
]
556-
.try_into()
557-
.expect("qed"),
563+
topics: Some(
564+
vec![
565+
VariadicValue::Null,
566+
VariadicValue::Multiple(vec![topic2, topic3]),
567+
]
568+
.try_into()
569+
.expect("qed"),
570+
),
558571
};
559-
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics);
572+
let topics_bloom = FilteredParams::topics_bloom_filter(&filter.topics());
560573
assert!(!FilteredParams::topics_in_bloom(
561574
block_bloom(),
562575
&topics_bloom
563576
));
564577
}
578+
579+
#[test]
580+
fn filter_topics_should_return_false_when_filter_has_more_topics_than_log() {
581+
let topic1 =
582+
H256::from_str("1000000000000000000000000000000000000000000000000000000000000000")
583+
.unwrap();
584+
let topic2 =
585+
H256::from_str("2000000000000000000000000000000000000000000000000000000000000000")
586+
.unwrap();
587+
let filter = Filter {
588+
from_block: None,
589+
to_block: None,
590+
block_hash: None,
591+
address: None,
592+
topics: Some(
593+
vec![
594+
VariadicValue::Null,
595+
VariadicValue::Single(topic2),
596+
VariadicValue::Null,
597+
]
598+
.try_into()
599+
.expect("qed"),
600+
),
601+
};
602+
let filtered_params = FilteredParams::new(filter);
603+
// Expected not to match, as the filter has more topics than the log.
604+
assert!(!filtered_params.filter_topics(&[]));
605+
// Expected to match, as the first topic is a wildcard.
606+
assert!(filtered_params.filter_topics(&[topic1, topic2]));
607+
}
565608
}

‎client/rpc/src/eth/filter.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ where
565565
_ => vec![],
566566
};
567567
let topics = filter
568-
.topics
568+
.topics()
569569
.iter()
570570
.map(|flat| match flat {
571571
VariadicValue::Single(item) => vec![*item],
@@ -692,7 +692,7 @@ where
692692

693693
// Pre-calculate BloomInput for reuse.
694694
let address_bloom_filter = FilteredParams::address_bloom_filter(&filter.address);
695-
let topics_bloom_filter = FilteredParams::topics_bloom_filter(&filter.topics);
695+
let topics_bloom_filter = FilteredParams::topics_bloom_filter(&filter.topics());
696696

697697
let mut logs = Vec::new();
698698
while current_number <= to {
@@ -763,7 +763,7 @@ pub(crate) fn filter_block_logs(
763763
removed: false,
764764
};
765765

766-
let topics_match = filter.topics.is_empty() || params.filter_topics(&log.topics);
766+
let topics_match = filter.topics().is_empty() || params.filter_topics(&log.topics);
767767
let address_match = filter
768768
.address
769769
.as_ref()

0 commit comments

Comments
 (0)