search

vacp2p / nim-libp2p

libp2p implementation in Nim
https://vacp2p.github.io/nim-libp2p/docs/
MIT License
242 stars 52 forks source link

fix(issue-1052): Single topic for RPC Message #1061

Closed AlejandroCabeza closed 3 months ago

AlejandroCabeza commented 3 months ago

Description

Rename RPC's Message topicIds to topicId. Please, do look carefully into this one as I'm not sure how a well covered by tests is the repo, and my changes could be breaking stuff.

Issue

Closes (hopefully): https://github.com/status-im/nim-libp2p/issues/1052

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 82.07547% with 38 lines in your changes are missing coverage. Please review.

Project coverage is 82.52%. Comparing base (ca01ee0) to head (6eec9db). Report is 2 commits behind head on unstable.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061/graphs/tree.svg?width=650&height=150&src=pr&token=UR5JRQ249W&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im)](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im) ```diff @@ Coverage Diff @@ ## unstable #1061 +/- ## ============================================ - Coverage 82.81% 82.52% -0.30% ============================================ Files 91 91 Lines 15750 15820 +70 ============================================ + Hits 13043 13055 +12 - Misses 2707 2765 +58 ``` | [Files](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im) | Coverage Δ | | |---|---|---| | [libp2p/protocols/pubsub/floodsub.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvZmxvb2RzdWIubmlt) | `87.70% <100.00%> (+0.10%)` | :arrow_up: | | [libp2p/protocols/pubsub/gossipsub/behavior.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvZ29zc2lwc3ViL2JlaGF2aW9yLm5pbQ==) | `88.83% <100.00%> (ø)` | | | [libp2p/protocols/pubsub/mcache.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvbWNhY2hlLm5pbQ==) | `92.00% <100.00%> (-0.60%)` | :arrow_down: | | [libp2p/protocols/pubsub/pubsub.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvcHVic3ViLm5pbQ==) | `84.29% <100.00%> (ø)` | | | [libp2p/protocols/pubsub/pubsubpeer.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvcHVic3VicGVlci5uaW0=) | `88.58% <ø> (ø)` | | | [libp2p/protocols/pubsub/rpc/message.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvcnBjL21lc3NhZ2Uubmlt) | `93.33% <100.00%> (ø)` | | | [libp2p/protocols/pubsub/gossipsub/scoring.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvZ29zc2lwc3ViL3Njb3Jpbmcubmlt) | `90.25% <90.90%> (-0.71%)` | :arrow_down: | | [libp2p/protocols/pubsub/rpc/protobuf.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvcnBjL3Byb3RvYnVmLm5pbQ==) | `85.42% <76.19%> (-1.38%)` | :arrow_down: | | [libp2p/protocols/pubsub/rpc/messages.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvcnBjL21lc3NhZ2VzLm5pbQ==) | `54.54% <61.90%> (+1.68%)` | :arrow_up: | | [libp2p/protocols/pubsub/gossipsub.nim](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im#diff-bGlicDJwL3Byb3RvY29scy9wdWJzdWIvZ29zc2lwc3ViLm5pbQ==) | `85.33% <79.82%> (-1.72%)` | :arrow_down: | ... and [6 files with indirect coverage changes](https://app.codecov.io/gh/status-im/nim-libp2p/pull/1061/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=status-im)
arnetheduck commented 3 months ago

one thing missing here is that the code must check that there is (at least) one topic when decoding the message from bytes

AlejandroCabeza commented 3 months ago

one thing missing here is that the code must check that there is (at least) one topic when decoding the message from bytes

What's the expected behaviour if there isn't?

arnetheduck commented 3 months ago

What's the expected behaviour if there isn't?

penalize the peer for sending an invalid message (same as if signature was bad or it wasn't protobuf at all)

codecov-commenter commented 3 months ago

Welcome to Codecov :tada:

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered :open_umbrella: