Skip to content

Add block filter via BadBits#825

Merged
hannahhoward merged 6 commits intorelease/lotus1.17.2from
feat/block-filter
Sep 26, 2022
Merged

Add block filter via BadBits#825
hannahhoward merged 6 commits intorelease/lotus1.17.2from
feat/block-filter

Conversation

@hannahhoward
Copy link
Copy Markdown
Contributor

@hannahhoward hannahhoward commented Sep 21, 2022 •
edited
Loading

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
Copy Markdown
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
Copy Markdown
Contributor Author

hannahhoward commented Sep 23, 2022

@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

dirkmc
dirkmc approved these changes Sep 23, 2022
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

hannahhoward commented Sep 26, 2022

@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
@nonsense @hannahhoward
@dirkmc @rvagg
release/lotus1.17.2 to main (#794)
6e0ac5c 
* 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 <dirkmdev@gmail.com>
Co-authored-by: Anton Evangelatov <anton.evangelatov@gmail.com>

* 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 <rod@vagg.org>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <rod@vagg.org>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <rod@vagg.org>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <rod@vagg.org>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <rod@vagg.org>

* 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 <dirkmdev@gmail.com>

* 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 <rod@vagg.org>
Co-authored-by: dirkmc <dirkmdev@gmail.com>

* 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 <dirkmdev@gmail.com>

* fix(blockfilter): minor PR fixups

Co-authored-by: dirkmc <dirkmdev@gmail.com>

* 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 <dirkmdev@gmail.com>

Co-authored-by: dirkmc <dirkmdev@gmail.com>

* 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 <hannah@hannahhoward.net>
Co-authored-by: Dirk McCormick <dirkmdev@gmail.com>
Co-authored-by: Rod Vagg <rod@vagg.org>
dirkmc added a commit that referenced this pull request Oct 4, 2022
@dirkmc @hannahhoward
@nonsense @rvagg
bitswap client (#856)
f0726d2 
* 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 <dirkmdev@gmail.com>
Co-authored-by: Anton Evangelatov <anton.evangelatov@gmail.com>

* 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 <rod@vagg.org>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <rod@vagg.org>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <rod@vagg.org>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <rod@vagg.org>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <rod@vagg.org>

* 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 <dirkmdev@gmail.com>

* 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 <rod@vagg.org>
Co-authored-by: dirkmc <dirkmdev@gmail.com>

* 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 <dirkmdev@gmail.com>

* fix(blockfilter): minor PR fixups

Co-authored-by: dirkmc <dirkmdev@gmail.com>

* 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 <dirkmdev@gmail.com>

Co-authored-by: dirkmc <dirkmdev@gmail.com>

* 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 <hannah@hannahhoward.net>
Co-authored-by: Anton Evangelatov <anton.evangelatov@gmail.com>
Co-authored-by: Rod Vagg <rod@vagg.org>
@nonsense nonsense deleted the feat/block-filter branch October 17, 2022 09:57
@jacobheun jacobheun mentioned this pull request Nov 25, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rvagg rvagg Awaiting requested review from rvagg

1 more reviewer

@dirkmc dirkmc dirkmc approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hannahhoward @dirkmc