{"action":"created","issue":{"url":"https://api.github.com/repos/ExchangeUnion/xud/issues/210","repository_url":"https://api.github.com/repos/ExchangeUnion/xud","labels_url":"https://api.github.com/repos/ExchangeUnion/xud/issues/210/labels{/name}","comments_url":"https://api.github.com/repos/ExchangeUnion/xud/issues/210/comments","events_url":"https://api.github.com/repos/ExchangeUnion/xud/issues/210/events","html_url":"https://github.com/ExchangeUnion/xud/pull/210","id":343749243,"node_id":"MDExOlB1bGxSZXF1ZXN0MjAzMzA3OTQ2","number":210,"title":"Allow empty packets","user":{"login":"sangaman","id":3440835,"node_id":"MDQ6VXNlcjM0NDA4MzU=","avatar_url":"https://avatars1.githubusercontent.com/u/3440835?v=4","gravatar_id":"","url":"https://api.github.com/users/sangaman","html_url":"https://github.com/sangaman","followers_url":"https://api.github.com/users/sangaman/followers","following_url":"https://api.github.com/users/sangaman/following{/other_user}","gists_url":"https://api.github.com/users/sangaman/gists{/gist_id}","starred_url":"https://api.github.com/users/sangaman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sangaman/subscriptions","organizations_url":"https://api.github.com/users/sangaman/orgs","repos_url":"https://api.github.com/users/sangaman/repos","events_url":"https://api.github.com/users/sangaman/events{/privacy}","received_events_url":"https://api.github.com/users/sangaman/received_events","type":"User","site_admin":false},"labels":[{"id":930952990,"node_id":"MDU6TGFiZWw5MzA5NTI5OTA=","url":"https://api.github.com/repos/ExchangeUnion/xud/labels/in%20progress","name":"in progress","color":"ededed","default":false}],"state":"open","locked":false,"assignee":{"login":"sangaman","id":3440835,"node_id":"MDQ6VXNlcjM0NDA4MzU=","avatar_url":"https://avatars1.githubusercontent.com/u/3440835?v=4","gravatar_id":"","url":"https://api.github.com/users/sangaman","html_url":"https://github.com/sangaman","followers_url":"https://api.github.com/users/sangaman/followers","following_url":"https://api.github.com/users/sangaman/following{/other_user}","gists_url":"https://api.github.com/users/sangaman/gists{/gist_id}","starred_url":"https://api.github.com/users/sangaman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sangaman/subscriptions","organizations_url":"https://api.github.com/users/sangaman/orgs","repos_url":"https://api.github.com/users/sangaman/repos","events_url":"https://api.github.com/users/sangaman/events{/privacy}","received_events_url":"https://api.github.com/users/sangaman/received_events","type":"User","site_admin":false},"assignees":[{"login":"sangaman","id":3440835,"node_id":"MDQ6VXNlcjM0NDA4MzU=","avatar_url":"https://avatars1.githubusercontent.com/u/3440835?v=4","gravatar_id":"","url":"https://api.github.com/users/sangaman","html_url":"https://github.com/sangaman","followers_url":"https://api.github.com/users/sangaman/followers","following_url":"https://api.github.com/users/sangaman/following{/other_user}","gists_url":"https://api.github.com/users/sangaman/gists{/gist_id}","starred_url":"https://api.github.com/users/sangaman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sangaman/subscriptions","organizations_url":"https://api.github.com/users/sangaman/orgs","repos_url":"https://api.github.com/users/sangaman/repos","events_url":"https://api.github.com/users/sangaman/events{/privacy}","received_events_url":"https://api.github.com/users/sangaman/received_events","type":"User","site_admin":false}],"milestone":null,"comments":1,"created_at":"2018-07-23T18:42:36Z","updated_at":"2018-07-25T22:12:17Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/ExchangeUnion/xud/pulls/210","html_url":"https://github.com/ExchangeUnion/xud/pull/210","diff_url":"https://github.com/ExchangeUnion/xud/pull/210.diff","patch_url":"https://github.com/ExchangeUnion/xud/pull/210.patch"},"body":"Closes #208.\r\n\r\nThis commit creates a `GENERIC_REQUEST` packet direction which does not use a body and instead specifies a response packet type. Sending a generic request packet solicits any packet of the corresponding response packet type. This also refactors the `GET_HOSTS` and `GET_ORDERS` packets as generic requests.\r\n\r\nAn approach like this was discussed in #204. I'm open to feedback but I thought that it would be good to have a working example for further discussion. I've tested successfully between two separate instances running on my machine."},"comment":{"url":"https://api.github.com/repos/ExchangeUnion/xud/issues/comments/407913390","html_url":"https://github.com/ExchangeUnion/xud/pull/210#issuecomment-407913390","issue_url":"https://api.github.com/repos/ExchangeUnion/xud/issues/210","id":407913390,"node_id":"MDEyOklzc3VlQ29tbWVudDQwNzkxMzM5MA==","user":{"login":"sangaman","id":3440835,"node_id":"MDQ6VXNlcjM0NDA4MzU=","avatar_url":"https://avatars1.githubusercontent.com/u/3440835?v=4","gravatar_id":"","url":"https://api.github.com/users/sangaman","html_url":"https://github.com/sangaman","followers_url":"https://api.github.com/users/sangaman/followers","following_url":"https://api.github.com/users/sangaman/following{/other_user}","gists_url":"https://api.github.com/users/sangaman/gists{/gist_id}","starred_url":"https://api.github.com/users/sangaman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sangaman/subscriptions","organizations_url":"https://api.github.com/users/sangaman/orgs","repos_url":"https://api.github.com/users/sangaman/repos","events_url":"https://api.github.com/users/sangaman/events{/privacy}","received_events_url":"https://api.github.com/users/sangaman/received_events","type":"User","site_admin":false},"created_at":"2018-07-25T22:12:17Z","updated_at":"2018-07-25T22:12:17Z","author_association":"COLLABORATOR","body":"> I think i'd rather keep the linkage.\r\n\r\nYou mean the linkage to an identifier?\r\n\r\n> Also, requests might have optional parameters, but we're defining PacketDirection in a static manner (it's not being serialized). This might create an issue.\r\n\r\nI don't think I follow. If all peers are following the same protocol with the same `PacketDirection`s and `PacketType`s, shouldn't it be fine?\r\n\r\n> I think that every packet should have a unique identifier (from the sender perspective). Relying merely on hashing won't work, because the packet might lack uniqueness (not only because the body is empty). We can introduce uniqueness to the body, but I think that supporting packets without a body is a good idea. If we'll add the uniqueness to the header, the hashing would not be straight-forward.\r\n> A solution can be to separate between the id and the checksum hash. We can add a uuid as an id field to the header, and if the packet has a body, add hash as well. the response linkage would be by the id.\r\n\r\nAn identifier might be useful I'm thinking as well - and you're right about the hash not always being sufficient (particularly for bodyless packets). But I think a uuid might be overkill actually. How about tracking a packet count for each peer, using that as the id for the next packet and incrementing each time we send a packet? Nodes responding to a request packet will include its id in the response packet. The Hello packet can always be id 0 and we always expect a packet of id 0. That guarantees packet uniqueness for a particular socket and we don't care about global uniqueness.  A running count should need fewer cycles and less data transmission than a uuid.\r\n\r\nI think that might make sense as part of this PR actually, as I'd have to rework some of the code.\r\n\r\nAs a minor bonus, this might also be a useful statistic to have about a socket connection, total packets sent.\r\n\r\n> Another change I wanted to do is to include the packet type inside of the header. currently it's being serialized separately. It would be better if each serialized packet would be one complete JSON, so that the header would need to be deserialized first in order to determine the packet type.\r\n\r\nI like this idea. I can implement this in a separate PR unless you want to work on it. I can also open an issue for it if you'd like."}}
{"actor":{"display_login":"sangaman"}}