Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Remove dead bootnodes, add new geth bootnodes #11441

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

dvdplm
Copy link
Collaborator

@dvdplm dvdplm commented Feb 3, 2020

This PR removes all the bootnodes that do not respond to connection attempts with telnet <ip address> <port>. This leaves us with the 4 parity bootnodes plus the "new" geth bootnodes (as per holiman/go-ethereum@a4eeeed) and removes all legacy nodes that I (we?) do not know the exact status of.

There's a discussion to be had about using JSON as a config file format (in short: it's a terrible idea), but this PR is not the place for that.

Questions to reviewers: what is a better way to test if a bootnode is up? And if it's down, what's a reasonable course of action to figure out if it's temporary or permanent? Is "telnet them for a few days" the best we can do?

@dvdplm dvdplm self-assigned this Feb 3, 2020
@dvdplm dvdplm added A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. F5-task 🎬 Generic label for things that just have to get done but are no issues in first place. M2-config 📂 Chain specifications and node configurations. labels Feb 3, 2020
@varasev
Copy link
Contributor

varasev commented Feb 3, 2020

what is a better way to test if a bootnode is up?

BTW, we used nc -vz [host] [port] to test our POA bootnodes.

@niklasad1
Copy link
Collaborator

niklasad1 commented Feb 3, 2020

BTW, we used nc -vz [host] [port] to test our POA bootnodes.

Sounds good to run in the CI (maybe once per week) by reading the enodes of the chainspec or just extract a CLI tool from devp2p to try send a Hello message instead of just the ip address

There's a discussion to be had about using JSON as a config file format (in short: it's a terrible idea), but this PR is not the place for that.

+1

EDIT: As long as we have these in the chainspecs we should incorporate these checks in the chainspec binary

Copy link
Collaborator

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

Tested with a modified version chainspec bin

fn quit(s: &str) -> ! {
	println!("{}", s);
	process::exit(1);
}

fn main() {
	let mut args = env::args();
	if args.len() != 2 {
		quit("You need to specify chainspec.json\n\
		\n\
		./chainspec <chainspec.json>");
	}

	let path = args.nth(1).expect("args.len() == 2; qed");
	let file = match fs::File::open(&path) {
		Ok(file) => file,
		Err(_) => quit(&format!("{} could not be opened", path)),
	};

	let spec: Result<Spec, _> = serde_json::from_reader(file);

	let spec = match spec {
		Ok(spec) => spec,
		Err(err) => quit(&format!("{} {}", path, err.to_string())),
	};


	for node in spec.nodes.unwrap().iter() {
		let s: Vec<_> = node.split("@").collect();
		let sock_addr: SocketAddr = s[1].parse().expect("valid ip address");
		println!("testing node: {}", sock_addr);
		let stream = TcpStream::connect_timeout(&sock_addr, Duration::from_secs(30)).expect("node is down");
	}

	println!("{} is valid", path);
}

@ordian ordian merged commit 7d9ff1d into master Feb 3, 2020
@ordian ordian deleted the dp/chore/new-geth-bootnodes branch February 3, 2020 13:34
dvdplm added a commit that referenced this pull request Feb 4, 2020
* master:
  update kvdb-rocksdb to 0.4 (#11442)
  Rough architecutre diagram. (#11396)
  ethjson: impl Copy for hash type wrapper (#11423)
  Remove dead bootnodes, add new geth bootnodes (#11441)
  goerli: replace foundation bootnode (#11433)
dvdplm added a commit that referenced this pull request Feb 4, 2020
…pstream

* master:
  Avoid long state queries when serving GetNodeData requests (#11444)
  Cargo.lock: cargo update -p kvdb-rocksdb (#11447)
  rlp_derive: cleanup (#11446)
  chore: remove unused dependencies (#11432)
  update kvdb-rocksdb to 0.4 (#11442)
  Rough architecutre diagram. (#11396)
  ethjson: impl Copy for hash type wrapper (#11423)
  Remove dead bootnodes, add new geth bootnodes (#11441)
  goerli: replace foundation bootnode (#11433)
  Update publish-docker.sh (#11428)
  Revert "[Trace] Distinguish between `create` and `create2` (#11311)" (#11427)
@s3krit s3krit mentioned this pull request Feb 5, 2020
s3krit added a commit that referenced this pull request Feb 5, 2020
* update classic testnet bootnodes (#11398)

* update classic testnet bootnodes

* Update kotti.json

* Update kotti.json

* Update kotti.json

* Update mordor.json

* verification: fix race same block + misc (#11400)

* ethcore: fix race in verification

* verification: fix some nits

* verification: refactor err type `Kind::create`

* fix: tests

* address grumbles

* address grumbles: don't panic

* fix: export hardcoded sync format (#11416)

* fix: export hardcoded sync format

* address grumbles

* make tests compile with rustc_hex 2.0

* fix grumbles: impl LowerHex for encoded Header

* goerli: replace foundation bootnode (#11433)

* Remove dead bootnodes, add new geth bootnodes (#11441)

* update kvdb-rocksdb to 0.4 (#11442)

* Avoid long state queries when serving GetNodeData requests (#11444)

* Remove dead bootnodes, add new geth bootnodes

* More granular locking when fetching state
Finish GetDataNode requests early if queries take too long

* typo

* Use latest kvdb-rocksdb

* Cleanup

* Update ethcore/sync/src/chain/supplier.rs

Co-Authored-By: Andronik Ordian <write@reusable.software>

* Address review grumbles

* Fix compilation

* Address review grumbles

Co-authored-by: Andronik Ordian <write@reusable.software>

* rlp_derive: cleanup (#11446)

* rlp_derive: update syn & co

* rlp_derive: remove dummy_const

* rlp_derive: remove unused attirubutes

* rlp-derive: change authors

* Cargo.lock: cargo update -p kvdb-rocksdb (#11447)

* Cargo.lock: new lockfile format

Manual backport of #11448

* gcc to clang (#11453)

* gcc to clang

test
```
export CC="sccache "$CC
export CXX="sccache "$CXX
```
darwin build
```
CC=clang
CXX=clang
```

* darwin - > default clang

* Bump version and CHANGELOG.md

* chore: remove unused dependencies (#11432)

* fix: compiler warnings

* chore: remove unused dependencies

* Update CHANGELOG.md

* update Cargo.lock

* update CHANGELOG.md

* backwards compatible call_type creation_method (#11450)

* rlp_derive: update syn & co

* rlp_derive: remove dummy_const

* rlp_derive: remove unused attirubutes

* rlp-derive: change authors

* rlp_derive: add rlp(default) attribute

* Revert "Revert "[Trace] Distinguish between `create` and `create2` (#11311)" (#11427)"

This reverts commit 5d4993b.

* trace: backwards compatible call_type and creation_method

* trace: add rlp backward compatibility tests

* cleanup

* i know, i hate backwards compatibility too

* address review grumbles

* update CHANGELOG.md

* just to make sure we don't screw up this again (#11455)

* Update CHANGELOG.md

Co-authored-by: Talha Cross <47772477+soc1c@users.noreply.github.com>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Co-authored-by: Martin Holst Swende <martin@swende.se>
Co-authored-by: David <dvdplm@gmail.com>
Co-authored-by: Andronik Ordian <write@reusable.software>
Co-authored-by: Denis S. Soldatov aka General-Beck <general.beck@gmail.com>
@voron voron mentioned this pull request Feb 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. F5-task 🎬 Generic label for things that just have to get done but are no issues in first place. M2-config 📂 Chain specifications and node configurations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants