OffchainLabs / decentralized-timeboost-spec

6 stars 0 forks source link

Comments on new incusion.md file relating to inbox indices #26

Closed victorshoup closed 4 months ago

victorshoup commented 4 months ago

The requirement section says a block in round R should contain I{R,first} and I{R,next}, which define an interval [I{R,first},I{R,next}).

In the reference impl, one bullet point says that a block contains two things: a consensus delayed inbox index (computed as the max of two things...note that these two things are in bullet points that should be indented a level further). The next bullet point says that there should also be a possibly empty sequence defining an interval.

First, this interval is inconsistent with the requirements (open on left, rather than open on right). I'm not sure which is correct. Second, it's not clear to me why we need both this interval and the consensus index. Third, the requirements section only mentions the interval, and it is not optional (and the ref impl does not say when the interval should or should not be included). It does not mention a consensus delayed inbox index.

Suggestion: if we stick with the explict round start regime, let's just use exactly the same syntax and semantics for indices as we do for timestamps (essentially treating these indices as just another "clock"). Namely, just define index(m,R) to the the largest index seen by m at the time it starts round R, and then let I_R denote the consensus index, and then require that max(I_P,index(m,R)) <= I_R <= max(I_P,index(m',R)) for some honest members m, m'.

I think this will do what we want. It also allows us to decouple the semantics of index(m,R) from the consensus protocol -- yes, it has a meaning outside of consensus, but the consensus protocol does not need to know what it means at all (it only needs to assume that it is a non-decreasing integer variable that may be inspected at various points in time).

If I'm missing something, let me know. I'm not at all sure why we need this interval business for indices but not for timestamps. If we do want the inclusion list to contain the consensus index for the current round and the previous round, we can certainly do that, but then let's just say that is what we are doing.

edfelten commented 4 months ago

This makes sense. I would amend it slightly to say that index(m,R) is the latest index seen as finalized on L1 by the time m starts round R.

victorshoup commented 4 months ago

Ok. I'll update the branch