Homework 0 forum

Clarifications NewMessageCallback

 
Picture of Dorian Ros
Clarifications NewMessageCallback
by Dorian Ros - Wednesday, 23 September 2020, 13:12
 

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!

Picture of Cristina Basescu
Re: Clarifications NewMessageCallback
by Cristina Basescu - Thursday, 24 September 2020, 09:29
 

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

Picture of Reka Inovan
Re: Clarifications NewMessageCallback
by Reka Inovan - Thursday, 24 September 2020, 10:25
 

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


Picture of Cristina Basescu
Re: Clarifications NewMessageCallback
by Cristina Basescu - Thursday, 24 September 2020, 10:45
 

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

Picture of Reka Inovan
Re: Clarifications NewMessageCallback
by Reka Inovan - Thursday, 24 September 2020, 13:33
 

I see, thanks for the clarification!

Picture of Cristina Basescu
Re: Clarifications NewMessageCallback
by Cristina Basescu - Thursday, 24 September 2020, 10:47
 

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


Picture of Dorian Ros
Re: Clarifications NewMessageCallback
by Dorian Ros - Thursday, 24 September 2020, 10:54
 
All clear. Thank you!
Picture of Huan-Cheng Chang
Re: Clarifications NewMessageCallback
by Huan-Cheng Chang - Thursday, 24 September 2020, 12:40
 

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.

Picture of Cristina Basescu
Re: Clarifications NewMessageCallback
by Cristina Basescu - Thursday, 24 September 2020, 13:00
 

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. 

Picture of Dorian Ros
Re: Clarifications NewMessageCallback
by Dorian Ros - Thursday, 24 September 2020, 13:09
 
Should we do a deep copy and give the callback the original message then ?
Picture of Huan-Cheng Chang
Re: Clarifications NewMessageCallback
by Huan-Cheng Chang - Thursday, 24 September 2020, 13:21
 

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.

Picture of Samuel Jean Michel Jacquier
Re: Clarifications NewMessageCallback
by Samuel Jean Michel Jacquier - Friday, 25 September 2020, 12:27
 
I have questions too.

If I understand correctly, the callback is a function we will call for every message we get from the peers, whatever the message will be.
The NewMessageCallback function need an origin string, which is the original sender of the message, and the gossipPacket which is the packet to send. This function is registerd via the "RegisterCallback" function, which, I guess, should be called only one time during the construction of our gossiper.

But in the subject, we learn that :

If it comes from another peer A, the gossiper (1) stores A’s address from the “relay peer” field in its list of known peers, (2) changes the “relay peer” field to its own address , and (3) sends the message to all known peers besides peer A, leaving the “origin peer name” field unchanged

But as we established in the answers to this question, the gossipPacket should have its RelayPeer address changed before the call with our own address to pass some tests. It means that the original RelayPeerAddress will not be in the gossipPacket... So the callback function will not be able to recover it.

In this case, how would the callback have the address of A ? It is not in the arguments, but we need it to send the packets correctly.

So, am I understanding it wrong here ?