Re-add deku needs #56
Conversation
Gau-thier
force-pushed
the
feat/dekuIntegrationFinalAct
branch
from
88bbdd9 to
28aaf63
Compare
rosalogia
left a comment
rosalogia
left a comment
There was a problem hiding this comment.
I think payload_signature should be an optional arg, otherwise looks good 👍
lib/node/server.ml
Outdated
| disseminated across the network. | ||
| 4. Wait 0.001 seconds before restarting the procedure. *) | ||
| let rec run node preprocessor msg_handler = | ||
| let rec run node preprocessor (msg_handler: Message.t -> bytes option * bytes option ) = |
There was a problem hiding this comment.
Shouldn't it be (bytes * bytes option) option? Since you can't have a payload_signature without a payload?
There was a problem hiding this comment.
Making msg_handler: Message.t -> bytes * bytes option will force the client to send a response to every received message, which is not wanted.
@AngryStitch am I right?
There was a problem hiding this comment.
Shouldn't it be msg_handler: Message.t -> (bytes * bytes) option because if you have a payload, you have a signature but sometimes you don't do anything with a message (like, it's an ACK)?
There was a problem hiding this comment.
On a Deku POV, "if you have a payload you have a message" but not on a Pollinate side.
Failure_detector is sending payload message, without signature.
Maybe we should propose to add a function to sign them, to make sure the node sending is the real node, or is it overkilled? @rosalogia, what is your opinion?
…kuIntegrationFinalAct
Gau-thier
force-pushed
the
feat/dekuIntegrationFinalAct
branch
from
90c2c93 to
7068755
Compare
Gau-thier
force-pushed
the
feat/dekuIntegrationFinalAct
branch
from
1476ea2 to
08a6418
Compare
Gau-thier
force-pushed
the
feat/dekuIntegrationFinalAct
branch
from
af88cc1 to
5c2f49e
Compare
Gau-thier
force-pushed
the
feat/dekuIntegrationFinalAct
branch
from
1872f7b to
0c34e8d
Compare
lib/pnode/server.ml
Outdated
| | Some response -> | ||
| response | ||
| |> Client.create_response node message | ||
| | Some msg -> |
There was a problem hiding this comment.
We can destructure here (just a matter of syntax); up to you
| Some Message.{ data ; signature = payload_signature }
Client.create_response node message ?payload_signature dataThere was a problem hiding this comment.
Nice one, totally missed it. Done.
| |> String.to_bytes | ||
| |> Client.create_post node in | ||
| |> String.to_bytes in | ||
| let message = Client.create_post node payload in |
There was a problem hiding this comment.
I will always advocate for long pipelines, but ultimately doesn't matter 😄
There was a problem hiding this comment.
I am still not fond of long pipelines, I will let as it is right now 😉
Gau-thier
force-pushed
the
feat/dekuIntegrationFinalAct
branch
from
c4e8c63 to
17848cf
Compare
Gau-thier
force-pushed
the
feat/dekuIntegrationFinalAct
branch
from
eacbce7 to
e3e337d
Compare
|
Forgot to bring this up in review: what is the purpose of sub-category? Why is it not just encoded in the payload? If we want a general purpose way to allow users to include their own metadata/header data in messages, we can do that, but this feels oddly specific for no particular reason. It is always possible for the consumer to design their payloads to include any kind of labels necessary on their end. If we want to do pollinate-side metadata for consumer use, I'd say add a field called "headers" to Message.t whose type is (string * string) list. Thoughts @gsebil08? |
That is a great remark! |
No description provided.