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

feat: add options to enable identify and identify push #1464

Closed
wants to merge 2 commits into from

Conversation

wemeetagain
Copy link
Member

Without rate limiting, identify push may be a DoS vector.

In the mean while it is useful to configure libp2p to completely disable identify and identify push if desired.

FYI was not able to test this test due to type errors installing the latest dependencies :(

Copy link
Member

@mpetrunic mpetrunic left a comment

Choose a reason for hiding this comment

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

Other than that comment, LGTM

@achingbrain
Copy link
Member

The general pattern is enabled: true (defaults to true) rather than disableIdentify: true (defaults to false) (see circuit relay, hop, nat, etc config).

The types seem ok in CI but the tests are failing.

I think adding an enabled: true flag is ok as a tactical fix but longer term maybe we want to split the default protocols (identify, identify-push, ping, fetch) out into modules and have them added as part of config, same as transports or peer discovery modules, etc. What do you think?

@wemeetagain
Copy link
Member Author

and have them added as part of config, same as transports or peer discovery modules, etc.

Absolutely. the way it works now is a too 'magic'.

@p-shahi p-shahi changed the title feat: add options to disable identify and identify push feat: add options to enable identify and identify push Nov 8, 2022
@p-shahi p-shahi added the P3 Low: Not priority right now label Nov 8, 2022
@mpetrunic
Copy link
Member

Triage: No longer important enough.

@mpetrunic mpetrunic closed this Nov 8, 2022
@mpetrunic mpetrunic deleted the feat/identify-off branch November 8, 2022 16:40
@mpetrunic mpetrunic restored the feat/identify-off branch November 8, 2022 16:40
@p-shahi
Copy link
Member

p-shahi commented Nov 8, 2022

relates to #1469

@achingbrain achingbrain deleted the feat/identify-off branch May 18, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 Low: Not priority right now
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants