scripts: Read suspicious hosts from a file instead of hardcoding

sanjaykdragon commented at 0:39 am on December 29, 2019: contributor

referring to: #17020 good first issue: reading SUSPICIOUS_HOSTS from a file. I haven’t changed the base hosts that were included in the original source, just made it readable from a file.

fanquake added the label P2P on Dec 29, 2019

fanquake added the label Scripts and tools on Dec 29, 2019

in contrib/seeds/makeseeds.py:24 in c22fdc9948 outdated

26-    "54.66.214.167", "54.66.220.137", "54.67.33.14", "54.77.251.214",
27-    "54.94.195.96", "54.94.200.247"
28-}
29+f = open("suspicious_hosts.txt")
30+SUSPICIOUS_HOSTS = {x if "\n" not in x else x.replace("\n", "") for x in f.readlines()}
31+f.close()

practicalswift commented at 1:31 pm on December 29, 2019:

Nit: Slightly shorter using strip() and RAII:

0with open("suspicious_hosts.txt") as f:
1    SUSPICIOUS_HOSTS = {s.strip() for s in f if s.strip()}

sanjaykdragon commented at 3:06 pm on December 29, 2019:

Nit: Slightly shorter using strip() and RAII:

0with open("suspicious_hosts.txt") as f:
1    SUSPICIOUS_HOSTS = {s.strip() for s in f if s.strip()}

Didn’t know that strip() would work in this case, just changed it

practicalswift commented at 3:10 pm on December 29, 2019: contributor

Please squash and use a more descriptive commit message + PR title :)

Suggestion: contrib: makeseeds: Read suspicious hosts from a file instead of hardcoding

Also you might want to sort the entries in contrib/seeds/suspicious_hosts.txt to make it clear where in the file to add new entries without causing unnecessary merge conflicts.

BTW, welcome as a contributor! :)

sanjaykdragon commented at 3:26 pm on December 29, 2019: contributor

Please squash and use a more descriptive commit message + PR title :)

Suggestion: contrib: makeseeds: Read suspicious hosts from a file instead of hardcoding

Also you might want to sort the entries in contrib/seeds/suspicious_hosts.txt to make it clear where in the file to add new entries without causing unnecessary merge conflicts.

BTW, welcome as a contributor! :)

what do you mean sort entries? And thanks for the welcome, also just squashed the commits.

sanjaykdragon renamed this:
~~script: makeseeds.py improvements~~
contrib/scripts: makeseeds: Read suspicious hosts from a file instead of hardcoding
on Dec 29, 2019

fanquake renamed this:
~~contrib/scripts: makeseeds: Read suspicious hosts from a file instead of hardcoding~~
scripts: Read suspicious hosts from a file instead of hardcoding
on Dec 29, 2019

practicalswift commented at 4:21 pm on December 29, 2019: contributor

what do you mean sort entries?

Sort the lines in the file :)

sort -u -o contrib/seeds/suspicious_hosts.txt contrib/seeds/suspicious_hosts.txt in your shell, or M-x sort-lines in an editor built on the shoulders of matching parentheses :)

sanjaykdragon commented at 4:32 pm on December 29, 2019: contributor

what do you mean sort entries?

Sort the lines in the file :)

sort -u -o contrib/seeds/suspicious_hosts.txt contrib/seeds/suspicious_hosts.txt in your shell, or M-x sort-lines in an editor built on the shoulders of matching parentheses :)

Seems pretty pointless, but I did it anyways.

practicalswift commented at 4:41 pm on December 29, 2019: contributor

Sort the lines in the file :) sort -u -o contrib/seeds/suspicious_hosts.txt contrib/seeds/suspicious_hosts.txt in your shell, or M-x sort-lines in an editor built on the shoulders of matching parentheses :)

Seems pretty pointless, but I did it anyways.

The reason to sort entries is that it makes it clear where in the file to add new entries in the future.

In unsorted files people typically append new entries to the end of the file which is likely to cause merge conflicts if two different PR:s add entries to the same file.

Thus sorting the file will help avoid future merge conflicts :)

Makes sense? :)

sanjaykdragon commented at 5:19 pm on December 29, 2019: contributor

Sort the lines in the file :) sort -u -o contrib/seeds/suspicious_hosts.txt contrib/seeds/suspicious_hosts.txt in your shell, or M-x sort-lines in an editor built on the shoulders of matching parentheses :)

Seems pretty pointless, but I did it anyways.

The reason to sort entries is that it makes it clear where in the file to add new entries in the future.

In unsorted files people typically append new entries to the end of the file which is likely to cause merge conflicts if two different PR:s add entries to the same file.

Thus sorting the file will help avoid future merge conflicts :)

Makes sense? :)

Oh, I thought you mean that people were just idiots and didn’t know how to append to a file. Thanks for the explanation!

paymog commented at 9:04 am on January 4, 2020: none

ack b9eb76ece28b4e86b94ecad32f6b5fb35310a7ef

laanwj commented at 1:24 pm on January 4, 2020: member

Thank you! I’m not even sure the suspicious hosts file should be in the repository, but that’s another concern. ACK b9eb76ece28b4e86b94ecad32f6b5fb35310a7ef

laanwj commented at 1:51 pm on January 4, 2020: member

A few linter issues:

 0Python's open(...) seems to be used to open text files without explicitly
 1
 2specifying encoding="utf8":
 3
 4contrib/seeds/makeseeds.py:with open("suspicious_hosts.txt") as f:
 5
 6^---- failure generated from test/lint/lint-python-utf8-encoding.sh
 7
 8contrib/seeds/makeseeds.py:23:1: W191 indentation contains tabs
 9
10contrib/seeds/makeseeds.py:30:1: E101 indentation contains mixed spaces and tabs

fanquake added the label Waiting for author on Jan 4, 2020

sanjaykdragon commented at 4:13 pm on January 4, 2020: contributor

A few linter issues:

 0Python's open(...) seems to be used to open text files without explicitly
 1
 2specifying encoding="utf8":
 3
 4contrib/seeds/makeseeds.py:with open("suspicious_hosts.txt") as f:
 5
 6^---- failure generated from test/lint/lint-python-utf8-encoding.sh
 7
 8contrib/seeds/makeseeds.py:23:1: W191 indentation contains tabs
 9
10contrib/seeds/makeseeds.py:30:1: E101 indentation contains mixed spaces and tabs

fixed, I think

fanquake commented at 11:32 pm on January 6, 2020: member

@sanjaykdragon Can you squash your commits.

contrib: makeseeds: Read suspicious hosts from a file instead of hardcoding e1c582cbaa

practicalswift commented at 11:09 am on January 7, 2020: contributor

ACK e1c582cbaa4c094d204da34c3b1fdd0d4c557519 – diff looks correct

fanquake removed the label Waiting for author on Jan 7, 2020

sanjaykdragon commented at 5:54 pm on January 12, 2020: contributor

@laanwj you mind merging this?

in contrib/seeds/makeseeds.py:22 in e1c582cbaa

24-    "83.81.130.26", "88.198.17.7", "148.251.238.178", "176.9.46.6",
25-    "54.173.72.127", "54.174.10.182", "54.183.64.54", "54.194.231.211",
26-    "54.66.214.167", "54.66.220.137", "54.67.33.14", "54.77.251.214",
27-    "54.94.195.96", "54.94.200.247"
28-}
29+with open("suspicious_hosts.txt", mode="r", encoding="utf-8") as f:

MarcoFalke commented at 6:59 pm on January 13, 2020:

Pretty sure this tries to read the file in the pwd, not in this folder. Is the script required to run in this folder, so that pwd==this folder already holds for other reasons?

sanjaykdragon commented at 11:16 pm on January 13, 2020:

Pretty sure this tries to read the file in the pwd, not in this folder. Is the script required to run in this folder, so that pwd==this folder already holds for other reasons?

Not sure where this script is supposed to be run from.

laanwj commented at 7:22 pm on January 20, 2020:

It’s supposed to be run from the folder it is in, at least according to the instructions in the README (that I always follow): https://github.com/bitcoin/bitcoin/blob/master/contrib/seeds/README.md

(would make sense to make this path configurable, though, for example through a command line option)

laanwj referenced this in commit 7e841f3f9b on Jan 20, 2020

laanwj merged this on Jan 20, 2020

laanwj closed this on Jan 20, 2020

sidhujag referenced this in commit ac6cf09572 on Jan 24, 2020

sidhujag referenced this in commit 54aa12fd8b on Nov 10, 2020

DrahtBot locked this on Feb 15, 2022

scripts: Read suspicious hosts from a file instead of hardcoding #17823