Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gradle dependency refactor #5561

Closed
1 task
tomatoishealthy opened this issue Oct 30, 2023 · 10 comments · Fixed by #5625
Closed
1 task

Gradle dependency refactor #5561

tomatoishealthy opened this issue Oct 30, 2023 · 10 comments · Fixed by #5625

Comments

@tomatoishealthy
Copy link
Contributor

Rationale

The java-tron code has been refactored once and is currently divided into multiple modules:

  • actuator: transaction module
  • chainbase: DB module
  • common: common base module
  • consensus: consensus module
  • crypto: encryption module
  • framework: architectural organization
  • protocol: definition of core protocol data structure and API
  • plugins: toolset

Each module has its own dependencies, but currently there are situations where multiple modules reference the same dependency at the same time, which means the same dependency is declared multiple times in multiple build.gradle files.

For example:
Most of all the modules contain this dependency:compile group: 'org.bouncycastle', name: 'bcprov-jdk15on', version: '1.69' . Some dependencies in different modules even don't adopt the same version which makes it more chaotic.

There are some drawbacks to this situation:

  1. Bringing additional maintenance costs to development
  2. It is impossible to clearly identify which version of dependency the final Fullnode.jar references
  3. Additional work is added to system maintenance. For example, during daily security inspections, there may be false positive reports of vulnerabilities.

Therefore, this issue aims to solve the problem of dependency confusion and manage dependencies in a unified manner.

Implementation

  • TODO: The detailed dependency sorting and reconstruction plan will be posted later.
@halibobo1205
Copy link
Contributor

Great, it looks good to me.

@lurais
Copy link
Contributor

lurais commented Oct 31, 2023

This means to change some dependencies version in the modules, and the difference of the different dependencies shall be listed.

@CarlChaoCarl
Copy link
Contributor

Good idea, The relationship of dependencies will be much clearer and simpler.

@tomatoishealthy
Copy link
Contributor Author

This means to change some dependencies version in the modules, and the difference of the different dependencies shall be listed.

Yes, I will sort it out and post it later

@forfreeday
Copy link
Contributor

Good idea, this will make the dependencies between projects clearer.

@tomatoishealthy
Copy link
Contributor Author

tomatoishealthy commented Dec 20, 2023

Implementation:

  1. Transfer the repeatedly declared dependencies of the subproject to the root project. such as: migrate bouncycastle and unit test related dependencies to the root project
  2. Eliminate duplicate declarations of dependencies caused by referencing subprojects, such as: chainbase project declares dependence on common project, but both declare references to leveldb and rocksdb

Since there are many dependencies on adjustments, here will not list them one by one.

Test

To ensure the safety of refactoring, the following checks need to be performed

  • Unit test: passed
  • Fullnode syncing on mainnet check: passed
  • Security check: TODO
  • Dependency tree consistency check: handling by dependency-tree-diff

@tomatoishealthy
Copy link
Contributor Author

tomatoishealthy commented Dec 20, 2023

Dependency tree consistency check

Generate dependency trees for the framework of the develop branch and the refactor branch respectively.

./gradlew dependencies > dependency1.txt   // for develop branch
./gradlew dependencies > dependency2.txt   // for refactor branch

compare

dependency-tree-diff dependency1.txt dependency2.txt

output

++--- com.github.briandilley.jsonrpc4j:jsonrpc4j:1.6
+|    \--- org.apache.httpcomponents:httpcore-nio:4.4.5
+|         \--- org.apache.httpcomponents:httpcore:4.4.5 -> 4.4.13
-+--- com.typesafe:config:1.3.2
-+--- com.cedarsoftware:java-util:1.8.0
-|    +--- commons-logging:commons-logging:1.1.1 -> 1.2
-|    \--- com.cedarsoftware:json-io:2.4.1
-+--- com.beust:jcommander:1.72
-+--- junit:junit:4.13.2
-|    \--- org.hamcrest:hamcrest-core:1.3
-+--- net.jcip:jcip-annotations:1.0
-+--- com.fasterxml.jackson.core:jackson-core:2.8.5 -> 2.13.4
-+--- com.google.api.grpc:proto-google-common-protos:2.15.0
-|    \--- com.google.protobuf:protobuf-java:3.21.12
-+--- org.apache.httpcomponents:httpasyncclient:4.1.1
-|    +--- org.apache.httpcomponents:httpcore:4.4.4 -> 4.4.13
-|    +--- org.apache.httpcomponents:httpcore-nio:4.4.4 -> 4.4.5
-|    |    \--- org.apache.httpcomponents:httpcore:4.4.5 -> 4.4.13
-|    +--- org.apache.httpcomponents:httpclient:4.5.1 -> 4.5.13
-|    |    +--- org.apache.httpcomponents:httpcore:4.4.13
-|    |    +--- commons-logging:commons-logging:1.2
-|    |    \--- commons-codec:commons-codec:1.11 -> 1.15
-|    \--- commons-logging:commons-logging:1.2
-+--- com.github.briandilley.jsonrpc4j:jsonrpc4j:1.6
-|    \--- org.apache.httpcomponents:httpcore-nio:4.4.5 (*)
 +--- project :chainbase
+|    +--- org.bouncycastle:bcprov-jdk15on:1.69
-|    +--- org.fusesource.leveldbjni:leveldbjni-all:1.8
-|    +--- org.rocksdb:rocksdbjni:5.15.10
-|    +--- com.typesafe:config:1.3.2
-|    +--- com.fasterxml.jackson.core:jackson-core:2.8.5 -> 2.13.4
 |    +--- project :protocol
-|    |    \--- io.grpc:grpc-protobuf:1.52.1
-|    |         \--- com.google.api.grpc:proto-google-common-protos:2.9.0 -> 2.15.0 (*)
+|    |    +--- org.bouncycastle:bcprov-jdk15on:1.69
+|    |    \--- io.grpc:grpc-protobuf:1.52.1
+|    |         \--- com.google.api.grpc:proto-google-common-protos:2.9.0 -> 2.15.0
+|    |              \--- com.google.protobuf:protobuf-java:3.21.12
 |    +--- project :common
-|    |    +--- com.cedarsoftware:java-util:1.8.0 (*)
+|    |    +--- com.cedarsoftware:java-util:1.8.0
+|    |    |    +--- commons-logging:commons-logging:1.1.1 -> 1.2
+|    |    |    \--- com.cedarsoftware:json-io:2.4.1
-|    |    \--- org.apache.httpcomponents:httpasyncclient:4.1.1 (*)
+|    |    \--- org.apache.httpcomponents:httpasyncclient:4.1.1
+|    |         +--- org.apache.httpcomponents:httpcore:4.4.4 -> 4.4.13
+|    |         +--- org.apache.httpcomponents:httpcore-nio:4.4.4 -> 4.4.5 (*)
+|    |         +--- org.apache.httpcomponents:httpclient:4.5.1 -> 4.5.13
+|    |         |    +--- org.apache.httpcomponents:httpcore:4.4.13
+|    |         |    +--- commons-logging:commons-logging:1.2
+|    |         |    \--- commons-codec:commons-codec:1.11 -> 1.15
+|    |         \--- commons-logging:commons-logging:1.2
-+--- project :actuator
-|    +--- commons-codec:commons-codec:1.11 -> 1.15
-|    \--- org.reflections:reflections:0.9.11 (*)
-\--- project :consensus
-     \--- commons-codec:commons-codec:1.11 -> 1.15

The results show that there are version differences between the two branches, but the differences are all upgrades from the lower version to the higher version.

I think these are not problems, but 100% sure. I don't know if there are other better ideas to compare the dependencies of the final packaged jars of the two branches. Can anyone have some other suggestions? @forfreeday @halibobo1205 @lurais

BTW, could you help me with the security check? @forfreeday

@forfreeday
Copy link
Contributor

I can help you verify the security of the dependency

@lurais
Copy link
Contributor

lurais commented Dec 21, 2023

In order to compare the dependencies of the final jars, jdeps command shall be used .

@tomatoishealthy
Copy link
Contributor Author

In order to compare the dependencies of the final jars, jdeps command shall be used .

The jdeps command reports an error, which seems to be because the jdk version is too low.

I compared the above dependency difference with the develop branch and found that the difference did not exist. There should be no problem with the version referenced after optimization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants