Skip to content

Conversation

@diegomrsantos
Copy link
Member

Issue Addressed

Fixes CI failure: https://github.com/sigp/anchor/actions/runs/20666620673/job/59340149125

Proposed Changes

  • Update alloy from 1.0.22 to 1.2.1
  • Pin libp2p-core and libp2p-swarm to exact versions to resolve patch ambiguity
  • Updates ruint from 1.17.0 to 1.17.2, fixing RUSTSEC-2025-0137 (unsoundness in reciprocal_mg10)

Additional Info

The ruint vulnerability (RUSTSEC-2025-0137) was causing cargo audit to fail in CI. The fix requires ruint >= 1.17.1.

- Update alloy from 1.0.22 to 1.2.1
- Pin libp2p-core and libp2p-swarm to exact versions to resolve patch ambiguity
- Fixes RUSTSEC-2025-0137 (ruint unsoundness in reciprocal_mg10)
@claude-code-actions-sigp
Copy link

claude-code-actions-sigp bot commented Jan 2, 2026

Claude finished @diegomrsantos's task in 1m 54s —— View job


Code Review Complete

Tasks

  • Read Cargo.toml changes
  • Verify security vulnerability fix
  • Check for potential breaking changes in alloy
  • Review libp2p version pinning strategy
  • Check CI test results
  • Provide comprehensive feedback

Summary

This PR appropriately addresses a security vulnerability and updates dependencies. The changes are focused and minimal, which is good for a dependency update. CI tests are passing.

Detailed Review

1. Security Fix ✅

RUSTSEC-2025-0137 (ruint unsoundness): The PR correctly updates ruint from 1.17.0 to 1.17.2, which fixes the unsoundness issue in reciprocal_mg10. This vulnerability is brought in transitively through alloy, and the update to alloy 1.2.1 resolves it.

Verification: Cargo.lock:164 shows ruint version updated to 1.17.2, which is the patched version (fix requires >= 1.17.1).

2. Alloy Update (1.0.22 → 1.2.1)

Impact Assessment:

  • This is a minor version bump within the 1.x series, so breaking changes are not expected per semver
  • The PR author should have tested compilation and basic functionality
  • Usage in codebase: alloy is used in 4 crates:
    • anchor/eth (execution layer sync, event processing, contract interactions)
    • anchor/subnet_service (U256 from ruint)
    • anchor/keysplit (key splitting operations)
    • anchor/common/ssv_network_config (network configuration)

Observations:

  • The update spans 4 minor versions (1.0.22 → 1.2.1), which could introduce subtle API changes
  • No code changes were required, suggesting API compatibility
  • CI tests are in progress and early checks have passed

3. libp2p Version Pinning

Change: Modified patch section to use exact version matching (=0.43.1 instead of 0.43.1)

[patch.'https://github.com/libp2p/rust-libp2p.git']
-libp2p-core = "0.43.1"
-libp2p-swarm = "0.47.0"
+libp2p-core = "=0.43.1"
+libp2p-swarm = "=0.47.0"

Analysis:

  • Purpose: The PR description mentions this resolves "patch ambiguity"
  • Effect: The = prefix forces exact version matching, preventing Cargo from selecting any patch version (e.g., 0.43.1 vs 0.43.2)
  • Context: This patch section exists because of a TODO comment: "remove when there is a proper release for peer-store"

Considerations:

  • Exact version pinning is more restrictive but provides deterministic builds
  • This is appropriate for patching specific upstream commits where you need exact version control
  • The approach aligns with the temporary nature indicated by the TODO comment

4. Code Quality

Positive aspects:

  • Minimal, focused changes
  • Clear commit message following conventional commits format
  • PR description provides context and links to CI failure
  • No unnecessary refactoring mixed with dependency updates

5. Testing

CI Status: Tests are in progress. Key jobs:

  • ✅ check-fmt: Passed
  • ✅ cargo-sort: Passed
  • ✅ spellcheck: Passed
  • ✅ lockbud: Passed
  • ⏳ check-code: In progress
  • ⏳ check-msrv: In progress
  • ⏳ release-tests-ubuntu: In progress
  • ⏳ debug-tests-ubuntu: In progress
  • ⏳ cargo-udeps: In progress

Recommendations

  1. Monitor CI completion: Ensure all tests pass, particularly:

    • Compilation checks (check-code, check-msrv)
    • Unit and integration tests (release-tests-ubuntu, debug-tests-ubuntu)
    • Dependency audits (cargo-udeps)
  2. Consider alloy changelog review: While no code changes were required, reviewing the alloy changelog for versions 1.0.22 through 1.2.1 would help identify any behavioral changes or deprecations.

  3. Future cleanup: Remember to address the TODO comment at Cargo.toml:162 about removing the libp2p patch when there's a proper peer-store release.

Conclusion

The PR correctly addresses the security vulnerability and makes appropriate dependency updates. The approach is conservative and well-documented. Once CI completes successfully, this is ready to merge.


Copy link
Member

@jking-aus jking-aus left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link

@shane-moore shane-moore left a comment

Choose a reason for hiding this comment

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

@diegomrsantos, i was able to resolve this over in lighthouse a few days ago in PR by bumping the ruint version. here, did you bump the alloy version, and then got the updated ruint as a side effect? jc. it looks like the build passes so don't see an issue with this approach

I'm unfamiliar with the libp2p dependency source overrides so hard for me to comment on whether pinning the versions is necessary or otherwise

@diegomrsantos
Copy link
Member Author

diegomrsantos commented Jan 5, 2026

@diegomrsantos, i was able to resolve this over in lighthouse a few days ago in PR by bumping the ruint version. here, did you bump the alloy version, and then got the updated ruint as a side effect? jc. it looks like the build passes so don't see an issue with this approach

I'm unfamiliar with the libp2p dependency source overrides so hard for me to comment on whether pinning the versions is necessary or otherwise

I understand that ruint is a transitive dependency from alloy. I'd appreciate a double check

@shane-moore
Copy link

@diegomrsantos, yep, ruint is transitive dependency of alloy. so makes sense that bumping alloy resolved the issue here. i went with the cargo update ruint approach just to fix the vulnerability at the lowest dependency possible as to keep blast radius small

@mergify mergify bot merged commit b91656a into sigp:unstable Jan 5, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants