Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce Version property to messages #949

Merged
merged 11 commits into from
Aug 14, 2020

Conversation

limebell
Copy link
Member

Removed Peer.AppProtocolVersion which were used to check version between swarms. Instead, added AppProtocolVersion-typed Version property to Message and use it to check validity of messages.

If received message has different app protocol version, NetMQTransport will reply a DifferentVersion message regardless of received message's type and does not process anything.

@codecov
Copy link

codecov bot commented Aug 10, 2020

Codecov Report

Merging #949 into main will decrease coverage by 0.03%.
The diff coverage is 92.34%.

@@            Coverage Diff             @@
##             main     #949      +/-   ##
==========================================
- Coverage   88.22%   88.18%   -0.04%     
==========================================
  Files         280      282       +2     
  Lines       25458    25524      +66     
==========================================
+ Hits        22460    22509      +49     
- Misses       1532     1539       +7     
- Partials     1466     1476      +10     
Impacted Files Coverage Δ
...ibplanet.Tests/Net/SwarmTest.AppProtocolVersion.cs 94.96% <ø> (-0.25%) ⬇️
Libplanet/Net/Messages/DifferentVersion.cs 66.66% <66.66%> (ø)
Libplanet.Tests/Net/Messages/MessageTest.cs 72.72% <72.72%> (ø)
Libplanet.Tests/Net/Messages/BlockHashesTest.cs 93.54% <100.00%> (+0.21%) ⬆️
Libplanet.Tests/Net/Messages/BlockStatesTest.cs 100.00% <100.00%> (ø)
Libplanet.Tests/Net/Messages/RecentStatesTest.cs 89.70% <100.00%> (ø)
Libplanet.Tests/Net/PeerTest.cs 100.00% <100.00%> (ø)
Libplanet.Tests/Net/Protocols/RoutingTableTest.cs 94.48% <100.00%> (-2.49%) ⬇️
Libplanet.Tests/Net/Protocols/TestTransport.cs 77.23% <100.00%> (-0.07%) ⬇️
Libplanet.Tests/Net/SwarmTest.cs 94.86% <100.00%> (+0.03%) ⬆️
... and 13 more

riemannulus
riemannulus previously approved these changes Aug 10, 2020
@riemannulus
Copy link
Member

/rebase

Libplanet/Net/Peer.cs Show resolved Hide resolved
Libplanet/Net/Peer.cs Show resolved Hide resolved
Libplanet/Net/Peer.cs Show resolved Hide resolved
Libplanet/Net/NetMQTransport.cs Outdated Show resolved Hide resolved
Libplanet/Net/BoundPeer.cs Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
Libplanet.Tests/Net/Messages/BlockHashesTest.cs Outdated Show resolved Hide resolved
longfin
longfin previously approved these changes Aug 10, 2020
earlbread
earlbread previously approved these changes Aug 10, 2020
@limebell limebell marked this pull request as draft August 11, 2020 05:07
@limebell limebell dismissed stale reviews from earlbread and longfin via ceb703e August 12, 2020 07:14
@limebell limebell marked this pull request as ready for review August 12, 2020 07:21
@longfin longfin self-requested a review August 13, 2020 01:56
@limebell limebell added the network Related to networking (Libplanet.Net) label Aug 14, 2020
earlbread
earlbread previously approved these changes Aug 14, 2020
longfin
longfin previously approved these changes Aug 14, 2020
dahlia
dahlia previously approved these changes Aug 14, 2020
@longfin longfin dismissed stale reviews from dahlia, earlbread, and themself via 25d36f0 August 14, 2020 09:38
@longfin longfin merged commit b4f9f7c into planetarium:main Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
network Related to networking (Libplanet.Net)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants