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.

Sign up for GitHub

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

[Remove] obsolete WsServer #3582

Merged
merged 3 commits into from
Nov 18, 2024
+5 −31
Merged

[Remove] obsolete WsServer #3582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5e617ba
Remove WSServer obsolete
cschuchardt88 Nov 16, 2024
f205c66
Revert `global.json`
cschuchardt88 Nov 16, 2024
10301ce
Update ServerCapability.cs
shargon Nov 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/Neo/Network/P2P/Capabilities/NodeCapability.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ public static NodeCapability DeserializeFrom(ref MemoryReader reader)
NodeCapabilityType type = (NodeCapabilityType)reader.ReadByte();
NodeCapability capability = type switch
{
#pragma warning disable CS0612 // Type or member is obsolete
NodeCapabilityType.TcpServer or NodeCapabilityType.WsServer => new ServerCapability(type),
#pragma warning restore CS0612 // Type or member is obsolete
NodeCapabilityType.TcpServer => new ServerCapability(type),
NodeCapabilityType.FullNode => new FullNodeCapability(),
_ => throw new FormatException(),
};
Expand Down
8 changes: 0 additions & 8 deletions src/Neo/Network/P2P/Capabilities/NodeCapabilityType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using System;

namespace Neo.Network.P2P.Capabilities
{
/// <summary>
Expand All @@ -25,12 +23,6 @@ public enum NodeCapabilityType : byte
/// </summary>
TcpServer = 0x01,

/// <summary>
/// Indicates that the node is listening on a WebSocket port.
/// </summary>
[Obsolete]
WsServer = 0x02,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's correct to remove it like this. Technically it existed in the P2P protocol, so at least the number should be kept as reserved.

Copy link
Member

Choose a reason for hiding this comment

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

ObsoleteWsServer

Copy link
Member Author

Choose a reason for hiding this comment

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

we can always repurpose it in the future. Reserving it says to me that it still WsServer or active.

Copy link
Contributor

@Jim8y Jim8y Nov 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I do not agree, either we should implement it or w should remove it, having somthing in the core that is not implemented (and is not documented) is not a good practice, might exist in the past by mistake, but should be fixed, there is no hardfork for this, should not bother.

I have to say that there are too many things in the neo core that is not documented, not commented, not explained, not in proposal, the only way we learn it is by reading the code, but this wss is even a trap of implementation, it exists, but not work, as chris said "Reserving it says to me that it still WsServer or active.". only very few of us know how they works, this is by no means a good practice.


#endregion

#region Others
Expand Down
6 changes: 2 additions & 4 deletions src/Neo/Network/P2P/Capabilities/ServerCapability.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,11 @@ public class ServerCapability : NodeCapability
/// <summary>
/// Initializes a new instance of the <see cref="ServerCapability"/> class.
/// </summary>
/// <param name="type">The type of the <see cref="ServerCapability"/>. It must be <see cref="NodeCapabilityType.TcpServer"/> or <see cref="NodeCapabilityType.WsServer"/></param>
/// <param name="type">The type of the <see cref="ServerCapability"/>. It must be <see cref="NodeCapabilityType.TcpServer"/></param>
/// <param name="port">The port that the node is listening on.</param>
public ServerCapability(NodeCapabilityType type, ushort port = 0) : base(type)
{
#pragma warning disable CS0612 // Type or member is obsolete
if (type != NodeCapabilityType.TcpServer && type != NodeCapabilityType.WsServer)
#pragma warning restore CS0612 // Type or member is obsolete
if (type != NodeCapabilityType.TcpServer)
{
throw new ArgumentException(nameof(type));
}
Expand Down
18 changes: 2 additions & 16 deletions tests/Neo.UnitTests/Network/P2P/Capabilities/UT_ServerCapability.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,12 @@ public void Size_Get()
{
var test = new ServerCapability(NodeCapabilityType.TcpServer) { Port = 1 };
test.Size.Should().Be(3);

#pragma warning disable CS0612 // Type or member is obsolete
test = new ServerCapability(NodeCapabilityType.WsServer) { Port = 2 };
#pragma warning restore CS0612 // Type or member is obsolete
test.Size.Should().Be(3);
}

[TestMethod]
public void DeserializeAndSerialize()
{
#pragma warning disable CS0612 // Type or member is obsolete
var test = new ServerCapability(NodeCapabilityType.WsServer) { Port = 2 };
var test = new ServerCapability(NodeCapabilityType.TcpServer) { Port = 2 };
var buffer = test.ToArray();

var br = new MemoryReader(buffer);
Expand All @@ -45,21 +39,13 @@ public void DeserializeAndSerialize()
Assert.AreEqual(test.Port, clone.Port);
Assert.AreEqual(test.Type, clone.Type);

clone = new ServerCapability(NodeCapabilityType.WsServer, 123);
#pragma warning restore CS0612 // Type or member is obsolete
clone = new ServerCapability(NodeCapabilityType.TcpServer, 123);
br = new MemoryReader(buffer);
((ISerializable)clone).Deserialize(ref br);

Assert.AreEqual(test.Port, clone.Port);
Assert.AreEqual(test.Type, clone.Type);

clone = new ServerCapability(NodeCapabilityType.TcpServer, 123);

Assert.ThrowsException<FormatException>(() =>
{
var br2 = new MemoryReader(buffer);
((ISerializable)clone).Deserialize(ref br2);
});
Assert.ThrowsException<ArgumentException>(() =>
{
_ = new ServerCapability(NodeCapabilityType.FullNode);
Expand Down
Loading