Hi, I had some questions regarding the use of the NewMessageCallback:
- Are we expected to give it the unprocessed packet as it was received, or should we give it the processed packet (with its modified peerRelayAddress) ?
- What does the "origin" parameter represent ? A peer name ? An address ? From the tests I can deduce that it is the peer name of the original sender, but the information is also found in the packet so it seems redundant.
Thank you in advance.
Cheers!
Hi Dorian,
Thanks for your remarks, they're great points!
1. In general, the callback should receive the unmodified packet. However, for this homework, it does not make a difference, because the GUI is not interested in the relay address. We'll specify the behavior more clearly for the next homeworks.
2. For this homework, the origin is indeed the original sender and it is, indeed, redundant. However, the same callback method signature will be used by the next homeworks, for all kinds of messages encapsulated in a gossip packet, and not all messages will have a redundant origin field. Again, we will make sure to specify what the "origin" field should be.
Cristina
Hi,
I also have some problem about this. In the Gossip_Chain test (packets_test.go), one of the assertion seems to require that the callback is called after we modify the packet RelayPeerAddr.
In that test, we have a chain
(127.0.0.1:2001) -> (127.0.0.1:2002) -> (127.0.0.1:2003) -> (127.0.0.1:2004).
The testcase puts a callback to read the packet at (127.0.0.1:2002). But the assertion said that the packet should have RelayPeerAddr = 127.0.0.1:2002 (Line 72-80 in packets_test.go)
Currently, I deal with it by putting the callback after the SimpleMessage handler. But, maybe this testcase is a typo?
Best,
Reka
Thanks for pointing this out. You're totally right, the test does change the packet before the check and my answer above is wrong: you should call the callback *after* changing the relay peer address.
As I said, for the next assignments, we'll make sure to specify the behavior more completely.
Cristina
I see, thanks for the clarification!
I was wrong with my answer (1). In fact, as someone pointed out, there is a test checking that callback is called after changing the relay address. So please do that.
As I said, the next homeworks will specify when exactly the callback should be called for each message type.
Cristina
I don't think the order of calls matters. In GossipPacket, SimpleMessage is stored as a reference, and the callback function doesn't make a deepcopy of the incoming packet, so when the test checks the copy of the packet stored by the callback function, that SimpleMessage still points to the updated one.
You've ignored the caller in your reasoning, which is why it doesn't always work.
The caller (i.e., the handler) can pass to the callback a deep copy of the gossip message before updating the relay address. Then, the test would fail.
Otherwise, you're right that, if the caller does not make a deep copy, the test would pass.
You're right. I missed out this part. I think each handler should make a copy of the original packet (or at least the field to be touched) before proceeding, and thus the original packet is left unmodified.
Follow the pulses of EPFL on social networks
© 2023 EPFL, all rights reserved