Skip to content

refactor: use slices.Contains to simplify code#1159

Open
chuanshanjida wants to merge 1 commit into
cosmos:mainfrom
chuanshanjida:main
Open

refactor: use slices.Contains to simplify code#1159
chuanshanjida wants to merge 1 commit into
cosmos:mainfrom
chuanshanjida:main

Conversation

@chuanshanjida

@chuanshanjida chuanshanjida commented May 1, 2026

Copy link
Copy Markdown

Description

There is a new function added in the go1.21 standard library, which can make the code more concise and easy to read.

The new pr with unit test for #1101


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • tackled an existing issue or discussed with a team member
  • left instructions on how to review the changes
  • targeted the main branch

@chuanshanjida chuanshanjida requested a review from a team as a code owner May 1, 2026 18:24
@greptile-apps

greptile-apps Bot commented May 1, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

@chuanshanjida

chuanshanjida commented May 1, 2026

Copy link
Copy Markdown
Author

The newly added test code fully covers the changes made in this update.

image

and

image

@aljo242 aljo242 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

behavioral regression in filters/utils.go: the original loop sets match := len(sub) == 0 -- an empty sub-slice is a wildcard (matches any topic at that position). slices.Contains(sub, ...) on an empty slice returns false, so a filter like [][]common.Hash{{}} now rejects all logs at position 0 instead of accepting all. this breaks the ethereum filter spec.

test gap: the test case named "empty topic sub-array (wildcard)" uses topics: [][]common.Hash{} (empty outer array), which skips the loop entirely and doesn't exercise the broken path. add a case with [][]common.Hash{{}} (one empty inner slice = position-0 wildcard) and verify it still returns all logs.

the authz.go change is correct and clean.

@chuanshanjida

chuanshanjida commented May 28, 2026

Copy link
Copy Markdown
Author

behavioral regression in filters/utils.go: the original loop sets match := len(sub) == 0 -- an empty sub-slice is a wildcard (matches any topic at that position). slices.Contains(sub, ...) on an empty slice returns false, so a filter like [][]common.Hash{{}} now rejects all logs at position 0 instead of accepting all. this breaks the ethereum filter spec.

test gap: the test case named "empty topic sub-array (wildcard)" uses topics: [][]common.Hash{} (empty outer array), which skips the loop entirely and doesn't exercise the broken path. add a case with [][]common.Hash{{}} (one empty inner slice = position-0 wildcard) and verify it still returns all logs.

the authz.go change is correct and clean.

Modified and also rebased the latest code. Please review it again. @aljo242 Thanks~

Signed-off-by: chuanshanjida <chuanshanjida@outlook.com>
@chuanshanjida

Copy link
Copy Markdown
Author

@aljo242 Thanks for your review! If there's anything need I to do, please feel free to let me know.

@chuanshanjida

Copy link
Copy Markdown
Author

@aljo242 friendly ping~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants