Introduce the Limit on Eagerness

[tek]

!

[facundominguez]

Looks to me like prop_serveAdversarialBranches shouldn't be marked as a expected failure anymore after adding LoE. But please, let me know if this is not the case.

[Niols]

I don't think that is the case. First, for now, the LoE is disabled by default, so we should observe the normal behaviour. Still, it wouldn't be hard to enable it for that test in particular. Second, the property states both that the immutable tip should be honest and that it should not be too old. The LoE alone will not be able to provide that.

Now we could add a way to relax the property into only checking that the immutable tip is honest, and I think such a property should hold with the current LoE? However, in the current state of this PR, it doesn't.

[facundominguez]

Second, the property states both that the immutable tip should be honest and that it should not be too old. The LoE alone will not be able to prevent that.

How is that LoE could prevent the immutable tip to be at least within s+d+1 of the end of the honest chain? In this test, the adversarial peers serve adversarial branches but they do not withhold headers or blocks.

[amesgen]

Second, the property states both that the immutable tip should be honest and that it should not be too old. The LoE alone will not be able to prevent that.

How is that LoE could prevent the immutable tip to be at least within s+d of the end of the honest chain?

Without the GDD logic and the LoP, it is trivial for an attacker to serve some historical alternative, which means that the intersection of the candidate fragments (beyond which the node can't select more than $k$ blocks) is stuck forever in the distant past (no liveness). The LoE just guarantees that the immutable tip is always on "the" honest chain (safety).

[facundominguez]

Ah, I think my confusion is because I was expecting LoE to go together with the GDD governor.

[Niols]

The LoE just guarantees that the immutable tip is always on "the" honest chain (safety).

Still, we could provide a test case for that, maybe by providing a simpler property. Currently, there are cases with LoE enabled where the chain ends up on an alternative branch so it could be nice to catch that with a test.

[amesgen]

Hmm, do we actually want to include this for the milestone? It is convenient for testing as we don't need a separate monitoring thread, but it also shouldn't be too bad. Also, we have LoEUnlimited to disable the LoE directly for the purpose of mergeability into main.

[tek]

So my plan would be to make this the ultimate interface for the GDDG, in a synchronous fashion.

The concern you raised was, iirc, that this would mean that peers wouldn't be disconnected promptly, but only "on demand", i.e. when we want to apply the LoE limit.

Another possible issue one might suspect is that this will not be triggered after refusing to add a block, though I assume that that's not the case – can you confirm that?

At first I thought you wanted this to only be exposed to tests and not prod, but I guess you assumed that this approach was just temporary.

---

Since the limit is used directly by ChainSel while UpdateLoE is a black box, I think it's necessary to have both as parameters.
However, the limit is basically a boolean and we use k for the value unconditionally, so we could theoretically merge them so that UpdateLoE has a constructor that indicates it's disabled, and ChainSel would match on the callback constructor to decide to apply the limit...probably bad style though.

[amesgen]

Another possible issue one might suspect is that this will not be triggered after refusing to add a block, though I assume that that's not the case – can you confirm that?

By "refusing to add a block" you mean the event that we don't end up selecting the block? I think updateLoE is called sufficiently early (ie before checks whether it already is in the VolDB) that it should not be skipped even in that case.

---

Overall, the impact of updateLoEFrag is very minimal, and it is not a required part of the LoE deliverable, so we can leave it for now and decide this later when we implement the GDD.