Skip to content

Conversation

@317787106
Copy link
Contributor

@317787106 317787106 commented Aug 9, 2023

What does this PR do?

  • get external IPv4 from llibp2p instead of external url

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

@317787106 317787106 linked an issue Aug 9, 2023 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2023

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 61.25%. Comparing base (1536bc9) to head (3a96fa1).
⚠️ Report is 537 commits behind head on develop.

Files with missing lines Patch % Lines
.../src/main/java/org/tron/core/config/args/Args.java 75.00% 0 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #5407      +/-   ##
=============================================
+ Coverage      61.22%   61.25%   +0.03%     
- Complexity      9319     9326       +7     
=============================================
  Files            842      842              
  Lines          50161    50147      -14     
  Branches        5581     5580       -1     
=============================================
+ Hits           30711    30720       +9     
+ Misses         17040    17021      -19     
+ Partials        2410     2406       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@317787106 317787106 changed the title feat(net): get external ip from libp2p feat(net): get external IPv4 from libp2p Aug 10, 2023

private void buildAssetIssue() {
AssetIssueContract.Builder builder = AssetIssueContract.newBuilder();
builder.setOwnerAddress(ByteString.copyFromUtf8("Address1"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't add non-relevant tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

test coverage will get smaller if don't add non-relevant tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow the PR submission guidelines.


public static final String NODE_DISCOVERY_EXTERNAL_IP = "node.discovery.external.ip";
public static final String AMAZONAWS_URL = "http://checkip.amazonaws.com";
//public static final String AMAZONAWS_URL = "http://checkip.amazonaws.com";
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest just deleting the useless code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will delete it

method2.invoke(Args.class, config3);

Assert.assertNotEquals("127.0.0.1", CommonParameter.getInstance().getNodeDiscoveryBindIp());
Assert.assertNotEquals("46.168.1.1", CommonParameter.getInstance().getNodeExternalIp());
Copy link
Contributor

Choose a reason for hiding this comment

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

"46.168.1.1": What kind of node is this IP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

46.168.1.1 is the configed node.discovery.external.ip in Constant.TEST_CONF.
127.0.0.1 s the configed node.discovery.bind.ip in Constant.TEST_CONF.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then it is recommended to get the variables from the Constant.TEST_CONF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated this testcase.

@317787106 317787106 merged commit f9c42d6 into tronprotocol:develop Aug 30, 2023
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

Successfully merging this pull request may close these issues.

Acquire Internet IP from Libp2p API

5 participants