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

help text for --denylist is ambiguous #432

Open
grenade opened this issue Nov 24, 2021 · 2 comments
Open

help text for --denylist is ambiguous #432

grenade opened this issue Nov 24, 2021 · 2 comments

Comments

@grenade
Copy link

grenade commented Nov 24, 2021

telemetry_core --help states that --denylist is a "Space delimited list of the names of chains that are not allowed to connect to telemetry. Case sensitive".

this leaves ambiguity around how to specify chain names that contain spaces. presumably:

--denylist some_chain "Some Chain"

would probably work, but begs the questions:

  • why not use the chain id, which doesn't contain spaces?
  • where the help says "names of chains", does it really mean "chain id's"?

i currently just specify both the name and id separated by spaces in the hope that it gets figured out internally but i would like to understand what is actually required.

eg: here's how our terraform config for our production telemetry filters out testnet chains:

  telemetry_core_deny_list = [
    # Baikal is the relay name, westend2 is the relay id
    "Baikal", "westend2",
    # Como is the relay name, rococo_v1_12 is the relay id
    "Como", "rococo_v1_12",
    # calamari_testnet is the para id, 
    # "Calamari on Baikal" is the para name when it runs on the Baikal relay,
    # "Calamari on Como" is the para name when it runs on the Como relay
    "calamari_testnet", "Calamari on Baikal", "Calamari on Como",
    # manta_testnet is the para id,
    # "Manta on Baikal" is the para name when it runs on the Baikal relay,
    # "Manta on Como" is the para name when it runs on the Como relay
    "manta_testnet", "Manta on Baikal", "Manta on Como",
  ]
@dvdplm
Copy link
Contributor

dvdplm commented Nov 28, 2021

You are right the docs are not good here. It is the chain id and this should be clarified.

That said, I suspect that using chain ID isn't good enough though as there's nothing stopping two chains from having the same name (and sadly it's not even uncommon for this to occur). The fix for that would be to pass in the genesis hash, which is a bit unwieldy for users.

How about we do this:

  • --denylist accepts both a chain id (possibly ambiguous) and a genesis hash (0x-prefixed hex string)
  • any chain that presents a chain id that is on the denylist is rejected
  • no spaces or ", ' etc allowed (exits with error)

@jsdw
Copy link
Collaborator

jsdw commented Nov 29, 2021

From our internal chat, Telemetry is only sent the genesis hash and chain name (the thing that can contain spaces).

Soo, I'd be happy with --denylist accepting either genesis hash or chain name (the space separated pretty name), and tweaking the docs to make usage clear.

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

No branches or pull requests

3 participants