feat(ci): add multinode integration test workflow#7
Conversation
* fix(config): remove redundant JSON-RPC config fields and consolidate parameter binding Remove maxBlockFilterNum, maxAddressSize, and maxRequestTimeout from NodeConfig/CommonParameter since they were superseded by the jsonRpcMaxBatchSize/jsonRpcMaxResponseSize parameters. Consolidate the config binding path so all JSON-RPC size limits flow through a single reference.conf entry, and clean up stale test-config fixtures. * fix bug of NodeConfigTest * remove allowShieldedTransactionApi from reference.conf * add testcase of external.ip is null * change comment * fix the bug of --es and 7 failed testcases
…onprotocol#6760) Pre-3.0.0(The previous event-plugin public release is 2.2.0) event-plugin builds still link against com.alibaba.fastjson, which was removed from java-tron in tronprotocol#6701. When such a plugin is loaded, the NoClassDefFoundError surfaces inside the plugin's own worker thread and is swallowed by its catch(Throwable) handler. The node keeps running while silently dropping every trigger, leaving operators with no signal that the event subscription is broken. Enforce a minimum Plugin-Version at startup in EventPluginLoader.startPlugin using pf4j's VersionManager (semver). When the descriptor version is below 3.0.0, log a clear upgrade hint and return false; the existing call chain in Manager.startEventSubscribing wraps that into TronError(EVENT_SUBSCRIBE_INIT) and aborts node startup instead of silently degrading.
…rs (tronprotocol#6761) - Add `rate.limiter.apiNonBlocking` switch (default false). When off, callers wait for a permit; when on, they reject immediately and shed load. - Wire the switch through `RateLimiterConfig` → `Args` → `CommonParameter`; update `reference.conf` / `config.conf`. - Add blocking `acquire()` paths on `IRateLimiter`, `GlobalRateLimiter`, and `QpsStrategy` / `IPQpsStrategy` / `GlobalPreemptibleStrategy`. Only the semaphore-based strategy is bounded (2s); the QPS-based paths use unbounded Guava `RateLimiter.acquire()`. - Introduce `acquirePermit()` dispatcher (default method on `IRateLimiter`; static on `GlobalRateLimiter`) that picks blocking vs non-blocking based on the switch. - Swap `tryAcquire` → `acquirePermit` at the `RateLimiterServlet` and `RateLimiterInterceptor` call sites; preserve per-endpoint-before-global ordering to avoid consuming global quota on per-endpoint rejection. - Extract `loadIpLimiter()` in `GlobalRateLimiter` and `loadLimiter()` in `IPQpsStrategy` to share cache+exception handling between `tryAcquire` and `acquire`. - Add unit tests covering dispatcher routing (both directions), acquire IP-before-global ordering, and IP-loader-failure fail-closed behaviour.
Adds a workflow_dispatch-only workflow that runs the full multinode test suite (--multinode --clean) against a 3-witness docker-compose stack. FullNode.jar is built from the PR head and injected via a local Docker wrapper image (TRON_IMAGE=java-tron-local:pr). DinD path-alignment: the compose configs are extracted from the integration-test image to the host workspace so --project-directory resolves for the host daemon. --network host is used so the test container can reach the witness nodes at 127.0.0.1:50051-50053.
workflow_dispatch requires the file to exist on the default branch, which makes it impossible to trigger from a feature branch. Add pull_request trigger on develop and release_** so the workflow fires when a PR targeting those branches is opened or updated.
📝 WalkthroughWalkthroughThis PR adds a configurable rate-limiter mode (blocking vs non-blocking), implements blocking ChangesRate Limiter API Non-Blocking Mode
Event Configuration and Plugin Version Management
CI Workflow and Miscellaneous Fixes
Sequence Diagram(s)sequenceDiagram
participant Servlet as RateLimiterServlet
participant PerEndpoint as IRateLimiter
participant Global as GlobalRateLimiter
participant Strategy as RateLimiterStrategy
Servlet->>PerEndpoint: acquirePermit(runtimeData)
alt apiNonBlocking = true
PerEndpoint->>Strategy: tryAcquire(runtimeData)
else apiNonBlocking = false
PerEndpoint->>Strategy: acquire(runtimeData)
end
Strategy-->>PerEndpoint: permit result
PerEndpoint-->>Servlet: per-endpoint acquired?
alt per-endpoint acquired
Servlet->>Global: acquirePermit(runtimeData)
alt apiNonBlocking = true
Global->>Global: tryAcquire(ip)
else apiNonBlocking = false
Global->>Global: acquire(ip)
end
Global-->>Servlet: global permit result
else per-endpoint rejected
Servlet-->>Servlet: deny request / skip global
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/integration-test-multinode.yml (1)
41-52: ⚡ Quick winAdd verification that FullNode.jar was built successfully.
The workflow copies
build/libs/FullNode.jarwithout verifying it exists. If the build silently fails or produces no artifact, the Docker build will fail with a less clear error message.✅ Proposed verification step
Add a verification step between the build and Docker image creation:
- name: Verify FullNode.jar exists run: | if [ ! -f build/libs/FullNode.jar ]; then echo "Error: FullNode.jar not found after build" exit 1 fi ls -lh build/libs/FullNode.jar🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/integration-test-multinode.yml around lines 41 - 52, Add a verification step after the "Build FullNode.jar" job to ensure build/libs/FullNode.jar exists before the Docker build: create a step named "Verify FullNode.jar exists" that checks for the file build/libs/FullNode.jar, prints an error and exits non‑zero if missing, and runs an ls -lh on the artifact so the workflow fails early with a clear message rather than letting the subsequent docker build fail cryptically.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/integration-test-multinode.yml:
- Around line 70-85: Add a brief documented acknowledgment in the Run multinode
full tests step about the security tradeoffs of using --network host and
mounting /var/run/docker.sock, and immediately mitigate by pinning the
warku123/java-tron-integration-test:latest image to a reviewed digest (replace
image reference with its SHA), add a pre-step to scan that pinned image (e.g.,
Trivy) before running docker run, and note alternatives (rootless Docker or
sysbox runtime, avoid socket mount if possible, or host a vetted image in the
tronprotocol org and enable GitHub Actions OIDC for private registry pulls) so
reviewers can see the risk and the applied mitigations.
- Around line 44-56: Replace the unpinned :latest image tags used in the
workflow steps with digest-pinned references to ensure reproducible, secure
builds: update the Dockerfile base line in the "Build local java-tron Docker
image (wraps PR-built FullNode.jar)" step so the FROM uses a specific digest
(e.g., tronprotocol/java-tron@sha256:...), and change the "Pull integration-test
image" step to pull the warku123/java-tron-integration-test image by digest
(warku123/java-tron-integration-test@sha256:...); obtain the correct digests
using docker pull + docker inspect and substitute those digest-pinned strings in
place of tronprotocol/java-tron:latest and
warku123/java-tron-integration-test:latest so the workflow always uses exact
image versions.
In
`@framework/src/main/java/org/tron/core/services/ratelimiter/GlobalRateLimiter.java`:
- Around line 34-45: The current acquire(RuntimeData) uses blocking
RateLimiter.acquire() (both for per-IP limiter returned by loadIpLimiter(ip) and
the global rateLimiter) which can block request threads; change both calls to
non-blocking tryAcquire with a reasonable timeout (e.g., a small constant like
PERMIT_TIMEOUT_MS) and if tryAcquire returns false immediately return false so
callers (via acquirePermit()/RateLimiterServlet.service() and
RateLimiterInterceptor.interceptCall()) fail fast; ensure you import/use the
same TimeUnit and update any callers/tests expecting blocking behavior
accordingly.
---
Nitpick comments:
In @.github/workflows/integration-test-multinode.yml:
- Around line 41-52: Add a verification step after the "Build FullNode.jar" job
to ensure build/libs/FullNode.jar exists before the Docker build: create a step
named "Verify FullNode.jar exists" that checks for the file
build/libs/FullNode.jar, prints an error and exits non‑zero if missing, and runs
an ls -lh on the artifact so the workflow fails early with a clear message
rather than letting the subsequent docker build fail cryptically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1ea3ef9-8e89-4e8b-8a90-4b31425cda8a
📒 Files selected for processing (28)
.github/workflows/integration-test-multinode.ymlcommon/src/main/java/org/tron/common/parameter/CommonParameter.javacommon/src/main/java/org/tron/core/config/args/EventConfig.javacommon/src/main/java/org/tron/core/config/args/RateLimiterConfig.javacommon/src/main/resources/reference.confcommon/src/test/java/org/tron/core/config/args/RateLimiterConfigTest.javaframework/src/main/java/org/tron/common/logsfilter/EventPluginLoader.javaframework/src/main/java/org/tron/core/config/args/Args.javaframework/src/main/java/org/tron/core/net/peer/PeerConnection.javaframework/src/main/java/org/tron/core/services/http/RateLimiterServlet.javaframework/src/main/java/org/tron/core/services/ratelimiter/GlobalRateLimiter.javaframework/src/main/java/org/tron/core/services/ratelimiter/RateLimiterInterceptor.javaframework/src/main/java/org/tron/core/services/ratelimiter/adapter/DefaultBaseQqsAdapter.javaframework/src/main/java/org/tron/core/services/ratelimiter/adapter/GlobalPreemptibleAdapter.javaframework/src/main/java/org/tron/core/services/ratelimiter/adapter/IPQPSRateLimiterAdapter.javaframework/src/main/java/org/tron/core/services/ratelimiter/adapter/IRateLimiter.javaframework/src/main/java/org/tron/core/services/ratelimiter/adapter/QpsRateLimiterAdapter.javaframework/src/main/java/org/tron/core/services/ratelimiter/strategy/GlobalPreemptibleStrategy.javaframework/src/main/java/org/tron/core/services/ratelimiter/strategy/IPQpsStrategy.javaframework/src/main/java/org/tron/core/services/ratelimiter/strategy/QpsStrategy.javaframework/src/main/resources/config.confframework/src/test/java/org/tron/common/logsfilter/EventLoaderTest.javaframework/src/test/java/org/tron/core/config/args/ArgsTest.javaframework/src/test/java/org/tron/core/net/messagehandler/ChainInventoryMsgHandlerTest.javaframework/src/test/java/org/tron/core/services/http/RateLimiterServletTest.javaframework/src/test/java/org/tron/core/services/ratelimiter/GlobalRateLimiterTest.javaframework/src/test/java/org/tron/core/services/ratelimiter/RateLimiterInterceptorTest.javaframework/src/test/java/org/tron/core/services/ratelimiter/adaptor/AdaptorTest.java
| - name: Build local java-tron Docker image (wraps PR-built FullNode.jar) | ||
| run: | | ||
| mkdir -p /tmp/tron-image | ||
| cp build/libs/FullNode.jar /tmp/tron-image/ | ||
| cat > /tmp/tron-image/Dockerfile <<'EOF' | ||
| FROM tronprotocol/java-tron:latest | ||
| COPY FullNode.jar /java-tron/lib/FullNode.jar | ||
| EOF | ||
| docker build -t java-tron-local:pr /tmp/tron-image | ||
|
|
||
| - name: Pull integration-test image | ||
| run: docker pull warku123/java-tron-integration-test:latest | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest stable version of tronprotocol/java-tron Docker image?
💡 Result:
Latest stable version of the tronprotocol/java-tron Docker image (the DockerHub “latest” tag) is: latest (digest sha256:a9e70a6c3…). [1] How this maps to a specific Java-Tron release: the Docker image “latest” is updated from the repository’s master branch, but DockerHub does not expose a human-readable Java-Tron version number on the page; it only provides the “latest” tag and its digest. [2] If you need the exact pinned tag (e.g., GreatVoyage-vX.Y.Z) rather than “latest”, you’ll need to check the specific tag listings or pull by digest and inspect the container’s reported version at runtime.
Citations:
- 1: https://hub.docker.com/r/tronprotocol/java-tron
- 2: https://github.com/tronprotocol/java-tron/blob/e4fac13b8ac8848de9b6450b9e429e613e63c44c/docker/docker.md
🏁 Script executed:
# Read the full workflow file to verify all claims
cat -n .github/workflows/integration-test-multinode.yml | head -100Repository: warku123/java-tron
Length of output: 4902
🌐 Web query:
warku123 java-tron-integration-test Docker image official tronprotocol
💡 Result:
Official TRON Java Docker image: tronprotocol/java-tron (Docker Hub). [1] What’s going on with “warku123 java-tron integration test Docker image”: warku123/java-tron is a GitHub fork of tronprotocol/java-tron, not an official Docker image name. [2] Official “docker image” details (tronprotocol): - The Docker Hub listing identifies “tronprotocol/java-tron” as the official Java implementation of the TRON protocol, updated about 1 month ago, with tag information including “latest”. [1] - tronprotocol’s documentation in tron-docker states there are two official Java-Tron Docker images: tronprotocol/java-tron (for Mainnet; mostly usable for Nile) and tronnile/java-tron (for Nile testnet hard-fork testing when needed). [3][4] - The same docs show example usage: docker run -it --name tron-node -d --memory="16g" -p 8090:8090 -p 50051:50051 tronprotocol/java-tron. [3][4] Integration testing from Docker (what the official tooling provides): - The official tron-docker repository includes “Gradle Docker” to automate Docker image builds and testing of java-tron. [3][4] Practical guidance (how to use the official image for testing): 1) Pull the official image: docker pull tronprotocol/java-tron [5] 2) Run a node container (Mainnet by default per docs/examples): docker run -it --name tron-node -d --memory="16g" -p 8090:8090 -p 50051:50051 tronprotocol/java-tron [3][4] 3) For CI/integration workflows, use the official tron-docker repo tooling (Gradle Docker / test-related commands) rather than relying on a third-party fork/image name. [3][4] If you want, paste the exact Docker image reference you saw for “java-tron-integration-test” (e.g., full repo/name:tag). I can verify whether it’s official (tronprotocol/*) or just a community/testing fork.
Citations:
- 1: https://hub.docker.com/r/tronprotocol/java-tron
- 2: https://github.com/warku123/java-tron
- 3: https://github.com/tronprotocol/tron-docker
- 4: https://github.com/tronprotocol/tron-docker/blob/main/README.md
- 5: https://github.com/tronprotocol/java-tron/blob/develop/quickstart.md
Pin Docker image versions for reproducibility and supply chain security.
Using :latest tags makes the workflow non-reproducible and introduces supply chain risk:
- Line 49:
tronprotocol/java-tron:latestcould change between runs - Lines 55, 66, 84:
warku123/java-tron-integration-test:latest(from a community fork, not official tronprotocol) receives full Docker daemon access (line 78), creating a supply chain vulnerability if the image or warku123's account is compromised
Unpinned images can introduce breaking changes or malicious code silently.
Recommended fix: Pin to specific digests
Replace :latest tags with digest-pinned versions:
- FROM tronprotocol/java-tron:latest
+ FROM tronprotocol/java-tron:GreatVoyage-v4.7.6@sha256:ACTUAL_DIGEST_HERE- run: docker pull warku123/java-tron-integration-test:latest
+ run: docker pull warku123/java-tron-integration-test@sha256:ACTUAL_DIGEST_HEREUpdate lines 66 and 84 similarly:
- docker create --name it-extract warku123/java-tron-integration-test:latest
+ docker create --name it-extract warku123/java-tron-integration-test@sha256:ACTUAL_DIGEST_HERE- warku123/java-tron-integration-test:latest \
+ warku123/java-tron-integration-test@sha256:ACTUAL_DIGEST_HERE \To get the digest: docker pull IMAGE:TAG && docker inspect --format='{{index .RepoDigests 0}}' IMAGE:TAG
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/integration-test-multinode.yml around lines 44 - 56,
Replace the unpinned :latest image tags used in the workflow steps with
digest-pinned references to ensure reproducible, secure builds: update the
Dockerfile base line in the "Build local java-tron Docker image (wraps PR-built
FullNode.jar)" step so the FROM uses a specific digest (e.g.,
tronprotocol/java-tron@sha256:...), and change the "Pull integration-test image"
step to pull the warku123/java-tron-integration-test image by digest
(warku123/java-tron-integration-test@sha256:...); obtain the correct digests
using docker pull + docker inspect and substitute those digest-pinned strings in
place of tronprotocol/java-tron:latest and
warku123/java-tron-integration-test:latest so the workflow always uses exact
image versions.
| - name: Run multinode full tests | ||
| run: | | ||
| # --network host: multinode tests talk to nodes via 127.0.0.1:50051 etc. | ||
| # DinD socket + HOST_WORKDIR path-alignment lets the container orchestrate | ||
| # the 3-witness compose stack via the host daemon. | ||
| # Don't override --workdir so the container's default /app entrypoint works. | ||
| docker run --name integration-multinode \ | ||
| --network host \ | ||
| -v /var/run/docker.sock:/var/run/docker.sock \ | ||
| -v "${{ github.workspace }}:${{ github.workspace }}" \ | ||
| -e HOST_WORKDIR="${{ github.workspace }}" \ | ||
| -e TRON_IMAGE=java-tron-local:pr \ | ||
| -e JAVA_HOME=/usr/lib/jvm/temurin-8 \ | ||
| -e JAVA_HOME_17=/opt/java/openjdk \ | ||
| warku123/java-tron-integration-test:latest \ | ||
| --multinode --clean |
There was a problem hiding this comment.
Document security tradeoffs of privileged container configuration.
This step uses two high-privilege patterns that warrant explicit acknowledgment:
-
--network host(line 77): Removes network isolation, exposing all host network interfaces. The container can access any localhost service and bind to any host port. -
Docker socket mount (line 78): Grants full Docker daemon control, effectively root-equivalent access. The container can manipulate any container, mount any host path, and access the entire Docker environment.
While these are necessary for the Docker-in-Docker pattern, they create significant attack surface—especially combined with the unpinned personal repository image flagged above. If warku123/java-tron-integration-test:latest is compromised, an attacker gains full host network and Docker access.
Mitigation recommendations:
- Pin the integration-test image (see previous comment) to reduce supply chain risk
- Use official/verified images if available, or self-host in the tronprotocol organization
- Consider rootless Docker or sysbox runtime to reduce privilege escalation risk
- Add image scanning (e.g., Trivy) in a pre-step to detect vulnerabilities before granting Docker access
- Use GitHub Actions OIDC for image pulls if moving to a private registry
For immediate risk reduction, at minimum pin the integration-test image to a reviewed digest.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/integration-test-multinode.yml around lines 70 - 85, Add a
brief documented acknowledgment in the Run multinode full tests step about the
security tradeoffs of using --network host and mounting /var/run/docker.sock,
and immediately mitigate by pinning the
warku123/java-tron-integration-test:latest image to a reviewed digest (replace
image reference with its SHA), add a pre-step to scan that pinned image (e.g.,
Trivy) before running docker run, and note alternatives (rootless Docker or
sysbox runtime, avoid socket mount if possible, or host a vetted image in the
tronprotocol org and enable GitHub Actions OIDC for private registry pulls) so
reviewers can see the risk and the applied mitigations.
| public static boolean acquire(RuntimeData runtimeData) { | ||
| String ip = runtimeData.getRemoteAddr(); | ||
| if (!Strings.isNullOrEmpty(ip)) { | ||
| RateLimiter r = loadIpLimiter(ip); | ||
| if (r == null) { | ||
| return false; | ||
| } | ||
| r.acquire(); | ||
| } | ||
| rateLimiter.acquire(); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Blocking acquire() calls on request handler threads can exhaust thread pool under high load.
The acquire() method calls RateLimiter.acquire() at lines 41 and 43, which are blocking operations that will wait indefinitely until a permit becomes available. This method is invoked via acquirePermit() from:
RateLimiterServlet.service()(Tomcat request handler threads)RateLimiterInterceptor.interceptCall()(gRPC request handler threads)
When rate limits are exceeded under high load, request handler threads will block waiting for permits, which can:
- Exhaust the fixed-size thread pool (both Tomcat and gRPC use bounded thread pools by default)
- Cause cascading failures as new requests queue up waiting for threads
- Lead to service unavailability
Consider one of these alternatives:
- Use
tryAcquire(timeout)with a reasonable timeout to fail fast rather than block indefinitely - Document that blocking mode requires unbounded thread pools and should be used carefully
- Move rate-limiting to a separate thread pool to isolate blocking operations
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@framework/src/main/java/org/tron/core/services/ratelimiter/GlobalRateLimiter.java`
around lines 34 - 45, The current acquire(RuntimeData) uses blocking
RateLimiter.acquire() (both for per-IP limiter returned by loadIpLimiter(ip) and
the global rateLimiter) which can block request threads; change both calls to
non-blocking tryAcquire with a reasonable timeout (e.g., a small constant like
PERMIT_TIMEOUT_MS) and if tryAcquire returns false immediately return false so
callers (via acquirePermit()/RateLimiterServlet.service() and
RateLimiterInterceptor.interceptCall()) fail fast; ensure you import/use the
same TimeUnit and update any callers/tests expecting blocking behavior
accordingly.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 28 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/integration-test-multinode.yml">
<violation number="1" location=".github/workflows/integration-test-multinode.yml:49">
P2: Use an immutable base image reference (version tag or digest) instead of `tronprotocol/java-tron:latest` for reproducible test environments.</violation>
<violation number="2" location=".github/workflows/integration-test-multinode.yml:55">
P1: Unpinned `:latest` tag from a personal Docker Hub account (`warku123/java-tron-integration-test`) combined with Docker socket access at line 78 creates a supply chain vulnerability. If the `warku123` account is compromised, the attacker gains full Docker daemon control (root-equivalent) on the runner. Pin to a digest (`@sha256:...`) or move the image to the `tronprotocol` organization to reduce this risk.</violation>
</file>
<file name="common/src/main/resources/reference.conf">
<violation number="1" location="common/src/main/resources/reference.conf:762">
P2: Changing the default `useNativeQueue` to false makes event subscription startup fail for configs that only enable `event.subscribe.enable` and rely on defaults.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| docker build -t java-tron-local:pr /tmp/tron-image | ||
|
|
||
| - name: Pull integration-test image | ||
| run: docker pull warku123/java-tron-integration-test:latest |
There was a problem hiding this comment.
P1: Unpinned :latest tag from a personal Docker Hub account (warku123/java-tron-integration-test) combined with Docker socket access at line 78 creates a supply chain vulnerability. If the warku123 account is compromised, the attacker gains full Docker daemon control (root-equivalent) on the runner. Pin to a digest (@sha256:...) or move the image to the tronprotocol organization to reduce this risk.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/integration-test-multinode.yml, line 55:
<comment>Unpinned `:latest` tag from a personal Docker Hub account (`warku123/java-tron-integration-test`) combined with Docker socket access at line 78 creates a supply chain vulnerability. If the `warku123` account is compromised, the attacker gains full Docker daemon control (root-equivalent) on the runner. Pin to a digest (`@sha256:...`) or move the image to the `tronprotocol` organization to reduce this risk.</comment>
<file context>
@@ -0,0 +1,116 @@
+ docker build -t java-tron-local:pr /tmp/tron-image
+
+ - name: Pull integration-test image
+ run: docker pull warku123/java-tron-integration-test:latest
+
+ - name: Extract compose configs to host (for DinD path-alignment)
</file context>
| mkdir -p /tmp/tron-image | ||
| cp build/libs/FullNode.jar /tmp/tron-image/ | ||
| cat > /tmp/tron-image/Dockerfile <<'EOF' | ||
| FROM tronprotocol/java-tron:latest |
There was a problem hiding this comment.
P2: Use an immutable base image reference (version tag or digest) instead of tronprotocol/java-tron:latest for reproducible test environments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/integration-test-multinode.yml, line 49:
<comment>Use an immutable base image reference (version tag or digest) instead of `tronprotocol/java-tron:latest` for reproducible test environments.</comment>
<file context>
@@ -0,0 +1,116 @@
+ mkdir -p /tmp/tron-image
+ cp build/libs/FullNode.jar /tmp/tron-image/
+ cat > /tmp/tron-image/Dockerfile <<'EOF'
+ FROM tronprotocol/java-tron:latest
+ COPY FullNode.jar /java-tron/lib/FullNode.jar
+ EOF
</file context>
|
|
||
| native = { | ||
| useNativeQueue = true | ||
| useNativeQueue = false |
There was a problem hiding this comment.
P2: Changing the default useNativeQueue to false makes event subscription startup fail for configs that only enable event.subscribe.enable and rely on defaults.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At common/src/main/resources/reference.conf, line 762:
<comment>Changing the default `useNativeQueue` to false makes event subscription startup fail for configs that only enable `event.subscribe.enable` and rely on defaults.</comment>
<file context>
@@ -758,7 +759,7 @@ event.subscribe = {
native = {
- useNativeQueue = true
+ useNativeQueue = false
bindport = 5555
sendqueuelength = 1000
</file context>
| useNativeQueue = false | |
| useNativeQueue = true |
The jacocoagent.jar is downloaded at runtime by start-multinode.sh into /app/docker/multi-node/ inside the container, but compose's path-alignment resolves ./jacocoagent.jar to the HOST path ($GITHUB_WORKSPACE/docker/multi-node/). Without a bind-mount at /app/docker/multi-node, the download never reaches the host and compose fails with "Error opening zip file: /jacoco/jacocoagent.jar".
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/integration-test-multinode.yml">
<violation number="1" location=".github/workflows/integration-test-multinode.yml:54">
P2: This workflow logs into DockerHub with repository secrets on every `pull_request`, but secrets are unavailable for fork PRs, so the integration job will fail for external contributors.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
warku123
force-pushed
the
feat/ci-multinode
branch
from
0b667ac to
f4d7ec6
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/integration-test-multinode.yml (1)
22-40: ⚡ Quick winHarden GitHub Actions for supply chain security.
Static analysis (zizmor) flags several security concerns:
Unpinned actions (lines 24, 27, 33): Using version tags like
@v5or@v4instead of commit SHAs allows supply chain attacks if the action repository is compromised.Credential persistence (line 24): The checkout action doesn't set
persist-credentials: false, meaning git credentials remain available and could leak via artifacts.♻️ Recommended fix
- name: Checkout java-tron - uses: actions/checkout@v5 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false - name: Set up JDK 8 - uses: actions/setup-java@v5 + uses: actions/setup-java@c5195efecf7bdfc987ee8bae7a71cb8b11521c00 # v4.7.1 with: java-version: '8' distribution: 'temurin' - name: Cache Gradle packages - uses: actions/cache@v4 + uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3 with:Get current SHAs via:
gh api repos/actions/checkout/git/ref/tags/v4.2.2 --jq .object.sha🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/integration-test-multinode.yml around lines 22 - 40, The workflow uses unpinned action tags (actions/checkout@v5, actions/setup-java@v5, actions/cache@v4) and leaves git credentials persistent; replace each action tag with its corresponding commit SHA (pin to the immutable git ref) and add persist-credentials: false to the Checkout step (the actions/checkout invocation) to avoid leaking credentials; update the three uses entries (checkout, setup-java, cache) to the SHAs you obtain from GitHub and keep the descriptive names the same so the steps remain identifiable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/integration-test-multinode.yml:
- Around line 22-40: The workflow uses unpinned action tags
(actions/checkout@v5, actions/setup-java@v5, actions/cache@v4) and leaves git
credentials persistent; replace each action tag with its corresponding commit
SHA (pin to the immutable git ref) and add persist-credentials: false to the
Checkout step (the actions/checkout invocation) to avoid leaking credentials;
update the three uses entries (checkout, setup-java, cache) to the SHAs you
obtain from GitHub and keep the descriptive names the same so the steps remain
identifiable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 624ba88d-f44c-4396-ba19-36ac40d12f4f
📒 Files selected for processing (1)
.github/workflows/integration-test-multinode.yml
4 participants
Summary
workflow_dispatch+pull_requesttriggered workflow that runs the full multinode integration test suite (--multinode --clean) against a 3-witness docker-compose stackTRON_IMAGE=java-tron-local:pr)--project-directoryresolves for the host daemon--network hostis used so the test container can reach witness nodes at127.0.0.1:50051-50053Test plan
develop🤖 Generated with Claude Code
Summary by cubic
Adds a multinode CI workflow (Full), configurable blocking vs non‑blocking API rate limiting, and enforces event‑plugin >= 3.0.0 to prevent silent failures.
New Features
FullNode.jar, inject viajava-tron-local:pr, use host networking, extract compose files to the host, bind‑mount the multinode dir for Jacoco, collect reports/logs, and clean up. Usestroninfra/troninfra-ci:latest(public).rate.limiter.apiNonBlocking(default false) and blockingacquire()paths, with anacquirePermit()dispatcher onIRateLimiterandGlobalRateLimiter.pf4j; defaultevent.subscribe.native.useNativeQueueto false.Bug Fixes
--esnow takes effect: apply event config after CLI and build plugin config when enabled only via CLI.acquirePermit().relayNodesinPeerConnection.Written for commit 564770a. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes
Configuration Changes
rate.limiter.apiNonBlocking(defaults to false)Tests