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

Policy filter GetRelayResult message #543

Merged
merged 4 commits into from
Jan 15, 2019
Merged

Policy filter GetRelayResult message #543

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
70fb3e1
Policy filter GetRelayResult message
vncoelho Jan 12, 2019
0c4ff29
adding fixed numbering for return codes
igormcoelho Jan 12, 2019
27c573e
Merge branch 'master' into policy-filter-GetRelayResult
shargon Jan 13, 2019
1e92cbe
Removed enum fixed values
igormcoelho Jan 15, 2019
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
14 changes: 7 additions & 7 deletions neo/Ledger/RelayResultReason.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
{
public enum RelayResultReason : byte
{
Succeed,
AlreadyExists,
OutOfMemory,
UnableToVerify,
Invalid,
PolicyFail,
Unknown
Succeed = 0x00,
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need explicit enumeration values?

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps the idea was to avoid changing the value of Unknown again, since Unknown was changed to PolicyFail. Maybe only needs to be explicit for unknown.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think explicit values make it easier for documentation purposes, otherwise the developer will need to count one by one just to know the expected value xD Regarding Unknown, it's good to have it fixed, even with future changes. If you prefer other numbers, we can change too.

Copy link
Member

Choose a reason for hiding this comment

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

For documentation, we can just use the enumeration names, such as Succeed, AlreadyExists, etc.
The enumeration values are not used anywhere. Even RpcServer is using enumeration names.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll rethink this, because I'm probably very wrong then :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, now I got your point Erik, I'm actually wrong. But still, perhaps a better idea is to automate the Rpc result numbers using -500 + enum value. So, as soon as we add more, it will get automatically documented. Do you think that's reasonable? Otherwise, we can just remove my commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds reasonable, brother.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I took for granted that they were not actually exposed outside anywhere. Nice it wasn’t needed.

Copy link
Contributor

@igormcoelho igormcoelho Jan 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I tried to integrate with actual return values (-500 - enum_Value), but it was a bad idea... too complex because of the subtraction, it would get more complicated, what is not good. Sorry for the misunderstanding.

AlreadyExists = 0x01,
OutOfMemory = 0x02,
UnableToVerify = 0x03,
Invalid = 0x04,
PolicyFail = 0x05,
Unknown = 0xff
}
}
6 changes: 4 additions & 2 deletions neo/Network/RPC/RpcServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.IO;
using System.IO.Compression;
Expand Down Expand Up @@ -127,8 +127,10 @@ private static JObject GetRelayResult(RelayResultReason reason)
throw new RpcException(-503, "The block cannot be validated.");
case RelayResultReason.Invalid:
throw new RpcException(-504, "Block or transaction validation failed.");
case RelayResultReason.PolicyFail:
throw new RpcException(-505, "One of the Policy filters failed.");
default:
throw new RpcException(-500, "Unkown error.");
throw new RpcException(-500, "Unknown error.");
}
}

Expand Down