refactor(core-p2p): Remove unnecessary ping() call

[vasild]

• getPeers() is only interested in /peer/list, remove unnecessary ping()
  invocation from getPeers()

• increase ping timeout when accepting new peer

• re-enable peer-verification

Proposed changes

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Refactoring (improve a current implementation without adding a new feature or fixing a bug)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Build (changes that affect the build system)
• Docs (documentation only changes)
• Test (adding missing tests or fixing existing tests)
• Other... Please describe:

Checklist

• I have read the CONTRIBUTING documentation
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

[ghost]

@air1one @faustbrian @supaiku0 - please review this in the next few days. Be sure to explicitly select labels so I know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

[vasild]

3 separate changes go in this PR. They are related, but can be chopped off in separate PRs.

[ghost]

@supaiku0 The ci/circleci: test-node10-1 job is failing as of 7252448269ccd27480d73b1040ef2fc975b4f720. Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

[faustbrian]

@vasild tests need adjustment

[vasild]

@vasild tests need adjustment

Yes, I am looking into this. Looks like the test relied on the side effect of getPeers() calling ping(). I am considering one of the following:

1. Adjust the test to call ping() explicitly, however the test's usage looks sensible: monitor.discoverPeers(); monitor.getNetworkHeight();

2. Adjust discoverPeers() to do the ping, or even more elegant: use acceptNewPeer() (which does the ping) instead of addPeer(). In this case addPeer() will become unused and can be removed.

3. Adjust getNetworkHeight() to do the pings.

[faustbrian]

Would go with option 2

[vasild]

Would go with option 2

Yes. Another goodie popped out of this: acceptNewPeer() was returning early if it detected a test environment (CORE_ENV === "test"). In other words, it was practically disabled in test environments. However all tests that call acceptNewPeer() fool that check by setting CORE_ENV to something else than "test" so that the function proceeds un-disabled. So, I have removed the "return early" code from acceptNewPeer() and also the hackish tweaking of environment from tests.

[codecov-io]

Codecov Report

Merging #2123 into develop will increase coverage by 0.01%.
The diff coverage is 86.66%.

@@             Coverage Diff             @@
##           develop    #2123      +/-   ##
===========================================
+ Coverage    79.33%   79.35%   +0.01%     
===========================================
  Files          329      329              
  Lines         7921     7889      -32     
  Branches      1141     1096      -45     
===========================================
- Hits          6284     6260      -24     
+ Misses        1604     1597       -7     
+ Partials        33       32       -1

Impacted Files	Coverage Δ
packages/core-p2p/src/court/guard.ts	78.12% <ø> (-1.88%)	⬇️
...ackages/core-p2p/src/server/plugins/set-headers.ts	92% <100%> (ø)	⬆️
...ages/core-p2p/src/server/plugins/accept-request.ts	87.5% <100%> (ø)	⬆️
packages/core-p2p/src/peer.ts	88% <75%> (+2.56%)	⬆️
packages/core-p2p/src/monitor.ts	53.35% <88.88%> (-1.14%)	⬇️
...ackages/core-p2p/src/server/versions/1/handlers.ts	72.83% <0%> (-2.47%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d30d6f...89e3246. Read the comment docs.