Skip to content

Conversation

hannahhoward
Copy link
Collaborator

@hannahhoward hannahhoward commented Sep 21, 2022

Goals

Provide a mechanism for filtering out illegal CID requests in bitswap.

Implementation

  • This implements a lightweight block filter module that fetches the bad bits list and checks bitswap requests against what is in the list.
    • The list downloads and updates every five minutes in the background -- currently, it's only 1meg large
    • The in place block check is O(1) against an in memory copy of the list (currently, again, not a serious memory overhead)
  • integrates the block filter into the remote blockstore in booster bitswap

For discusion (that's why it's a draft)

  • Currently, this lives inside of booster bitswap -- should it instead live inside of boost and possibly the block API calls?

  • I didn't put optimization around passing If-Not-Modified as request header and checking for a 304 Not Modified response -- this may be prudent as the list updates a lot but not every five minutes

  • There's no on disk cache -- this was done for simplicity but does mean you need to fetch at startup. Also, this is a blocking call currently -- maybe that's not ideal?

  • What is the right way to handle fetching/parsing errors? Right now it's just log.Errorf

  • blocked on LoadBalancer for bitswap (and later, more of libp2p) #786 (currently that branch is the base for this one)

Base automatically changed from feat/libp2p-loadbalancer to release/lotus1.17.2 September 22, 2022 07:47
@dirkmc
Copy link
Contributor

dirkmc commented Sep 22, 2022

Currently, this lives inside of booster bitswap -- should it instead live inside of boost and possibly the block API calls?

Bitswap has a built-in mechanism for deny-lists that we could use.

I didn't put optimization around passing If-Not-Modified as request header and checking for a 304 Not Modified response -- this may be prudent as the list updates a lot but not every five minutes

I think that's a good idea - we expect the size of the list to grow in future so it would be good to avoid unnecessary downloads

There's no on disk cache -- this was done for simplicity but does mean you need to fetch at startup. Also, this is a blocking call currently -- maybe that's not ideal?

I'd suggest we do use an on-disk cache. The size of the list is likely to increase in future. I'd suggest that on startup

  • if the list hasn't ever been fetched, block on fetching it
  • if there is already an on-disk cache, just do a new fetch in the background

What is the right way to handle fetching/parsing errors? Right now it's just log.Errorf

That sounds right to me - I think it makes sense for the denylist to be best-effort

blocked on #786 (currently that branch is the base for this one)

Merged ✅

@hannahhoward hannahhoward marked this pull request as ready for review September 23, 2022 06:19
@hannahhoward
Copy link
Collaborator Author

@dirkmc

PR updated:

  • uses bitswap peer request block filter feature in bitswap for filtering
  • supports if-not-modified when fetching deny list over http
  • holds on disk cache of last fetched list

Copy link
Contributor

@dirkmc dirkmc left a comment

Choose a reason for hiding this comment

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

I've approved this PR, with a few suggestions for minor improvements. Nice work! 🙌

@dirkmc
Copy link
Contributor

dirkmc commented Sep 23, 2022

My one concern here is what happens as the badbits list grows large. Is there a way we can put a reasonable bound on the amount of memory reserved for the badbits filter? Would it make sense to use something like leveldb to keep it on disk instead?
We can experiment with solutions in a follow-up PR.

@hannahhoward
Copy link
Collaborator Author

@dirkmc yes, we can definitely do follow up experiments if the list grows large. One interesting thing to note: currently, the list is only about 1MB large, way smaller than I would have guessed given how long it's been in operation.

@hannahhoward hannahhoward merged commit c9d91e6 into release/lotus1.17.2 Sep 26, 2022
LexLuthr pushed a commit that referenced this pull request Oct 4, 2022
* booster bitswap MVP executable (#707)

* feat(booster-bitswap): booster bitswap MVP untested

* refactor(booster-bitswap): use API for fetching blocks

* fix(deps): update deps to compile

* feat(booster-bitswap): makefile & fixes

add commands to build booster-bitswap, and very a round tripped successful fetch from
booster-bitswap

* refactor: clean up unused vars etc

* fix: booster-bitsawp - check error when creating libp2p key

* refactor(node): avoid FreeAndUnsealed method

Co-authored-by: Dirk McCormick <[email protected]>
Co-authored-by: Anton Evangelatov <[email protected]>

* booster-bitswap devnet and tracing (#796)

* return ipld ErrNotFound from remote blockstore interface (#798)

* fix: return ipld ErrNotFound from remote blockstore interface

* test: add more tests for ipld ErrNotFound

* test: comment out part of TestDummydealOnline that is flaky due to a bug in latest lotus (#802)

* fix normaliseError nil ptr dereference (#803)

* feat: shard selector (#807)

* LoadBalancer for bitswap (and later, more of libp2p) (#786)

* feat(loadbalancer): add message types

* feat(messages): add utility functions

* feat(loadbalancer): initial load balancer impl

implementation of the load balancer node itself

* feat(loadbalancer): add service node

implements code for running a service node

* feat(loadbalancer): integrate into boost and booster-bitswap

* Update loadbalancer/loadbalancer.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <[email protected]>

* refactor(loadbalancer): remove routing protocol

remove the routing protocol, instead relying on a set config. also remove forwarding response for
inbound requests

* fix(loadbalancer): update tests

* refactor(loadbalancer): integrate simplified load balancer

removed pub keys to minimize network traffic, added api's to configure and update bitswap peer id,
added auto config of bitswap peer id in booster-bitswap

* docs(gen): regenerate api docs

* chore(lint): fix lint errors

* fix(loadbalancer): minor bridgestream fix

* Update loadbalancer/servicenode.go

Co-authored-by: dirkmc <[email protected]>

* refactor(protocolproxy): address PR comments

renames, reconfigured architecture, etc

* refactor(make init print out peer id): remove apis and transparent peer id setting. have init print

Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: dirkmc <[email protected]>

* Add block filter via BadBits (#825)

* feat(booster-bitswap): add block filter via BadBits

* refactor(booster-bitswap): use bitswap blockfilter for filtering

* feat(blockfilter): only update when list is modified

* feat(blockFilter): add on disk caching

* Update cmd/booster-bitswap/blockfilter/blockfilter.go

Co-authored-by: dirkmc <[email protected]>

* fix(blockfilter): minor PR fixups

Co-authored-by: dirkmc <[email protected]>

* Libp2p 0.22 upgrade (#837)

* chore(deps): upgrade to Lotus RC & libp2p v0.22

* chore(deps): update go to 1.18

* ci(circle): update circle to go 1.18

* style(imports): fix imports

* fix(build): update ffi

* fix(lint): fix deprecated strings.Title method

* fix(mod): mod tidy

* Protocol Proxy cleanup (#836)

* refactor(booster-bitswap): minor UI fixes for booster-bitswap UI

* Update cmd/booster-bitswap/init.go

Co-authored-by: dirkmc <[email protected]>

Co-authored-by: dirkmc <[email protected]>

* feat: update to dagstore v0.5.5 (#849)

* add booster-bitswap to devnet (#866)

* bump lotus-test version

* add docker/booster-bitswap target in Makefile

Co-authored-by: Hannah Howard <[email protected]>
Co-authored-by: Dirk McCormick <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
dirkmc added a commit that referenced this pull request Oct 4, 2022
* booster bitswap MVP executable (#707)

* feat(booster-bitswap): booster bitswap MVP untested

* refactor(booster-bitswap): use API for fetching blocks

* fix(deps): update deps to compile

* feat(booster-bitswap): makefile & fixes

add commands to build booster-bitswap, and very a round tripped successful fetch from
booster-bitswap

* refactor: clean up unused vars etc

* fix: booster-bitsawp - check error when creating libp2p key

* refactor(node): avoid FreeAndUnsealed method

Co-authored-by: Dirk McCormick <[email protected]>
Co-authored-by: Anton Evangelatov <[email protected]>

* booster-bitswap devnet and tracing (#796)

* return ipld ErrNotFound from remote blockstore interface (#798)

* fix: return ipld ErrNotFound from remote blockstore interface

* test: add more tests for ipld ErrNotFound

* test: comment out part of TestDummydealOnline that is flaky due to a bug in latest lotus (#802)

* fix normaliseError nil ptr dereference (#803)

* feat: shard selector (#807)

* LoadBalancer for bitswap (and later, more of libp2p) (#786)

* feat(loadbalancer): add message types

* feat(messages): add utility functions

* feat(loadbalancer): initial load balancer impl

implementation of the load balancer node itself

* feat(loadbalancer): add service node

implements code for running a service node

* feat(loadbalancer): integrate into boost and booster-bitswap

* Update loadbalancer/loadbalancer.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <[email protected]>

* refactor(loadbalancer): remove routing protocol

remove the routing protocol, instead relying on a set config. also remove forwarding response for
inbound requests

* fix(loadbalancer): update tests

* refactor(loadbalancer): integrate simplified load balancer

removed pub keys to minimize network traffic, added api's to configure and update bitswap peer id,
added auto config of bitswap peer id in booster-bitswap

* docs(gen): regenerate api docs

* chore(lint): fix lint errors

* fix(loadbalancer): minor bridgestream fix

* Update loadbalancer/servicenode.go

Co-authored-by: dirkmc <[email protected]>

* refactor(protocolproxy): address PR comments

renames, reconfigured architecture, etc

* refactor(make init print out peer id): remove apis and transparent peer id setting. have init print

Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: dirkmc <[email protected]>

* Add block filter via BadBits (#825)

* feat(booster-bitswap): add block filter via BadBits

* refactor(booster-bitswap): use bitswap blockfilter for filtering

* feat(blockfilter): only update when list is modified

* feat(blockFilter): add on disk caching

* Update cmd/booster-bitswap/blockfilter/blockfilter.go

Co-authored-by: dirkmc <[email protected]>

* fix(blockfilter): minor PR fixups

Co-authored-by: dirkmc <[email protected]>

* Libp2p 0.22 upgrade (#837)

* chore(deps): upgrade to Lotus RC & libp2p v0.22

* chore(deps): update go to 1.18

* ci(circle): update circle to go 1.18

* style(imports): fix imports

* fix(build): update ffi

* fix(lint): fix deprecated strings.Title method

* fix(mod): mod tidy

* Protocol Proxy cleanup (#836)

* refactor(booster-bitswap): minor UI fixes for booster-bitswap UI

* Update cmd/booster-bitswap/init.go

Co-authored-by: dirkmc <[email protected]>

Co-authored-by: dirkmc <[email protected]>

* feat: update to dagstore v0.5.5 (#849)

* feat: bitswap client

* feat: bitswap client - output car file

* refactor: bitswap client - remove tracing

* feat: debug logs

* fix: write blocks to blockstore

* fix: duration output

* fix: duration output for block received

* feat: add pprof to bitswap client

* feat: protocol proxy logging

* feat: bitswap client - check host supports bitswap protocol

* feat: listen for bitswap requests locally as well as through forwarding protocol

Co-authored-by: Hannah Howard <[email protected]>
Co-authored-by: Anton Evangelatov <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
@nonsense nonsense deleted the feat/block-filter branch October 17, 2022 09:57
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.

2 participants