Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(waku-filter): Waku node filter privacy and security tests #2096

Merged

Conversation

AlejandroCabeza
Copy link
Contributor

@AlejandroCabeza AlejandroCabeza commented Oct 2, 2023

Description

Implement privacy and security tests for waku node filter.

Changes

  • Implement privacy and security tests.

Dependencies

#2095

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2096

Built from cdd80d7

Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

tests/node/test_wakunode_filter.nim Show resolved Hide resolved
@AlejandroCabeza AlejandroCabeza requested review from jm-clius and removed request for ABresting and alrevuelta October 30, 2023 14:36
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

Good set of tests!

# Given the client refreshes the subscription
# TODO: CHECK IF THIS IS NECESSARY.
# AT FIRST GLANCE IT SEEMS TO COLLIDE WITH WAKU_FILTER_CLIENT'S BEHAVIOUR: SHOULD NOT NEED?
let subscribeResponse2 = await client.filterSubscribe(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this required for the test to pass? I would expect that it shouldn't be needed as long as:

  1. the service node didn't restart
  2. the client starts up again with the same peerId and transport as before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is required, yes.

I think so too, even more given the client doesn't do any unsubscribing, apparently. I'll open a ticket.

@AlejandroCabeza AlejandroCabeza merged commit e3c816a into test-waku-filter-unsubscribe-all Nov 15, 2023
6 of 8 checks passed
@AlejandroCabeza AlejandroCabeza deleted the test-waku-filter-node-e2e branch November 15, 2023 09:24
AlejandroCabeza added a commit that referenced this pull request Nov 15, 2023
* Implement unsubscribe waku filter tests.
* test(waku-filter): Unsubscribe all, payloads and security tests (#2095)
* Implement waku node filter Security and Privacy tests (#2096)
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.

None yet

3 participants