feat(net): get external IPv4 from libp2p by 317787106 · Pull Request #5407 · tronprotocol/java-tron

@317787106

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

jiangyuanshu added 3 commits

August 9, 2023 12:51

@codecov-commenter

⚠️ 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

halibobo1205

xxo1shine

jwrct

jiangyuanshu added 2 commits

August 15, 2023 15:07

317787106

jiangyuanshu added 6 commits

August 17, 2023 18:56

halibobo1205


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

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.

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.

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.

halibobo1205


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";

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.

Choose a reason for hiding this comment

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

I will delete it

jiangyuanshu added 4 commits

August 24, 2023 18:02

jwrct

halibobo1205

method2.invoke(Args.class, config3);

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

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?

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.

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

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

halibobo1205

@317787106