Bitswap support in Libp2p helper #9626

georgeee · 2021-10-15T17:59:18Z

Explain your changes:

This PR adds support of Bitswap in libp2p helper:

It introduces block format to split big data blobs into smaller bitswap blocks
- Encoding and decoding algorithms for the format are also provided
- Bitswap support relies heavily on this format to greatly reduce DoS attack surface
It integrates with bitswap using LMDB as block storage

Explain how you tested your changes:

There are four layers of testing

Testing encoding/decoding of block format
Testing small-step semantics of maintaining downloading state of a single resource
Testing large-step semantics of maintaining state of multiple resources
Testing bitswap in localhost network
- Network is configured to have maximum 15 connections for each node
- Testing 500-node network on one run
- Testing 100-node network on 10 runs
- Testing 20-node network on 100 runs

Testing on network involves random resource generation and random requesting of the resource by different nodes, deleting some resources from some nodes and then retrying random resource requesting.

Note that testing requires up to 30 GB of RAM and ulimit set to some large value (tested on 100000).

Checklist:

Modified the current draft of release notes with details on what is completed or incomplete within this project
Document code purpose, how to use it
- Mention expected invariants, implicit constraints
Tests were added for the new behavior
- Document test purpose, significance of failures
- Test names should reflect their purpose
All tests pass (CI will check this if you didn't)
Serialized types are in stable-versioned modules
Does this close issues? List them

Closes #0000

nholland94

Looks great overall, just some small suggestions. I haven't fully reviewed the tests yet, but all of the implementation looks good.

nholland94 · 2021-12-01T14:08:50Z

src/app/libp2p_helper/BUILD.bazel

@@ -1,3 +0,0 @@
-load("@bazel_gazelle//:def.bzl", "gazelle")


Why were the bazel build related files removed? AFAIK, we have a contractor that's working on bazel support for the repository, so doing this will probably step on their toes.

They are broken now. If someone is working on these, let them commit working build files once they're ready, maybe?

nholland94 · 2021-12-01T14:10:11Z

src/app/libp2p_helper/Makefile

 ../../libp2p_ipc/libp2p_ipc.capnp.go:
 make -C ../../libp2p_ipc libp2p_ipc.capnp.go

 libp2p_helper: ../../libp2p_ipc/libp2p_ipc.capnp.go
 $(WRAPAPP) ../../../scripts/build-go-helper.sh libp2p_helper

 test: ../../libp2p_ipc/libp2p_ipc.capnp.go
- $(WRAPAPP) ../../../scripts/build-go-helper.sh --test libp2p_helper
+ cd src/libp2p_helper && $(GO) test -short -timeout 15m # && $(GO) test -timeout 40m -run "^TestBitswapJumbo$$"


Is the commented version of the test command running different tests? Should we provide multiple Makefile targets for different kinds of tests?

Yeah, I will experiment with these a bit more

I wasn't able to run BitswapMedium and BitswapJumbo tests in CI, so left them as make test-large which can be executed on a developer machine.

Other tests are now being executed in CI

P.S. BitswapMedium and BitswapJumbo tests timeout in CI, on my laptop both of them complete in an hour, occupying 90% of 32 GB RAM and a chunk of processor power though.

Ok. These seem like good candidates for nightly CI tests, once we have those setup. Perhaps we can revisit putting them in CI once that infrastructure exists.

nholland94 · 2021-12-01T14:26:23Z

src/app/libp2p_helper/src/libp2p_helper/bitswap.go

+ }
+ delete(states, root)
+ state.allDescedants.ForEach(func(c cid.Cid) error {
+ np, hasNp := nodeParams[c]


Probably worth noting in a comment that this will need modified when we wish to support shared nodes across Bitswap DAGs (which we will want/need to whenever we put ledger into Bitswap).

@nholland94 could you expand on this please? I'm not following you here.

Shared nodes are already supported or at least were meant to be.

If I understand this algorithm, whenever you clear the download state for a root, you clear that state for all descendants as well. If in the future we have shared nodes across roots, then you would need to keep a refcount for all the descendant nodes and only clear their download state when the refcount reaches 0.

src/app/libp2p_helper/src/libp2p_helper/bitswap.go

src/app/libp2p_helper/src/libp2p_helper/bitswap_downloader.go

src/app/libp2p_helper/src/bitswap_storage.go

src/app/libp2p_helper/src/libp2p_helper/app.go

src/app/libp2p_helper/src/libp2p_helper/bitswap_downloader.go

nholland94 · 2021-12-01T15:30:31Z

src/app/libp2p_helper/src/libp2p_helper/bitswap_downloader.go

+ rootDownloadStates := bs.RootDownloadStates()
+ depthIndices := bs.DepthIndices()
+ oldPs, foundRoot := nodeDownloadParams[id]
+ delete(nodeDownloadParams, id)


What happens if we a processing the same downloaded block in parallel? Is that possible? I'm worried about potential race conditions on accessing the node download params.

You can check that Loop is only ran once. All these data structures are accessed only from functions called from within the Loop(). These are most importantly kickStartRootDownload and processDownloadedBlock. You can check that within these functions no new goroutines are produced.

So, basically, the execution is single-threaded by design.

src/app/libp2p_helper/src/libp2p_helper/bitswap.go

Plus allow to configure Bitswap with Config message and fix Daemon compilation.

+ Some bugfix

+ abstract block downloading logic from irrelevant implementation details

Implement test for big-step semantic of block download state machine.

Problem: jumbo test is resource-intensive and is better to be executed separately. Solution: skip jumbo test in "short" mode.

They fail in CI due to lack of resources currently, but can be executed on developer's laptop.

Problem: case of non-empty block storage is not well covered. An existing bug wasn't caught due to this (was caught only via PR review). Solution: add test (which fails).

Some stylistic changes + bugfix

bkase

Product review is good otherwise

bkase · 2022-01-05T17:20:31Z

src/app/libp2p_helper/LICENSE

@@ -1,201 +0,0 @@
- Apache License


can you un-remove the license?

@bkase my intuition was that LICENSE at the root of Mina repository applies here unless specified otherwise.

https:/MinaProtocol/mina/blob/compatible/LICENSE this file

sounds good 👍

georgeee requested review from bkase, imeckler, mrmr1993, psteckler and a team as code owners October 15, 2021 17:59

georgeee changed the title ~~Bitswap support~~ [WIP] Bitswap support Oct 15, 2021

georgeee force-pushed the georgeee/bitswap branch 2 times, most recently from 1479013 to 1876539 Compare October 21, 2021 19:39

georgeee force-pushed the georgeee/bitswap branch from 6d04c0b to ba52882 Compare November 15, 2021 21:03

georgeee changed the title ~~[WIP] Bitswap support~~ Bitswap support in Libp2p helper Nov 15, 2021

georgeee force-pushed the georgeee/bitswap branch from ba52882 to bd63db8 Compare November 22, 2021 16:01

georgeee added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Nov 22, 2021

georgeee force-pushed the georgeee/bitswap branch 10 times, most recently from fbf0a5c to 9234741 Compare November 29, 2021 17:20

nholland94 requested changes Dec 1, 2021

View reviewed changes

georgeee force-pushed the georgeee/bitswap branch 5 times, most recently from 3a3e46b to 3116ffc Compare December 29, 2021 08:41

nholland94 approved these changes Jan 5, 2022

View reviewed changes

georgeee added 21 commits January 5, 2022 20:06

Implement bitswap block split and join

baa9e91

Remove non-functional nix, bazel build files

e68738e

Bump deps for libp2p_helper

2d50309

Implement bitswap-based resource downloading

a905cfb

Allow to configure min connections

caecb29

Plus allow to configure Bitswap with Config message and fix Daemon compilation.

Implement test for Bitswap; lot of bugfix

fd7b6c6

Implement length-prefix+tag encoding for blocks

1e9f9de

+ Some bugfix

Fix incorrect handling of DAGs

49e937b

+ abstract block downloading logic from irrelevant implementation details

Implement test for processDownloadedBlockImpl

836fd98

Fix tiny glitch in test sample generation

1723322

Implement big-step test for block download

d8713dc

Implement test for big-step semantic of block download state machine.

Implement TestBroadcastInNotFullConnectedNetwork

f0912bf

Make bitswap tests pass

9d33b71

Make TestBitswapJumbo a "long" test

110af96

Problem: jumbo test is resource-intensive and is better to be executed separately. Solution: skip jumbo test in "short" mode.

Minor fix of tests

3e51236

Update libp2p test execution in Makefile

c783232

Try to make tests work

a2619bc

Add large tests to Makefile

2946b4e

They fail in CI due to lack of resources currently, but can be executed on developer's laptop.

Test case of some bitswap blocks being known

6fe9026

Problem: case of non-empty block storage is not well covered. An existing bug wasn't caught due to this (was caught only via PR review). Solution: add test (which fails).

Apply post-review fixes

b237d8b

Some stylistic changes + bugfix

Run BitswapQC test in CI

53318b9

georgeee force-pushed the georgeee/bitswap branch from 3116ffc to 53318b9 Compare January 5, 2022 17:08

bkase reviewed Jan 5, 2022

View reviewed changes

bkase approved these changes Jan 5, 2022

View reviewed changes

georgeee merged commit d653a68 into compatible Jan 6, 2022

georgeee deleted the georgeee/bitswap branch January 6, 2022 11:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bitswap support in Libp2p helper #9626

Bitswap support in Libp2p helper #9626

georgeee commented Oct 15, 2021 •

edited

Loading

nholland94 left a comment

nholland94 Dec 1, 2021

georgeee Dec 1, 2021 •

edited

Loading

nholland94 Dec 1, 2021

georgeee Dec 1, 2021

georgeee Dec 29, 2021

georgeee Dec 29, 2021 •

edited

Loading

nholland94 Jan 5, 2022

nholland94 Dec 1, 2021

georgeee Dec 27, 2021 •

edited

Loading

nholland94 Jan 5, 2022

nholland94 Dec 1, 2021

georgeee Dec 27, 2021

bkase left a comment

bkase Jan 5, 2022

georgeee Jan 5, 2022

georgeee Jan 5, 2022

bkase Jan 5, 2022

Bitswap support in Libp2p helper #9626

Bitswap support in Libp2p helper #9626

Conversation

georgeee commented Oct 15, 2021 • edited Loading

nholland94 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgeee Dec 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgeee Dec 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgeee Dec 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkase left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgeee commented Oct 15, 2021 •

edited

Loading

georgeee Dec 1, 2021 •

edited

Loading

georgeee Dec 29, 2021 •

edited

Loading

georgeee Dec 27, 2021 •

edited

Loading