lp-gateway: Drop session ID invalidation

[cdamian]

Addressing some comments from - #1961.

[lemunozm]

Thanks Cosmin! I like the new direction. A couple of questions

Question 1. Can you elaborate on why we need the InboundEntry as an enum?

Could we not have something like (Option<Message>, SessionId, DomainAddress, count), avoiding to deal with the enum variants?

Question 2. How are we ensuring two proofs are not comming from the same router? Should not that information be stored somehow in the InboundEntry? (maybe a list of RouterId of the read proofs or similar)

[lemunozm]

Could not always the expected_proof_count be infered by knowing the number of routers under domain_address?

[cdamian]

Yes, but if we want to support duplicate messages and ensure they don't get lost, we need to keep track of this and increment it accordingly, as per this.

[cdamian]

Question 1. Can you elaborate on why we need the InboundEntry as an enum?
Could we not have something like (Option, SessionId, DomainAddress, count), avoiding to deal with the enum variants?

The enum made sense to me since it's a lot easier to deal with a struct-like entity like this instead of a tuple. Furthermore, it creates a clear distinction between message and message proof entries. Given that we have certain rules in place for defining which router can send what, such as these, this distinction allows us to perform validation such as this.

[cdamian]

Question 2. How are we ensuring two proofs are not comming from the same router? Should not that information be stored somehow in the InboundEntry? (maybe a list of RouterId of the read proofs or similar)

We don't need to ensure this, if we would, we wouldn't be able to support duplicate messages.

[lemunozm]

struct-like entity like this instead of a tuple

Yeah, when I wrote a tuple I wanted to write mostly a struct.

We don't need to ensure this, if we would, we wouldn't be able to support duplicate messages.

I'm pretty sure we need it... This is where the "vote" concept comes into play. Suppose you have 2 duplicated messages. Then you dispatch the first one to be processed once you have at least one vote from all associated routers. At least this is how it works in Solidity.

[cdamian]

struct-like entity like this instead of a tuple

Yeah, when I wrote a tuple I wanted to write mostly a struct.

We don't need to ensure this, if we would, we wouldn't be able to support duplicate messages.

I'm pretty sure we need it... This is where the "vote" concept comes into play. Suppose you have 2 duplicated messages. Then you dispatch the first one to be processed once you have at least one vote from all associated routers. At least this is how it works in Solidity.

We should expect multiple proofs from the same router. Given a setup with 3 routers x, y, z, where x is supposed to send the message, y and z are supposed to send the proofs, we would expect 1 message + 2 proofs in order to process 1 message. Given this, we can have a scenario where we receive 4 proofs from y and z, 2 each, and then 2 messages from x.

Cases similar to what I described above are covered in the unit tests - here.

[lemunozm]

We should expect multiple proofs from the same router.

Yes, and that is where the vote for that pair (message_hash, router) increments to 2.

In the current implementation, we can receive a message from x, two proofs from y, and the message will be processed without waiting for z, or am I missing something?

[cdamian]

In the current implementation, we can receive a message from x, two proofs from y, and the message will be processed without waiting for z, or am I missing something?

No, we also have a test that covers the case that you mentioned here.

The logic that counts votes can be found here.

[lemunozm]

Ah ok, you're right! I didn't see well that part 👍🏻

So, to double-check. The current implementation can:

• Receive multiple same messages.
• Receive multiple proofs for the same messages.
• Will only process a message when at least one proof for each router is received.

[cdamian]

Ah ok, you're right! I didn't see well that part 👍🏻

So, to double-check. The current implementation can:

• Receive multiple same messages.
• Receive multiple proofs for the same messages.
• Will only process a message when at least one proof for each router is received.

Correct!

[cdamian]

Anyway @lemunozm, this PR is dealing with some other changes not necessarily related to votes etc. Do you mind if I merge this in the main branch and continue discussions there?

[lemunozm]

Yeah, sure! Having understood that, I think everything now is working as it should 👍🏻 Let's merge it

[lemunozm]

Also, I think now InboundEntry makes more sense to be an enum 👍🏻