All Core Devs - Testing #97 (CL Breakout)

Transcript

Justin Traglia: Hello, hello, just another minute or two. Hello, everyone. Maybe 15, 20 more seconds, then we'll get started. Alright. So it's not on the agenda, but I think we should very quickly discuss, the issue that Michael Sproul raised yesterday regarding, Possible edge case at the CLOS fork transition. With Validator 0 indices. Have you guys seen this issue?

Happy. Yeah, she describes it pretty well, but There could be a split, a consensus split, if Validator is 0. Voted in the first epoch. After the fork. Because technically, it's part of the PTC. So anyway, there was a… there was a suggestion to just initialize the PTC with, Instead of using ValidatorIndex 0, you just use a higher value, which isn't active, it's assigned. That's probably the fix I would do. It was suggested by Kyle. Would that be an issue for anyone? I'm planning to just use the value he suggested, which is 2 to the 22, which is approximately 4 million.

Potuz: Why is it a consensus split? Because… Because people would treat them as invalid, some clients would treat them as invalid.

Justin Traglia: I don't recall. Let me check the issue.

Potuz: Howdy. that the Lighthouse treats them as invalid, which is… Yep, I don't…

Justin Traglia: So yeah, I mean, this is something we should probably fix. I know it's just, like, a stupid edge case, but… What do you guys think about the potential solutions? Also, I can just propose a PR after this call. And share it with you guys. Just be aware of that, and I'll… I'll… Contact you guys more about it later. Mehdi, there… the first agenda item is for you. You proposed two PRs related to, gossip tests. Would you like to discuss those?

Mehdi Aouadi: Yep, I just needed to bring some attention to those PARs, because there were basically, two failing tests in TECU. First one is for the gossip proposal preferences, and the other one was the BNS2 execution change. yeah, I managed to merge the first PR, I got some reviews this morning, and the other ones too, I got some reviews from Jihoon, so I'm working on it. So guess, I guess, yeah, that's all I needed to… to have. Thanks.

Justin Traglia: Okay. Thank you. And I believe, yeah, Jihoon suggested swapping those two checks. I think that also makes sense.

Mehdi Aouadi: Yeah, definitely. Yeah, I already pushed a commit with, swapping, I'm, swapping those changes, those, checks, and I'm, checking the second, review, yeah.

Justin Traglia: Okay, cool. Yeah, I'll take a look at that later, too.

Mehdi Aouadi: Cool, thanks.

Jihoon Song: But there is another test that swapping to checks didn't really solve it's about… what's it called? The Cloud Disparity check? It's a bit weird edge case that we're testing and swapping to You know, two checks doesn't really actually solve the problem, so… I found it a bit weird to use Capella scene when we are still at Bellatrix, but at the same time, I'm also not sure that whether this is actually worth to test, because we're already past Capella.

Mehdi Aouadi: Yeah,

Justin Traglia: no strong feelings for me. It doesn't really matter, this is just a great edge case. Not weird, but annoying, edge case.

Jihoon Song: Yeah, so if it's not a big of a deal, maybe we can swap those two checks and remove the clock disparity check, but… I don't know, what do other people think about this?

Mehdi Aouadi: Yeah, honestly, that was one of the options I was thinking of, but yeah, I didn't know if… Would be okay, for… to reduce the coverage, even though it's, yeah, it's a weird edge case, but… I'd say yes for that.

Justin Traglia: I'll think about it and leave your comments on the PR.

Mehdi Aouadi: Okay, cool.

Justin Traglia: Okay, moving on, Jihoon, you wanted to discuss merging that PR. Only return viable payload status variants from GitHead.

Jihoon Song: Yeah, yeah, thanks. James. I want to ask, what do you want to do with 55… 9? What the PR does is it updates GetHead to traverse only nodes that filter 3 returns. Several people, including myself, reviewed the PR, So, assuming the PR is good, the last thing we need to discuss is March timing, because we are close to Sepolia. I saw a few people discussing the GitHub issue, but I figure… It doesn't hurt to bring this to ACD and socialize it with more people.

Justin Traglia: Is this a consensus-breaking change? Like, could we merge it, and then clients wouldn't actually have to do anything?

Potuz: Well, it depends. There might be a weird edge case that can trigger a consensus split, but if that happens, it already exists, because FRISM already implements that logic. Having said so, Michael. Michael Sproul said that there would be implementation churn. So I don't know what that means. Perhaps Lighthouse needs to make changes. And he suggests not merging it before the… the fork. at any rate, if there is a consensus split, it already happens. Prism already implements this. Before this came in, so… So, which also leads me to check that there's no test for this. Having said so on, so it's impossible to trigger. Oops. Just not gonna happen.

Nico Flaig: I think there are tests for this. The compliance tests cover this.

Potuz: Yeah, so we are… we are not passing compliant tests. We only have four-choice tests, but not compliant tests.

Jihoon Song: Yeah, so… Regarding this issue, we can stick to the PR or the spec without the PR, but I think we should decide which one we want to stick to. And that's also… What, because…

Potuz: I'm not gonna…

Jihoon Song: Testing.

Potuz: and fork choice, so regardless.

Jihoon Song: I mean, yes.

Potuz: merge it or not.

Jihoon Song: Oh my goodness. can remain specimen compliant, but we have other clients, so yeah, what other clients do want to… do with SPR. It seems like Lighthouse is okay with going with the PR.

Potuz: Well, Michael says no. Michael explicitly says not to merge it.

Jihoon Song: In the issue, he said, He's okay with it. Am I wrong?

Justin Traglia: He did approve it.

Jihoon Song: Yeah.

Justin Traglia: I mean, he is concerned with, implementation churn, but… Yeah, that's his last comment.

Jihoon Song: Yeah, but in the issue, He's sad. There's… This is a link to the issue. So, assuming Lighthouse, we're gonna implement this, we're gonna have Lighthouse, Prism, and Lone Star implemented this PR. Sounds like. We're gonna go merge it.

Justin Traglia: I don't have any stronger opinions on this. I think I'm fine with merging it as well, but if… If clients want to save this until after. The fork. That's also perfectly acceptable to me. I don't care.

Jihoon Song: I'm also not… I also don't have a strong opinion whether we should merge this or not, but I just think that we should decide which implementation we wanna go with. It could be this PR, it could be the spec without this PR, But yeah, I think we just need to pick one.

Justin Traglia: Nico asked, when else… oh, yeah, when else do you want to ship this change?

Jihoon Song: I guess, after we ship Sepolia.

Justin Traglia: I'm not sure I'd want to ask clients to change anything principally.

Jihoon Song: I mean…

Justin Traglia: And it's already a bit too late to include this for Sepolia.

Enrico Del Fante: Yeah, I believe the same. Is this, something that we can postpone later. Actually, I don't even know if Tech will… does that already. We need to check.

Potuz: I think it can be triggered anyway, so it doesn't really matter.

Enrico Del Fante: Which turns out that we can just… Leave it open.

Potuz: It doesn't really matter. You're not going to justify a checkpoint, and then… sync. and change your justification to a branch that wasn't FFG compatible before. It's just not going to happen.

Justin Traglia: If it were up to me, I would just delay until after the work. June.

Jihoon Song: I'm okay with that, but I just wanna… Confirm that doesn't mean that we're gonna… Embrace the possibility of split, because some clients already implemented this, some clients might not.

Justin Traglia: Something like Potus is saying is pretty impossible. Or not impossible, just very, very unlikely, right?

Nico Flaig: So at least on the loadestar side, we have a few hacks due to this in our compliance test runner. So, I guess, I mean, you can compliance test this, right? I'm not sure where Lighthouse is there, but at least we could test it properly if we merge this.

Enrico Del Fante: So that's theoretically testable. Even if practically… Not possible.

Nico Flaig: I think this is impossible to even trigger on kurtosis, probably. I wouldn't know how to do it.

Enrico Del Fante: Okay.

Nico Flaig: But I think the concern from Michael is that they don't want to touch their code at all, right? Like, introduce different bugs unrelated to that, maybe.

Enrico Del Fante: Yeah, but if this, that actually doesn't affect anything into the standard reference test generation. It's just a misalignment that… It's temporary, or… Yeah.

Justin Traglia: June.

Enrico Del Fante: Do you know.

Justin Traglia: if this…

Enrico Del Fante: If you leave it open, you probably need to maintain that PR for… For a long time, until we… we go on mainnet with Gloss. Well, assuming the spec are kind of… Stabilized now, probably it's not a big deal.

Justin Traglia: I mean, maintaining it's not a big deal. We can do that. It's just whatever clients want.

Jihoon Song: Yep, Justin, you wanted to ask me something?

Justin Traglia: Oh, I was gonna ask, do you know if this changes any of the reference tests for gloss? Does it have an impact on it?

Jihoon Song: I have to check.

Justin Traglia: I think that's worth checking. Like, if it would force clients to make a change, that would be a good reason not to merge it. What if it doesn't change anything?

Nico Flaig: Yeah, I mean, it should, right? It changes the fork choice compliance tests for bloons.

Enrico Del Fante: Disregarding the classes. Yeah, because they're not really… not all clients are passing them. I believe Freedom and his tech is still… Partially failing a lot of them, so… I don't know if Lighthouse is full green, probably, maybe, maybe Lodestarry is?

Nico Flaig: Yeah, we are agreeing with HEX to pass these tests related to that PR.

Jihoon Song: If you were to ship this after Gloss, are we gonna ship it with ESI, or… Before, Hzzy.

Potuz: This is not a part for change. This can be shipped anytime.

Justin Traglia: Yeah, I mean, just immediately after the glasswork. Like, even we could do version 1.7.1. I can… or… Or 1.8.alpha0, it doesn't really matter. Just as soon as the fork is over.

Enrico Del Fante: Yeah, it'll be fine for me, probably better.

Jihoon Song: Yeah, it seems like We don't have enough consensus to merge this, so… But then we can put this PR open. And… and I guess what we're gonna let clients to do… like, clients to implement at their preferences when it comes to this issue, but after we ship Glas, we can ship, margin SPR, and then… Yeah, change, reference this, or whatever.

Justin Traglia: And worth mentioning that we're doing the exact same thing for the enforcing progressive list limits in the SSV library PR. Like, we're also gonna… Delay merging that until after the fork.

Potuz: If there's no reference desk that… gets added, or, I mean, that changes because of this PR, this PR should add such tests. Or… the compliance test needs to be forced compliance, which, as far as I know, it's not forced. But there's no regression, as far as I can see on that, on that, on that, on that PR.

Justin Traglia: Do you want us to force clients to pass the compliance test?

Potuz: No, I… Well, you can, but then… then we're not going to be passing spec tests, and And, this… this particular one, Prism is not failing anything, and Prism definitely already implements that logic.

Justin Traglia: Yeah. Alright, let's just discuss this more on Discord, or at the next ACD. We're already past time. So, bye everyone, I'm gonna jump off.

Nico Flaig: See ya.

Enrico Del Fante: Right.

Transcript adapted from ethereum/pm, CC BY-SA 3.0.