Skip to content

#418 libp2p integration #420

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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

NhoxxKienn
Copy link
Contributor

@NhoxxKienn NhoxxKienn commented Apr 25, 2025

Description

This PR solves the issue #418 of go-perun, which is part of the release v0.14.0 (#417). It migrates the wire implementation of Libp2p from perun-libp2p-wire directly into go-perun to be able to easily maintain in the future.

Location:

The implementation will be located in wire/net/libp2p

Features:

  • Libp2p's client wire implementation.
  • Unit tests
  • Integration test with go-perun's wire/net interface

…al channel in some chains

Signed-off-by: Minh Huy Tran <[email protected]>
@hyperledger-labs hyperledger-labs deleted a comment from mergify bot Apr 28, 2025
@NhoxxKienn NhoxxKienn requested review from Copilot and iljabvh April 30, 2025 11:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the Libp2p wire implementation into go‑perun and refactors related encoding, decoding, and testing functions to improve maintainability and integration. Key changes include incorporating new methods for balances and sub‑allocations, updating state types in the adjudicator, and removing duplicate or outdated functions.

Reviewed Changes

Copilot reviewed 164 out of 166 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
channel/allocation_test.go Replaces traditional loop iterations with “for range” on integer literals
channel/allocation.go Adds new balances methods and backend bounds checks; introduces an invalid loop in Sum
channel/adjudicator.go Updates state mappings from *State to *SignedState
channel/actionmachine.go Moves setStaging method without modifying behavior
backend/sim/wire/address.go Introduces a new random address function and removes its duplicate
backend/sim/wire/account.go Removes the duplicate NewRandomAccount implementation
backend/sim/wallet/wallet_test.go Replaces assert with require for stricter error checking
backend/sim/wallet/wallet_internal_test.go Switches loop iterations to “for range” on integer literals
backend/sim/wallet/wallet.go Introduces documentation for the Wallet struct and removes duplicate code
backend/sim/wallet/address_internal_test.go Uses “for range” on an int variable in test loops
backend/sim/wallet/address.go Introduces a new NewRandomAddress (with minor formatting issues)
backend/sim/channel/asset_test.go Updates loop iterations to use “for range” on integer literals
backend/sim/channel/asset.go Adds bounds checks to asset unmarshalling and ensures asset ID validity
backend/sim/channel/app.go Removes duplicate NewRandomAppID implementation
apps/payment/resolver_internal_test.go Uses require instead of assert for error checking
apps/payment/randomizer_internal_test.go Minimal adjustments on loop iterators
apps/payment/app_internal_test.go Uses “for range” on an int where a traditional loop is expected
.github/workflows/ci.yml Updates action versions for checkout, Go setup, and golangci-lint
Files not reviewed (2)
  • .golangci.json: Language not supported
  • .golangci.yml: Language not supported
Comments suppressed due to low confidence (5)

channel/allocation_test.go:166

  • Using 'for range 10' on an integer literal is not valid in Go. Please revert to a traditional for loop (e.g., for i := 0; i < 10; i++) to iterate the desired number of times.
for range 10 {

channel/allocation.go:309

  • Iterating with 'for i := range n' is invalid because 'n' is an integer. Consider iterating over the slice (e.g., for i := 0; i < n; i++) or using 'for i := range b' if b is the slice.
for i := range n {

backend/sim/wallet/wallet_internal_test.go:36

  • Using 'for range 10' on an integer literal is not valid in Go. Please use a traditional loop to iterate a fixed number of times.
for range 10 {

backend/sim/wallet/address_internal_test.go:53

  • Iterating with 'for i := range zeros' is invalid since 'zeros' is an integer. Use a conventional loop (e.g., for i := 0; i < zeros; i++) instead.
for i := range zeros {

apps/payment/app_internal_test.go:96

  • Looping with 'for i := range numParticipants' is invalid since numParticipants is an integer. Use a traditional loop such as 'for i := 0; i < numParticipants; i++' instead.
for i := range numParticipants {

@NhoxxKienn NhoxxKienn linked an issue May 14, 2025 that may be closed by this pull request
4 tasks
@iljabvh iljabvh requested a review from DragonDev1906 May 30, 2025 08:24
Copy link
Contributor

@DragonDev1906 DragonDev1906 left a comment

Choose a reason for hiding this comment

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

Feedback for future PRs:
Try to put changes like linter updates in a separate PR, especially when they need a lot of changes or want to move code around. Reviewing a PR is significantly easier if it is smaller and does one thing. When you need to update the linter during another change I'd suggest to temporarily disable the linters that require many changes (for example the one about the order of functions in a file or the integer overflow one) and fix them in a separate PR. That keeps the actual changes separate from changes that don't do much.

I have not looked too deep into wire/net/libp2p, as if I understood it correctly it was copied from another repository and (probably) only has smaller changes (e.g. for the linter).

As hinted at in the comments: There are some changes I'd put into a separate PR, as they are not about the libp2p integration but a more general issue I've noticed during review.

@NhoxxKienn NhoxxKienn requested a review from DragonDev1906 July 15, 2025 07:31
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.

🧩 Libp2p in go-perun
3 participants