eth_simulate Implementers' Meeting #54

Transcript

I guess we can start. You had some issues with the spec. >> Yes. I don't know if you can hear me cuz it's raining heavily over here. >> Yeah, >> we can hear it. >> Okay. Um there is an issue because on execution API if you run the spell check it throws an error which kind of like whenever a PR is being put up on execution API the CI fails because of that and that was because we we have like um we are supposed to have an um a type not an array so that's causing like a problem

and I have a question I don't know if it is a hard rule But if it's possible for us to modify the schema itself by including both an object and and a and an array it will be okay. I don't know if it is a hard if it had maybe that would be the better option. It's kind of like to modify the test itself. >> If the issue that the log state calls is an array and I don't know why it wouldn't be an array. White has to be object.

>> I don't know if I can. >> Can you give a can you share a link to the line of code in GitHub? That's the problem or the line of the spec. >> Okay. You can actually I can link the PR that it's blocking, but for you to actually see it, you need to like run spec check on execution API repo. >> PR is fine. >> Yeah, the PR is fine. I can go look at the GitHub action. >> Oh, okay.

where this text added to the repo afterwards it was merged since there was no errors when it was merged so I don't know if these are new text and now the new text detected this kind of issue >> um the thing is from the RPC test gen it doesn't hold up an issue but from um the execution API side it gives out an error Okay. So it's not the execution spec GitHub but the error is raised to somewhere else.

>> Yes. Because the two um the two repos are kind of talking to each other because whatever test that generated from the RPC test gen it's get transferred as a io file to execution API. Um, That's the P. You can check the CI fill. I don't know if it's possible for me to share my screen. Hold on. This this error is painful to read.

>> Yes. And it's kind it's long. That's another thing. The test has like several batch test to it. Can I share my screen? >> Should be able to. Yeah. Now I don't know how to shoot. Okay. >> It doesn't give a line number. That's frustrating.

>> No. Yeah, >> but it's I the the the the the what's it called? Um I think I shared the line on Telegram 4 something something. >> I mean the the line of the spec that's the problem, not the line of the code that threw the error. Can you see my screen? That is the arrow. >> Yes. Yes, we can see your screen. But this looks like error in the test cases, not in the spec that you are running a test case that we have and not an out of spec and they there can be test that are kind of not

valid in spec. >> No, it's not like the test is bad. The thing is that the repo is expecting um a type but an array is being given out. So that's a problem. The test is okay. So we need to kind of like modify the schema itself if it is not a hard to include both a and um an object and an array. Do do you know which where specifically in the spec the there's an issue like in the JSON?

>> Look at it here. >> Yeah, that looks like that looks like a test case. I agree with Delori. Why do you think this is not an issue with the test case? >> Also, where are those test cases defined? Shut up.

>> No, I see it's failing on the very first one, suggesting that maybe it's just would fail on all of them. >> Do you understand it now? Not fully. >> I also don't completely understand what's the issue since that that's clearly one test case and the test case is failing. But I don't know if it's because of the test case is bad or because the spec is bad. >> Maybe it's saying that the test doesn't match the spec.

>> No, what is happening? The problem is that we are calling multiple block state calls and then binding them into a response like it's coming is into a slice. That's the problem. It's not that the test case is bad. The the the spec itself requires um we have like the JSON area in our result of JSON output. It's supposed to be a type and not an array. That's where the issues coming from. So I'm still confused what is spitting out what so we we have a spec in JSON in the re

repository and then when the someone makes a change CI runs and it executes some tests one of those tests is failing with an error and that's the test the one you you listed >> and the test. Looking at the test, the test is literally just two JSON objects, right? I'm guessing an input and an output. >> Yes. >> Okay. And so do we know is that the input the input is what we're putting in. The output presumably is what we expect to get out. And as long as what we get out matches that then test passes. If what we get out doesn't match, it fails. The failure is the failure on parsing the input or is it on comparing the output?

The failure is that it's evaluating the expected object type that is supposed to give out. >> Okay, so it's >> the schema itself doesn't accept an array. It's looking at the output that so okay so it it took this input JSON it fed it into whatever and it's then um the test then gives an output out and then the test harness is trying to parse that output and that's where it's failing.

I'm trying to figure out if it's failing on parsing the input or failing on parsing the output >> the output. parsing the output and I think what Kari is saying is that these tests we shouldn't expect them to have valid output. Is that right? >> Well, some of the tests might not have I don't know which ones. Well, well, the the output should be always valid because it can be also error and the error should be valid. So, I think they should be always a valid specs.

I think the inputs the inputs might not always be valid because I want to test what happens on the invalid inputs. >> Okay. So, Mercy, you're saying that the expected output for this test, which I've got in front of me, doesn't match what the spec says the output should be shaped like. Is that correct? Yes, it should be a type. But we having like several slices which is giving out an array. That's just a problem. >> Hang on. What do you mean it should be a type? It should be an object that has JSON RPC, ID, result. The result is an array of objects. Those objects have properties. Which part of that do you think is wrong?

>> Okay. Um, our looks like is a type and an object. But the result we're getting out is the output we're getting out is an array, not an object. So we need to like modify the schema itself to contain the both of them in cases where we have object and array so that this kind of cases can repeat itself. >> Why do you think that a array is not allowed here? Hang on. Where in where in the repo here is the actual spec source schema?

>> you know glory where I'm looking at the execution API repo. Where is the actual >> sour at aum execute? I share the link and it executed. It's the last method defined in the in that execution y and then the result results is an object that it's result object. >> And where do I find those components that it points at? I think then this schemas >> under schemas where under schemas >> I think schemas execute yeah under schemas execute test the simulate result which it's an object

and don't remember what the simulate >> okay so I think I I think I'm finally catching up here so we have the the result should be an eat simulate result and then neat simulate result is defined as an object but the thing that we have in the test is an array. Okay. Yes. Um so it does seem like so why is the result an array here? Which one's correct? It's the first question. >> Okay. I that is why I don't know the reason behind the array and the type. I don't know if it's a hard rule or not.

So that's why I brought it to you guys because >> so which should we expect arrays responses from these? >> I think the answer is yes. One object one object per block. That's what I'm seeing here. I'm seeing a array of blocks. And if I look for the number 2 E2F, yeah, it's just giving me an array of blocks back. So the result object is is not in fact a an object as the schema suggests which is what Merc is explaining.

>> Oh yeah yeah yeah it should be object on array because it's invalid then it should be an object a success then should be an array. >> I guess that's the issue. Yeah. I don't know how that should be logged in the ammo. >> Yeah. So the the type though of the ETH simulate result is object. >> Yeah. >> It's not it's not array or object. >> But the simulate result is an object or array >> with no um discriminant.

How did this not come out during implementation and testing? I would have thought someone would have complained about this because they're using probably what get replies and not using the spec itself and then Cena has implemented the get kind of according to spec but not exactly like the spec and that I think that's the issue with this YAML things. >> Yeah, that's another I'm working on. What what is the actual thing that GU returns? Does it return an object that has like a prop two properties on it?

One of which is success or failure and one is the like data or is it literally just like is this a discriminated union or is this a non-discriminated union? Is there some something that someone who receives this could switch on like as a user? Why? Like you implement this as a user. That's where I'm confused, I guess, is because I would have imagined this would have been super annoying to actually implement. Did you get back an OB? You get back. Do you just like check is array and that's how you determine if it's an error?

>> Giving you an example. like here's two cases. The other where the is successful and returns result and then I just remove everything from the result or it does an error and then there's an error object.

>> I see. Okay, that's that's totally fine. Um just the the YAML is incorrect here. I understand. No. So this YAML should be let's see just looking through this. So the schema for the result is not an object. It's a one of No, no, no. Sorry, that's not correct. The schema of for the results. >> Yeah. I don't know how to fit the mod. >> Yeah. So, um, eat simulate v1 result.

Yeah. So, okay. E simulate result is an object and it is an object that has um you're missing you're missing a level here. So there's e simulate result is an object. That object has is is a union of two different objects. One of those objects is an object that has the word result and it and that that property is an array. Another one is an object that has a property called message which is an object or sorry it's a string or sorry is an object. Yeah, sorry.

One of the two objects in the union has a single property called result and that contain that type of that property is an array. The other object in that union has a single property called error and the type of that property is an object. And what we have here is we don't have that. So we we don't have that intermediate object that has either a result property or an error property. That's the piece we're missing. And so I think what we need to do is we need to change um so each simulate result I'm looking at the schema and source schema that's execute.l simulate result is an object that's correct but it's not a um and even one of is also correct. Think each simulate

block result success should be an object that has a single property on it called result and that property is of type array. >> And then the e simulate block result invalid should also be an object with a single property on it and that property is called error. >> Invalid is yeah invalid is correct. So all is missing is e simulate block result success needs to be an object with a single property on it called result and that property then its type is an array of that property not the type of the object. So, uh, yes, this is a good find, Mercy. The fix should be

pretty simple. There's just an issue with the spec. Um, do you think you can take care of that, Kari? >> Yeah, I can need to remind how the syntax works like how the >> Yeah, hardest part is the Y. I think we can ba basically just I think you can just kind of copy the e simulate block result invalid for the most part. So title >> reference.

>> Yeah. And so it's it's got a required property called error. That that property error is then a um type which then references your uh e simulate block result single success. >> Yeah, I think I have it. Let me I might actually be able to do it right now now that I'm already thinking about it. Unless you've already done it.

>> I'm still looking how that reference. >> But yeah, if you have it already something like this, but then I don't know if the ref is correct. I always wonder how the syntax for ref works. Uh if you look at the withdraw right above it in the spec file, they're using references for their property values. >> And so you can do the basic yeah then I think I guessed correctly except there's a title that is missing but the title is mandatory I guess.

>> Probably wouldn't hurt to include it even if it's not. No, no, no, no, sorry. Um, this what you have is not quite correct. You have properties, result, result is an array. So, the results will have a title and a type field. Type is array and then I don't actually know how to define array types. >> How do I make an array? I guess >> you need an you need an item. So, you have type array and then items. Then if you go look at eat simulate block result success that is very the syntax there is very

close to what you need or it's got an items field. Sorry this one is broken. Huh? But anyways that's just basically you just need to nest that. So that type array is actually the type of the property called result on the object. You should be able to do this all within this one object. >> But yeah, I can probably figure it out. >> Okay, if you submit a PR, I can review it. Thank you for finding that Mercy and doing the troubleshooting to debug it.

Um, the fix should be very small and easy and it should cause things to pass. I am curious, why did this fail now instead of when we submitted our PR before? Um >> I think there's no test. >> I don't really know why. So because I thought of the same thing but I couldn't come up come up with any reason why it didn't happen before now. So, >> and so if you >> or something, >> my guess is the way the CI is set up, the CI is incorrectly running against master. And so when you submit a PR, it's running the CI against master, not against the pull request.

This is just like a configuration of the CI problem. And so you won't get an error until the next person submits a PR after the PR was merged. That's when you'll get the errors. Um, this can happen if you in your CI setup you're like cloning a repo or something, you might accidentally clone the head instead of the PR. That's my best guess. If this is if this failed on the first PR after ours, I suggest the CI is wrong. Uh, which job was this? This was lint and check.

So if I submit a pull request, it doesn't fail until we merge to master. >> No. So if you submit that pull request, it's going to fail as well. Um because the CI is testing against master. It's not testing against the pull request. That's my guess. Just based on the symptoms and my experience working with CI, that's the most likely thing I can think of that would cause this sort of thing. I'm checking right now. Is it possible to like get light client on this because I think he's like the best person to >> uh he might know. Yeah.

>> Yeah. Yeah. >> Just just let him know that. >> He hardly responds to me. >> Watch him. >> I mean, sure. If you want to mention him and me in a channel somewhere. Um, are you in Discord the ETH R&D? I know both him and I are in that one. >> So, maybe in the JSON RPC API. Um, I can mention him actually. I keep saying other people should do things and I'm already halfway done by the time I explain it.

I mean, if I make a pull request about it and then I'm not fully confident that it works with and knowing that I cannot test if the SP is correct. >> Yeah, I'll mention him and I'll CCU on it. Like maybe this is correct but then I

would like to test or is it What's your um handle in Discord? Mercy. >> Bulma naps. B O M A N_naps. >> Bummanaps. >> Okay. I mentioned both you and Laurc Do anything else?

We can continue on the discord for this topic. >> Yeah, we can continue on discord. Um, let's see if light client has a good quick easy solution or he knows. If not, we can yolo and put your PR in and see if it fixes it after we merge. >> Okay. I was thinking um hold on. Yeah, I would prefer that light grit would fix that issue and then I make the pull request and then I would see if it works or not and then we merge. But >> submit the pull request and if the pull request fails for the same >> I don't so I don't think that's allowed

and also that's not correct. The the spec was wrong. You you were correct. The spec is wrong here and that was our our mistake. It it's supposed to be an object that has one of two properties. So it's a union of two objects. You can either have an object that has a result property on it or you can have an object that has a error property on it and the type of those reflective properties are different. So the type of the error property is a string and the type of the sorry type of the error property is an object. The type of the result property is an array. But the object can only have one of one one of the two. It cannot have both. So it's either >> the error property or it's the result property. Again, this is just a mistake

>> Okay. >> Nothing for me. >> We'll talk to you next week. I need the channel. >> Okay. Thank you. >> Later. Thanks, Mercy. >> Good fight.