handling IllegalArgumentException caused by Discovery Disabled Nodes in Endpoint.fromEnode

[vaidikcode]

Signed-off-by: vaidikcode [email protected]

PR description

This PR modifies the Endpoint.fromEnode method to handle cases where discovery is disabled for a given EnodeURL more gracefully. Previously, the method threw an IllegalArgumentException when getDiscoveryPort returned an empty Optional, leading to potential disruptions in the system.

Fixed Issue(s)

#7887

Thanks for sending a pull request! Have you done the following?

• Checked out our contribution guidelines?
• Considered documentation and added the doc-change-required label to this PR if updates are required.
• Considered the changelog and included an update if required.
• For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

• unit tests: ./gradlew build
• acceptance tests: ./gradlew acceptanceTest
• integration tests: ./gradlew integrationTest
• reference tests: ./gradlew ethereum:referenceTests:referenceTests

[macfarla]

if the discovery port is empty, the default port will be used, right? Can we add a test for the expected outcome?

And in that case, should this log be moved to debug? "Using default port for enode"

[macfarla]

looking good just a suggestion on naming and the exception in the test

[macfarla]

prob wouldn't catch this exception in the test, you can mark the test method throws that ex. (what would happen if this exception occurred? would the test pass or fail? should it pass or fail?)

[vaidikcode]

I don't believe this will significantly impact the test's primary function. Should I proceed with Throws it?

[macfarla]

yes I would keep this logic out of the test altogether

[macfarla]

@vaidikcode quick fix needed to resolve compile error

[vaidikcode]

Hi @macfarla , I’ve reviewed the code, and my IntelliJ IDE shows no compile errors. Could you please clarify the specific error or issue you are encountering?

[macfarla]

/Users/sm/workspace/b2/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/Endpoint.java:59: error: cannot find symbol
      log.debug("Discovery disabled for enode {}. Using default port {}.", enode, defaultPort);
      ^
  symbol:   variable log
  location: class Endpoint
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
1 error

[macfarla]

spotless is required to pass. converting to draft since it's not ready to review

[macfarla]

there is a unit test failure, have you run this locally?

/home/runner/work/besu/besu/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java:910: warning: [AddressSelection] Prefer InetAddress.getAllByName to APIs that convert a hostname to a single IP address

    when(enodeWithNoDiscovery.getIp()).thenReturn(InetAddress.getByName("127.0.0.1"));
                                                                       ^
> Task :ethereum:p2p:compileTestJava
    (see https://errorprone.info/bugpattern/AddressSelection)
  Did you mean 'when(enodeWithNoDiscovery.getIp()).thenReturn(InetAddress.getLoopbackAddress());'?
error: warnings found and -Werror specified