Skip to content

Conversation

@fredcarle
Copy link
Collaborator

@fredcarle fredcarle commented Oct 4, 2025

Relevant issue(s)

Resolves #4048
Resolves #4031

Description

This PR removes the defradb/net package in favour of sourcenetwork/go-p2p. It brings in the latest changes from go-p2p which implies making changes to the client.Host interface.

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

make test

Specify the platform(s) on which this was tested:

  • MacOS

@fredcarle fredcarle added this to the DefraDB v0.20 milestone Oct 4, 2025
@fredcarle fredcarle requested a review from a team October 4, 2025 01:03
@fredcarle fredcarle self-assigned this Oct 4, 2025
@fredcarle fredcarle added area/p2p Related to the p2p networking system area/network Related to internal/external network components labels Oct 4, 2025
@fredcarle fredcarle force-pushed the fredcarle/chore/use-go-p2p branch from e8368fc to 6c3acaa Compare October 4, 2025 01:54
@codecov
Copy link

codecov bot commented Oct 4, 2025

Codecov Report

❌ Patch coverage is 80.96774% with 59 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.46%. Comparing base (ef638df) to head (75ebc11).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
internal/db/p2p/replicator.go 63.64% 20 Missing and 8 partials ⚠️
internal/db/txn.go 0.00% 7 Missing ⚠️
cli/test/action/p2p_replicator_getall.go 78.26% 4 Missing and 1 partial ⚠️
http/handler_p2p.go 84.00% 3 Missing and 1 partial ⚠️
cbindings/p2p.go 76.92% 2 Missing and 1 partial ⚠️
cli/p2p_info.go 40.00% 2 Missing and 1 partial ⚠️
cbindings/node.go 0.00% 2 Missing ⚠️
cbindings/wrapper.go 87.50% 2 Missing ⚠️
http/client_p2p.go 85.71% 2 Missing ⚠️
cli/start.go 80.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4051      +/-   ##
===========================================
+ Coverage    74.21%   74.46%   +0.24%     
===========================================
  Files          489      488       -1     
  Lines        45491    45230     -261     
===========================================
- Hits         33761    33677      -84     
+ Misses        9445     9298     -147     
+ Partials      2285     2255      -30     
Flag Coverage Δ
all-tests 74.46% <80.97%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cli/p2p_connect.go 100.00% <100.00%> (+16.67%) ⬆️
cli/p2p_replicator_delete.go 100.00% <100.00%> (+13.04%) ⬆️
cli/p2p_replicator_getall.go 84.21% <100.00%> (+26.32%) ⬆️
cli/p2p_replicator_set.go 100.00% <100.00%> (+13.04%) ⬆️
cli/test/action/p2p_replicator_delete.go 100.00% <100.00%> (ø)
cli/test/action/p2p_replicator_set.go 100.00% <100.00%> (ø)
client/p2p.go 0.00% <ø> (-100.00%) ⬇️
http/openapi.go 97.52% <ø> (ø)
internal/core/block/store.go 63.43% <100.00%> (+1.71%) ⬆️
internal/datastore/blockstore.go 54.90% <ø> (-1.70%) ⬇️
... and 18 more

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef638df...75ebc11. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this Fred :)

I have a couple of requests that I would like resolving before merge, but they are all pretty localised.

Example: Connect to a peer
defradb client p2p connect /ip4/0.0.0.0/tcp/9171/p2p/12D3KooW...
Example: Connect to multiple peers
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Thanks for adding this

@fredcarle fredcarle force-pushed the fredcarle/chore/use-go-p2p branch from 6c3acaa to c43dbf3 Compare October 4, 2025 17:41
@fredcarle fredcarle requested a review from AndrewSisley October 4, 2025 17:41
tracer = telemetry.NewTracer()
)

type (
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: I had no idea we could do this, although it makes sense from a syntax perspective :) Thanks :)

@fredcarle fredcarle force-pushed the fredcarle/chore/use-go-p2p branch from c43dbf3 to 8fd3222 Compare October 8, 2025 01:30
storedRep.CollectionIDs = append(storedRep.CollectionIDs, col.CollectionID())
// Update the list of collections for each replicator prior to persisting.
storedRepCollectionIDs := make(map[string]map[string]struct{}) // replicatorID => collectionID
addedCols := make(map[string][]client.Collection) // peerID => list of collections added
Copy link
Member

Choose a reason for hiding this comment

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

question: What does this comment mean? peerID => list of collections added

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it means its a map of peerIDs to the list of collections added.

@fredcarle fredcarle force-pushed the fredcarle/chore/use-go-p2p branch from 9702b90 to 17e0b3b Compare October 9, 2025 18:53
@fredcarle fredcarle force-pushed the fredcarle/chore/use-go-p2p branch from 17e0b3b to d124e4d Compare October 16, 2025 18:07
@fredcarle fredcarle force-pushed the fredcarle/chore/use-go-p2p branch from 96f57a8 to 75ebc11 Compare October 17, 2025 13:42
@fredcarle fredcarle merged commit 83cdb74 into sourcenetwork:develop Oct 17, 2025
86 of 89 checks passed
@fredcarle fredcarle deleted the fredcarle/chore/use-go-p2p branch October 17, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/network Related to internal/external network components area/p2p Related to the p2p networking system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace Defra net package with go-p2p repo Drefra should automatically try to connect to bootstrap nodes on startup

3 participants