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.

Sign up for GitHub

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

Jump to bottom

Update configuration validation (remove superstruct) #406

Closed
jacobheun opened this issue Aug 20, 2019 · 1 comment · May be fixed by notadd/magnus#7 or adamlaska/ipfs-js-ipfs#3
Closed

Update configuration validation (remove superstruct) #406

jacobheun opened this issue Aug 20, 2019 · 1 comment · May be fixed by notadd/magnus#7 or adamlaska/ipfs-js-ipfs#3
Labels
exp/novice Someone with a little familiarity can pick up kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up

Comments

@jacobheun
Copy link
Contributor

Currently we use superstruct for config validation. This isn't really necessary and in most instances the validation is very loose. It also doesn't give us support for deep merge of default values, so default values may not get passed to the given module unless they are all explicitly defined.

Rather than restricting validation of the config, we should focus on providing defaults that are sane, and will be deeply merged into any passed configuration. We should do the minimum validation needed for libp2p to run, and create helpful error messages for users about how to correct their configuration.
@jacobheun jacobheun added exp/novice Someone with a little familiarity can pick up kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up labels Aug 20, 2019
@jacobheun jacobheun mentioned this issue Aug 20, 2019
Merged
jacobheun added a commit that referenced this issue Aug 21, 2019
@jacobheun
fix: improve config defaults
09bc0d4 
This removes defaults from superstruct and instead uses
mergeOptions to deeply set the defaults on configuration.
This ensures that defaults are properly set.

This is a step toward removing superstruct altogether, #406,
but it is still being used for basic type validation.
@jacobheun jacobheun mentioned this issue Aug 21, 2019
Merged
jacobheun added a commit that referenced this issue Aug 21, 2019
@jacobheun
fix: improve config defaults (#409)
3eef695 
This removes defaults from superstruct and instead uses
mergeOptions to deeply set the defaults on configuration.
This ensures that defaults are properly set.

This is a step toward removing superstruct altogether, #406,
but it is still being used for basic type validation.
@vasco-santos
Copy link
Member

Closing this as we are not using superstruct anymore

@snyk-bot snyk-bot mentioned this issue Mar 13, 2020
Open
@snyk-bot snyk-bot mentioned this issue Mar 26, 2020
Open
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
@joeltg @achingbrain
fix: treat /dns, /dns4, and /dns6 addrs as public (libp2p#406)
e27747a 
Updates filtering rules to treat dns addresses as public.

Closes libp2p#377

Co-authored-by: Alex Potsides <alex@achingbrain.net>
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
@semantic-release-bot
chore(release): 6.1.0 [skip ci]
aeaec5d 
## [6.1.0](libp2p/js-libp2p-kad-dht@v6.0.4...v6.1.0) (2022-12-07)

### Features

* allow passing ProvidersInit in KadDHT constructor ([libp2p#404](libp2p/js-libp2p-kad-dht#404)) ([e64af85](libp2p/js-libp2p-kad-dht@e64af85))

### Bug Fixes

* treat /dns, /dns4, and /dns6 addrs as public ([libp2p#406](libp2p/js-libp2p-kad-dht#406)) ([e27747a](libp2p/js-libp2p-kad-dht@e27747a)), closes [libp2p#377](libp2p/js-libp2p-kad-dht#377)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
exp/novice Someone with a little familiarity can pick up kind/enhancement A net-new feature or improvement to an existing feature P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Milestone
No milestone
2 participants
@jacobheun @vasco-santos