Contract factories and bytecode introspection #796

Merged
zxq9 merged 34 commits from factories into master 2021-05-18 19:21:57 +09:00
zxq9 commented 2021-04-07 15:45:09 +09:00 (Migrated from gitlab.com)

Created by: radrow

This PR adds support for CLONE, CLONE_G, CREATE and BYTECODE_HASH opcodes. fixes #197

The syntax details are under discussion. Forum thread

*Created by: radrow* This PR adds support for `CLONE`, `CLONE_G`, `CREATE` and `BYTECODE_HASH` opcodes. fixes #197 The syntax details are under discussion. [Forum thread](https://forum.aeternity.com/t/syntax-for-the-upcoming-features-factories-code-introspection/9069)
zxq9 commented 2021-05-12 18:21:47 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

This will cause some interoperability issues between contracts using the old and the new compiler, if there are entrypoints taking singleton record types as arguments or returning them as results.

Also, this PR doesn't actually change this??

*Created by: UlfNorell* This will cause some interoperability issues between contracts using the old and the new compiler, if there are entrypoints taking singleton record types as arguments or returning them as results. Also, this PR doesn't actually change this??
zxq9 commented 2021-05-12 18:27:34 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

               | stateful | private | payable | main | interface].
*Created by: UlfNorell* ```suggestion:-0+0 | stateful | private | payable | main | interface]. ```
zxq9 commented 2021-05-12 18:53:00 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Note that this fails because l isn't actually a LowerDisorderAnarchy contract.

*Created by: UlfNorell* Note that this fails because `l` isn't actually a `LowerDisorderAnarchy` contract.
zxq9 commented 2021-05-12 18:58:46 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Also failing tests for new type errors (main contract and varargs stuff)

*Created by: UlfNorell* Also failing tests for new type errors (main contract and varargs stuff)
zxq9 commented 2021-05-12 19:01:54 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

last_declaration_must_be_main_contract is what aeso_ast_to_fcode throws.

*Created by: UlfNorell* `last_declaration_must_be_main_contract` is what `aeso_ast_to_fcode` throws.
zxq9 commented 2021-05-12 19:02:52 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Expected a main contract

*Created by: UlfNorell* Expected a *main* contract
zxq9 commented 2021-05-12 20:17:28 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Don't we need to check that it's an entrypoint here?

*Created by: UlfNorell* Don't we need to check that it's an entrypoint here?
zxq9 commented 2021-05-12 20:40:56 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

This doesn't let us clone or get byte code hash of ourselves, right? This was one of the features asked for in #295.

This is something we probably want to fix later though, and the current design allows for it, as long as we add a contract type for the main contract.

*Created by: UlfNorell* This doesn't let us clone or get byte code hash of ourselves, right? This was one of the features asked for in #295. This is something we probably want to fix later though, and the current design allows for it, as long as we add a contract type for the main contract.
zxq9 commented 2021-05-12 21:30:57 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Missing mk_error for this one. Also type_error doesn't throw an exception (it returns true) so you need to return an actual type here. Suggestion: fresh_uvar(Ann).

*Created by: UlfNorell* Missing `mk_error` for this one. Also `type_error` doesn't throw an exception (it returns `true`) so you need to return an actual type here. Suggestion: `fresh_uvar(Ann)`.
zxq9 commented 2021-05-12 21:39:39 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

    type_error({unify_varargs, When});
*Created by: UlfNorell* ```suggestion:-0+0 type_error({unify_varargs, When}); ```
zxq9 commented 2021-05-12 21:49:46 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

"point it out" or "point to it"
"the main keyword"

*Created by: UlfNorell* "point it *out*" or "point *to* it" "*the* `main` keyword"
zxq9 commented 2021-05-12 21:50:11 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

    Msg = "Only one main contract can be defined.",
*Created by: UlfNorell* ```suggestion:-0+0 Msg = "Only one main contract can be defined.", ```
zxq9 commented 2021-05-12 21:54:28 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

    Msg = "Cannot unify variable argument list.\n",

io_lib:format/1 does not exist

*Created by: UlfNorell* ```suggestion:-0+0 Msg = "Cannot unify variable argument list.\n", ``` `io_lib:format/1` does not exist
zxq9 commented 2021-05-12 21:57:23 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Use also in pp_why_record

*Created by: UlfNorell* Use also in `pp_why_record`
zxq9 commented 2021-05-12 22:09:01 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

-spec ast_to_fcode(aeso_syntax:ast(), [option()]) -> {env(), fcode()}.
*Created by: UlfNorell* ```suggestion:-0+0 -spec ast_to_fcode(aeso_syntax:ast(), [option()]) -> {env(), fcode()}. ```
zxq9 commented 2021-05-12 22:15:40 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Consider renaming main_contract context?

*Created by: UlfNorell* Consider renaming `main_contract` context?
zxq9 commented 2021-05-12 22:20:31 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

We're not checking that the main contract is last anywhere else, so this needs to be a proper error.

*Created by: UlfNorell* We're not checking that the main contract is last anywhere else, so this needs to be a proper error.
zxq9 commented 2021-05-12 22:21:04 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

-spec to_fcode(env(), aeso_syntax:ast()) -> {env(), fcode()}.
*Created by: UlfNorell* ```suggestion:-0+0 -spec to_fcode(env(), aeso_syntax:ast()) -> {env(), fcode()}. ```
zxq9 commented 2021-05-12 23:02:18 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Handle in pp_fexpr

*Created by: UlfNorell* Handle in `pp_fexpr`
zxq9 commented 2021-05-12 23:07:33 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Future optimizations:

  • don't compile child contract for every literal
  • don't duplicate the literal for every Chain.create
*Created by: UlfNorell* Future optimizations: - don't compile child contract for every literal - don't duplicate the literal for every `Chain.create`
zxq9 commented 2021-05-12 23:10:04 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Not actually SCode. FateCode maybe?

*Created by: UlfNorell* Not actually SCode. `FateCode` maybe?
zxq9 commented 2021-05-12 23:23:28 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

This seems bad. This string is hashed and put on-chain so that people can verify the origin of on-chain contracts. This should be source_hash instead, with a hash of something like the full contract source + the name of the child contract.

*Created by: UlfNorell* This seems bad. This string is hashed and put on-chain so that people can verify the origin of on-chain contracts. This should be `source_hash` instead, with a hash of something like the full contract source + the name of the child contract.
zxq9 commented 2021-05-12 23:23:55 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

This field is not used by aeser_contract_code:serialize.

*Created by: UlfNorell* This field is not used by `aeser_contract_code:serialize`.
zxq9 commented 2021-05-12 23:26:30 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

*Created by: UlfNorell* ```suggestion:-0+0 ```
zxq9 commented 2021-05-12 23:31:09 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

This seems wrong. The compiler (ast_to_fcode) will put the arguments in the order they are listed in the type, so two functions with named arguments listed in different orders will get compiled differently.

*Created by: UlfNorell* This seems wrong. The compiler (`ast_to_fcode`) will put the arguments in the order they are listed in the type, so two functions with named arguments listed in different orders will get compiled differently.
zxq9 commented 2021-05-12 23:34:53 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Not actually pure.

*Created by: UlfNorell* Not actually pure.
zxq9 commented 2021-05-12 23:38:59 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Here would be a good place to compute source hashes for child contracts and store them in the ChildContracts map.

*Created by: UlfNorell* Here would be a good place to compute source hashes for child contracts and store them in the `ChildContracts` map.
zxq9 commented 2021-05-12 23:41:10 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Review: Changes requested

Reviewed together with @hanssv

*Created by: UlfNorell* **Review:** Changes requested Reviewed together with @hanssv
zxq9 commented 2021-05-13 00:30:16 +09:00 (Migrated from gitlab.com)

Created by: radrow

  • sneak in clone if the contract was created previously
*Created by: radrow* - sneak in `clone` if the contract was created previously
zxq9 commented 2021-05-13 00:36:42 +09:00 (Migrated from gitlab.com)

Created by: radrow

Let's postpone it to the next iteration

*Created by: radrow* Let's postpone it to the next iteration
zxq9 commented 2021-05-13 00:56:36 +09:00 (Migrated from gitlab.com)

Created by: radrow

contract_def

*Created by: radrow* `contract_def`
zxq9 commented 2021-05-13 00:58:53 +09:00 (Migrated from gitlab.com)

Created by: radrow

identify_main_contract will now move it to the end

*Created by: radrow* `identify_main_contract` will now move it to the end
zxq9 commented 2021-05-13 01:02:46 +09:00 (Migrated from gitlab.com)

Created by: radrow

So what's wrong?

*Created by: radrow* So what's wrong?
zxq9 commented 2021-05-13 01:03:24 +09:00 (Migrated from gitlab.com)

Created by: radrow

Let's move it to the next iteration

*Created by: radrow* Let's move it to the next iteration
zxq9 commented 2021-05-13 03:11:57 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

last_declartion_must_be_contract vs last_declaration_must_be_MAIN_contract

*Created by: UlfNorell* `last_declartion_must_be_contract` vs `last_declaration_must_be_MAIN_contract`
zxq9 commented 2021-05-14 15:43:31 +09:00 (Migrated from gitlab.com)

Created by: hanssv

Just had a quick look at the fixes and it looks promising! There is still a bunch of new errors not covered by tests and dialyzer seems unhappy.

*Created by: hanssv* Just had a quick look at the fixes and it looks promising! There is still a bunch of new errors not covered by tests and dialyzer seems unhappy.
zxq9 commented 2021-05-17 23:31:29 +09:00 (Migrated from gitlab.com)

Created by: hanssv

🙌

*Created by: hanssv* 🙌
zxq9 commented 2021-05-18 15:45:12 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

This is not a grammatically correct use of affine. If you really want to use affine it would be "The charged gas is an affine function of the size of the serialized bytecode", but I would say "increases linearly with" instead.

*Created by: UlfNorell* This is not a grammatically correct use of *affine*. If you really want to use affine it would be "The charged gas is an affine function of the size of the serialized bytecode", but I would say "increases linearly with" instead.
zxq9 commented 2021-05-18 15:45:33 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

affine

*Created by: UlfNorell* affine
zxq9 commented 2021-05-18 15:49:06 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Is this different from contracts deployed manually (Call.value not being set)?. If so, what's the reason for this?

*Created by: UlfNorell* Is this different from contracts deployed manually (`Call.value` not being set)?. If so, what's the reason for this?
zxq9 commented 2021-05-18 16:15:42 +09:00 (Migrated from gitlab.com)

Created by: radrow

It isn't different. I just got concerned that people using factories may want to validate the create value with Call.value (which also I did when testing the opcodes, as it was absolutely intuitive)

*Created by: radrow* It isn't different. I just got concerned that people using factories may want to validate the `create` value with `Call.value` (which also I did when testing the opcodes, as it was absolutely intuitive)
zxq9 commented 2021-05-18 16:18:46 +09:00 (Migrated from gitlab.com)

Created by: radrow

The more you know, fixed

*Created by: radrow* The more you know, fixed
zxq9 commented 2021-05-18 16:30:11 +09:00 (Migrated from gitlab.com)

Created by: UlfNorell

Review: Approved

*Created by: UlfNorell* **Review:** Approved
zxq9 commented 2021-05-18 18:03:35 +09:00 (Migrated from gitlab.com)

Created by: hanssv

Review: Approved

*Created by: hanssv* **Review:** Approved
zxq9 commented 2021-05-18 19:21:57 +09:00 (Migrated from gitlab.com)

Merged by: radrow at 2021-05-18 10:21:57 UTC

*Merged by: radrow at 2021-05-18 10:21:57 UTC*
Sign in to join this conversation.
No description provided.