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

OG-906: OG-903: Enable the private RelayServer registration and operation #887

Merged
merged 6 commits into from
Sep 6, 2022

Conversation

forshtat
Copy link
Member

OG-903: Add 'blacklisted recipients' configuration field

…tion

OG-903: Add 'blacklisted recipients' configuration field
Infinite loop due to stubbing. Not possible in real code.
const registerMethod = await this.contractInteractor.getRegisterRelayMethod(this.hubAddress, this.config.url)
let registeredUrl: string
if (this.config.privateMode) {
registeredUrl = ''
Copy link
Member

Choose a reason for hiding this comment

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

instead of ignoring the config url, maybe private mode should validate the url is not set

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I was thinking a relay operator who wants to change from private to public would rather flip a flag than remove/add the string. Especially relevant for testing.

Copy link
Member

Choose a reason for hiding this comment

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

I think it is far worse to have a configuration parameter ignored for not clear reason, than try to "optimize" adding a new config line in the rare case of changing "private" to "public"

@forshtat forshtat merged commit 1f827b7 into master Sep 6, 2022
@forshtat forshtat deleted the OG-906-allow-private-server-registration branch September 6, 2022 16:22
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