Wrongfully accepting interface definition in contract body #270

New Issue

2020-07-13T23:01:03+09:00

zxq9 commented

2020-07-13 23:01:03 +09:00

(Migrated from gitlab.com)

Created by: nikita-fuchs

The compiler doesn't complain if you put a contract function interface somewhere into the contract, like https://studio.aepps.com?highlight=19-5-17-1&contract=036202a2-5 (corresponding tab doesn't focus automatically yet, you need to click it)
When you try to deploy it, it of course throws you an error.

*Created by: nikita-fuchs* The compiler doesn't complain if you put a contract function interface somewhere into the contract, like https://studio.aepps.com?highlight=19-5-17-1&contract=036202a2-5 (corresponding tab doesn't focus automatically yet, you need to click it) When you try to deploy it, it of course throws you an error.

zxq9 commented

2020-07-13 23:17:21 +09:00

(Migrated from gitlab.com)

Created by: dincho

Isn't this a feature ? I found it in the tests https://github.com/aeternity/aesophia/blob/lima/test/contracts/address_literals.aes ?

*Created by: dincho* Isn't this a feature ? I found it in the tests https://github.com/aeternity/aesophia/blob/lima/test/contracts/address_literals.aes ?

zxq9 commented

2020-07-13 23:26:28 +09:00

(Migrated from gitlab.com)

Created by: radrow

No, because these entrypoints do have implementation. The case is when you provide just an entrypoint declaration without the body.

EDIT: the entrypoint declarations are okay (and actually mandatory) in the contract interface declarations. The problem is when such an empty declaration appears in a contract definition

*Created by: radrow* No, because these entrypoints do have implementation. The case is when you provide just an entrypoint declaration without the body. EDIT: the entrypoint declarations are okay (and actually mandatory) in the contract *interface declarations*. The problem is when such an empty declaration appears in a contract *definition*

zxq9 commented

2020-07-13 23:38:50 +09:00

(Migrated from gitlab.com)

Created by: radrow

Hey, wrong repo. This is an issue with aesophia, not the http interface

*Created by: radrow* Hey, wrong repo. This is an issue with `aesophia`, not the http interface

zxq9 commented

2020-07-14 06:27:03 +09:00

(Migrated from gitlab.com)

Created by: UlfNorell

Please inline the code (or a shorter version) in the OP.

The contract only compiles if you call the compiler with the no_code flag. Otherwise
it gives you a nice error:

Code generation error in 'RemoteTest.aes' at line 18, col 16:
Missing definition of function 'lol_remote'.

*Created by: UlfNorell* Please inline the code (or a shorter version) in the OP. The contract only compiles if you call the compiler with the `no_code` flag. Otherwise it gives you a nice error: ``` Code generation error in 'RemoteTest.aes' at line 18, col 16: Missing definition of function 'lol_remote'. ```

zxq9 commented

2020-07-14 07:02:40 +09:00

(Migrated from gitlab.com)

Created by: nikita-fuchs

I guess the no_code flag is being a default setting on the hosted compiler then, because the editor isn't calling any of that,

const httpOptions = {
      headers: new HttpHeaders({
        'Content-Type':  'application/json'
      })
    };
return this.http.post<any>(compilerUrl, {"code":`${code}`, "options":{}}, httpOptions);

summoning @dincho , who pointed me to this phenomenon 👋

*Created by: nikita-fuchs* I guess the `no_code` flag is being a default setting on the hosted compiler then, because the editor isn't calling any of that, ``` const httpOptions = { headers: new HttpHeaders({ 'Content-Type': 'application/json' }) }; return this.http.post<any>(compilerUrl, {"code":`${code}`, "options":{}}, httpOptions); ``` summoning @dincho , who pointed me to this phenomenon 👋

zxq9 commented

2020-07-14 16:19:04 +09:00

(Migrated from gitlab.com)

Created by: radrow

What does this flag do anyway?

*Created by: radrow* What does this flag do anyway?

zxq9 commented

2020-07-14 18:08:23 +09:00

(Migrated from gitlab.com)

Created by: UlfNorell

It's supposed to be used when creating calldata and decoding return values and such, where you don't need a complete contract.

*Created by: UlfNorell* It's supposed to be used when creating calldata and decoding return values and such, where you don't need a complete contract.

zxq9 commented

2020-08-31 20:22:48 +09:00

(Migrated from gitlab.com)

Created by: nikita-fuchs

are we going to do anything about this ?

*Created by: nikita-fuchs* are we going to do anything about this ?

ghallak commented

2022-05-05 23:52:38 +09:00

(Migrated from gitlab.com)

@nikita-fuchs I have taken a look, and it does not seem that no_code is used anywhere. I might be wrong about AE Studio not using the /compile endpoint but here is what I found:

The error about missing definition happens when trying to generate FATE code from the source code, but apparently AE Studio does not get to the stage of generating FATE code, since it uses the /aci endpoint to generate ACI and does not use the /compile endpoint which compiles the contract and generates the bytecode.

Generating ACI only does typechecking on the AST, and that is why you do not get the missing definitions error.

@nikita-fuchs I have taken a look, and it does not seem that `no_code` is used anywhere. I might be wrong about AE Studio not using the `/compile` endpoint but here is what I found: The error about missing definition happens when trying to generate FATE code from the source code, but apparently AE Studio does not get to the stage of generating FATE code, since it uses the `/aci` endpoint to generate ACI and does not use the `/compile` endpoint which compiles the contract and generates the bytecode. Generating ACI only does typechecking on the AST, and that is why you do not get the missing definitions error.

zxq9 commented

2022-05-08 18:15:46 +09:00

(Migrated from gitlab.com)

Created by: radrow

@nikita-fuchs could you confirm if the issue is still present? Things have changed since the factories are in, maybe we can close this?

*Created by: radrow* @nikita-fuchs could you confirm if the issue is still present? Things have changed since the factories are in, maybe we can close this?

zxq9 commented

2022-05-09 16:40:35 +09:00

(Migrated from gitlab.com)

Created by: dincho

Generating ACI only does typechecking on the AST, and that is why you do not get the missing definitions error.

Should be this one

*Created by: dincho* > Generating ACI only does typechecking on the AST, and that is why you do not get the missing definitions error. Should be this one

ghallak commented

2022-05-09 16:50:12 +09:00

(Migrated from gitlab.com)

could you confirm if the issue is still present?

@radrow I have checked in AE Studio, and the problem is still there. I just don't think that it's a problem with the compiler. I think we can close the issue after Nikita confirms that AE Studio is not using the /compile endpoint.

>could you confirm if the issue is still present? @radrow I have checked in AE Studio, and the problem is still there. I just don't think that it's a problem with the compiler. I think we can close the issue after Nikita confirms that AE Studio is not using the `/compile` endpoint.

zxq9 commented

2022-05-11 20:53:07 +09:00

(Migrated from gitlab.com)

Created by: nikita-fuchs

IIRC (and @hanssv might want to correct me just in case) we went for /aci because that is way faster than the full compilation process (which is fairly slow) and actually returns the aci that aestudio needs, which is not returned with /compile if I see correctly.

The good developer experience lives and dies with the fast responses we get from /aci. It's understandable that the crunching that takes place here is not that detailed like with /compile, but mistakes in contract code that result from bad syntax need to be shown while writing the contract, not only when it's being deployed.
So could we maybe add some sort linting in the compiler's ACI generator to watch out for certain patterns in some possibly extendable way? I know, some nordic people around here might consider this to not be the cleanest way 😛 but it would get the issue solved and get feedback about wrong syntax fast.

*Created by: nikita-fuchs* IIRC (and @hanssv might want to correct me just in case) we went for `/aci` because that is way faster than the full compilation process (which is fairly slow) and actually returns the `aci` that aestudio needs, which is not returned with `/compile` if I see correctly. The good developer experience lives and dies with the fast responses we get from `/aci`. It's understandable that the crunching that takes place here is not that detailed like with `/compile`, but mistakes in contract code that result from bad syntax need to be shown while writing the contract, not only when it's being deployed. So could we maybe add some sort linting in the compiler's ACI generator to watch out for certain patterns in some possibly extendable way? I know, some nordic people around here might consider this to not be the cleanest way 😛 but it would get the issue solved and get feedback about wrong syntax fast.

ghallak commented

2022-06-15 23:42:13 +09:00

(Migrated from gitlab.com)

@nikita-fuchs Can you share any example in which the call to /aci is way faster than a call to /compile?

There are a few errors that are caught during the transition from AST to fcode (this is the step right after the type checker) and it looks like aestudio is failing on all of them, you can find the list of the errors in the file aeso_code_errors.erl.

I think that it's important to pass through the step of transferring from AST to fcode to catch these errors and report them to the user.

If the problem is that /compile does not return the aci, then I think that could be fixed, but if the call to /aci is way faster than the call to /compile, then it would be beneficial to know which step is causing this slowdown, so if the slowdown is happening after the transition from AST to fcode, maybe we can add another endpoint in which the contract will be checked for code generation errors as well as type errors.

@nikita-fuchs Can you share any example in which the call to `/aci` is way faster than a call to `/compile`? There are a few errors that are caught during the transition from AST to fcode (this is the step right after the type checker) and it looks like aestudio is failing on all of them, you can find the list of the errors in the file [aeso_code_errors.erl](https://github.com/aeternity/aesophia/blob/master/src/aeso_code_errors.erl). I think that it's important to pass through the step of transferring from AST to fcode to catch these errors and report them to the user. If the problem is that `/compile` does not return the aci, then I think that could be fixed, but if the call to `/aci` is way faster than the call to `/compile`, then it would be beneficial to know which step is causing this slowdown, so if the slowdown is happening after the transition from AST to fcode, maybe we can add another endpoint in which the contract will be checked for code generation errors as well as type errors.

zxq9 commented

2022-06-20 18:56:04 +09:00

(Migrated from gitlab.com)

Created by: nikita-fuchs

@nikita-fuchs Can you share any example in which the call to /aci is way faster than a call to /compile?

That was one assumption @hanssv tossed me iirc, he might correct me? I think that was the reason why we had /aci separated from /compile in the first place.

Generally, what you propose would be a smart move. Error-wise, aestudio only displays what it receives from the /aci endpoint. It would of course be better to get 'all the errors'. And actually not everything from the ACI is needed when writing the code, just the ACI for the init function. At the point of deployment, it is of course necessary to have the ACI, the SDK relies on it. So, I don't know, you could add an /compileWithACI endpoint experimentally, and then we see what the performance will be ? Maybe @hanssv has some suggestions on why we should (not) spend our time on that?

*Created by: nikita-fuchs* > @nikita-fuchs Can you share any example in which the call to `/aci` is way faster than a call to `/compile`? That was one assumption @hanssv tossed me iirc, he might correct me? I think that was the reason why we had `/aci` separated from `/compile` in the first place. Generally, what you propose would be a smart move. Error-wise, aestudio only displays what it receives from the `/aci` endpoint. It would of course be better to get 'all the errors'. And actually not everything from the ACI is needed when writing the code, just the ACI for the init function. At the point of deployment, it is of course necessary to have the ACI, the SDK relies on it. So, I don't know, you could add an `/compileWithACI` endpoint experimentally, and then we see what the performance will be ? Maybe @hanssv has some suggestions on why we should (not) spend our time on that?

zxq9 commented

2022-06-21 04:41:42 +09:00

(Migrated from gitlab.com)

Created by: radrow

Can't we catch those errors in the TC so every endpoint is treated fairly? To me it is dodgy that we let code with user errors to be passed to the IR generation phase.

*Created by: radrow* Can't we catch those errors in the TC so every endpoint is treated fairly? To me it is dodgy that we let code with user errors to be passed to the IR generation phase.

zxq9 commented

2022-06-21 06:01:11 +09:00

(Migrated from gitlab.com)

Created by: marc0olo

is this issue related to https://gitlab.com/gajumaristas/aesophia/-/issues/319 ?

*Created by: marc0olo* is this issue related to https://gitlab.com/gajumaristas/aesophia/-/issues/319 ?

ghallak commented

2022-06-21 17:54:03 +09:00

(Migrated from gitlab.com)

Can't we catch those errors in the TC so every endpoint is treated fairly? To me it is dodgy that we let code with user errors to be passed to the IR generation phase.

@radrow You can check the code errors file, I can think of ways to catch most of them in the type checker, but they must be caught in the code generation phase for a reason (I think).

@UlfNorell Could you tell why these errors were not handled in the type checker and were handled during code generation instead? Even though some of them (if not all) can be handled in the type checker.

>Can't we catch those errors in the TC so every endpoint is treated fairly? To me it is dodgy that we let code with user errors to be passed to the IR generation phase. @radrow You can check the code errors file, I can think of ways to catch most of them in the type checker, but they must be caught in the code generation phase for a reason (I think). @UlfNorell Could you tell why these errors were not handled in the type checker and were handled during code generation instead? Even though some of them (if not all) can be handled in the type checker.

zxq9 commented

2022-06-21 18:11:40 +09:00

(Migrated from gitlab.com)

Created by: UlfNorell

I don't think there were any deep reasons. I suspect it was just easier to catch them during code generation. The only case I can think of where you might want to fail in code generation rather than in the type checker is if you're trying to generate code from an otherwise well formed ACI stub. But you could also solve that by looking at the no_code in the type checker.

*Created by: UlfNorell* I don't think there were any deep reasons. I suspect it was just easier to catch them during code generation. The only case I can think of where you might want to fail in code generation rather than in the type checker is if you're trying to generate code from an otherwise well formed ACI stub. But you could also solve that by looking at the `no_code` in the type checker.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QPQ-AG/sophia#270