Skip to content

Implement trait support - Part 2/3 - New ink! codegen #470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 170 commits into from
Sep 28, 2020

Conversation

Robbepop
Copy link
Collaborator

@Robbepop Robbepop commented Jul 4, 2020

Trait Support - New ink! Codegen

This PR implements the new ink! codegen to provide support for ink! trait implementations.

Follow-up PRs

  • Update all ink! crates to actually use version 3.0.0.
  • Make use of the new ink_lang_ir and ink_lang_codegen crates from ink_lang_macro. (Part 3)
  • Implement lint against __ink_ prefixed identifiers generically to be usable by all ink! language macros.
  • Rework ink_core/env provided hashes
    • For every crypto hash provide two APIs:
      1. hash_raw to hash some raw input bytes
      2. hash_encoded to hash some SCALE encodable entity of type T
    • For every crypto hash provide a type that provides simple access to the above 2 APIs.
    • Remove the ink_core/hash module and move all linkers to the above definitions.
  • Convert existing example smart contracts to ink! 3.0 syntax.
    • Flipper
    • Flipper (trait based)
    • Incrementer
    • ERC-20
    • ERC-721
    • Delegator
    • Multi-Signature
    • Domain Name Service (DNS)
  • Write and convert all the acceptance tests for ink! 3.0.
  • Make ink! event topics work, finally: Make ink! 2.0 event topics work #105
    • Find a way to generate event topic implementations in a way that they do not require heap allocations but instead make use of ink!'s static 16kB buffer. This might imply providing yet another high-level ink_core API specific to this purpose.
    • Use BLAKE2 cryptographic hash.
    • Avoid BLAKE2 hashing in case the SCALE encoded values fit into Env::Hash.
    • Encode topics[0] as the event's hashed signature if the ink! event is not anonymous.
    • Since Seal only supports up to MAX_EVENT_TOPICS but cannot distinguish between existing special-cased topics[0] or not we need to make sure that non-anonymous events can actually only have up to MAX_EVENT_TOPICS - 1 topics.

ToDo List

  • Think about #[ink(anonymous)] ink! event definitions.
    • An anonymous ink! event definition would not encode topics[0] as the hash of its signature.
    • Discussed with some Solidity experts: They concluded that anonymous events are not very commonly used. So instead of implementing this feature we will wait until people come up with serious feature requests for it.
  • Improve codegen if all ink! messages are not payable.
    • Instead of performing the check after the message dispatch we can do it once before.
  • Think about better default value for dynamic storage allocator config.
  • Rename storage_alloc ink! config to dynamic_storage_allocator.
  • Add check for every event to guard against maximum number of topics.
  • Adjust EnvTypes trait: Generalize EnvTypes to just Env, add MAX_EVENT_TOPICS constant #469
  • ink! constructors must not be payable. They implicitly are always payable due to endowment.
  • Implement codegen for non-payable ink! messages.
  • Implement codegen for enabled or disabled dynamic storage allocator.
  • Improve codegen of Env type alias. Should be no longer needed ideally and instead rely on ink_lang::ContractEnv trait implementation for the storage struct.
  • Fix potential BaseEvent bug: [ink!3.0] the trait BaseEvent is not implemented #483
  • #[ink::trait_definition] proc. macro that asserts that the given trait definition respects certain rules.
    • implemented analysis
    • implemented codegen
  • Experiment if proc-macro-error crate is actually useful for us for error reporting in the ink_lang_ir crate.
    • Result: It probably would not be of too much help for ink!. However, we could need the ability to output multiple errors at once.
  • Refactor core facilities for cross-contract calling to make them trait-impl proof:
    • Refactor CallBuilder et. al.
    • Refactor InstantiateBuilder et. al. (now called CreateBuilder)
  • Implement the entire rest of ink! codegen:
    • EnvTypes trait
    • ink! storage (#[ink(storage)]) struct
    • ink! event definitions
      • event struct generation + trait impls
      • EmitEvent trait implementation
    • ink! implementation blocks
      • ink! messages
      • ink! constructors
      • #[ink(impl)] defined impl blocks with no ink! specific items
      • non-ink! specific impl items
    • metadata generation procedures
    • cross-calling definitions
      • For ink! messages
      • For ink! trait messages
        • for short-hand syntax
        • for long-hand syntax
      • For ink! constructors
      • For ink! trait contructors
        • for short-hand syntax
        • for long-hand syntax
    • dispatch routines
      • ink! messages dispatch enum
      • ink! constructors dispatch enum
      • dispatch trait impls
      • dispatch entry points
      • DispatchUsingMode impl
      • implement support for trait implementations in dispatch trait impls
      • Make Constr and Msg implementations more concrete around trait impls
    • All other non-ink! specific (Rust) items

Description

The entire codegen library is going to be based on the new ink! IR that has been added in this previous PR.

In the end the lang_lang_macro crate that is the actual proc. macro will only have dependencies to ink_lang_ir and ink_lang_codegen in order to completely do its job of generating the ink! smart contract code.

API

The target API is for ink_lang_codegen is kept minimal:

2020-07-04-163638_1462x209_scrot

Problematic Behavior

Default Trait Impls

Read more about this potential footgun here:
https://gist.github.com/Robbepop/de8a16d6b476a7d0b3bf88f343ac8384

New ink_core::env::call module

The ink_core::env::call module got significantly cleaned up. Usage should be more clear now.
Also its builders are now checking more properly the construction process and should have even less overhead than before.

2020-07-11-132834_1773x1096_scrot

Generated Metadata

Here is the metadata for the Flipper example contract generated with the new codegen:

{
  "metadata_version": "0.1.0",
  "source": {
    "hash": "0x5f9c6583fab4323caebd7db0e57266c6973e3299406dc5ac3324f71017c4edbe",
    "language": "ink! 2.1.0",
    "compiler": "rustc 1.47.0-nightly"
  },
  "contract": {
    "name": "flipper",
    "version": "2.1.0",
    "authors": [
      "Parity Technologies <[email protected]>"
    ]
  },
  "spec": {
    "constructors": [
      {
        "args": [
          {
            "name": "init_value",
            "type": {
              "displayName": [
                "bool"
              ],
              "id": 1
            }
          }
        ],
        "docs": [],
        "name": [
          "new"
        ],
        "selector": "0xd183512b"
      },
      {
        "args": [],
        "docs": [],
        "name": [
          "default"
        ],
        "selector": "0x6a3712e2"
      }
    ],
    "docs": [],
    "events": [],
    "messages": [
      {
        "args": [],
        "docs": [],
        "isPayable": false,
        "mutates": true,
        "name": [
          "flip"
        ],
        "returnType": null,
        "selector": "0xc096a5f3"
      },
      {
        "args": [],
        "docs": [],
        "isPayable": false,
        "mutates": false,
        "name": [
          "get"
        ],
        "returnType": {
          "displayName": [
            "bool"
          ],
          "id": 1
        },
        "selector": "0x1e5ca456"
      }
    ]
  },
  "storage": {
    "struct": {
      "fields": [
        {
          "layout": {
            "cell": {
              "key": "0x0000000000000000000000000000000000000000000000000000000000000000",
              "ty": 1
            }
          },
          "name": "value"
        }
      ]
    }
  },
  "types": [
    {
      "def": {
        "primitive": "bool"
      }
    }
  ]
}

@codecov-commenter
Copy link

codecov-commenter commented Jul 4, 2020

Codecov Report

Merging #470 into master will decrease coverage by 3.92%.
The diff coverage is 1.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #470      +/-   ##
==========================================
- Coverage   84.83%   80.90%   -3.93%     
==========================================
  Files         125      133       +8     
  Lines        5374     5630     +256     
==========================================
- Hits         4559     4555       -4     
- Misses        815     1075     +260     
Impacted Files Coverage Δ
lang/codegen/src/generator/contract.rs 0.00% <0.00%> (ø)
lang/codegen/src/generator/cross_calling.rs 0.00% <0.00%> (ø)
lang/codegen/src/generator/dispatch.rs 0.00% <0.00%> (ø)
lang/codegen/src/generator/env.rs 0.00% <0.00%> (ø)
lang/codegen/src/generator/events.rs 0.00% <0.00%> (ø)
lang/codegen/src/generator/storage.rs 0.00% <0.00%> (ø)
lang/codegen/src/lib.rs 0.00% <0.00%> (ø)
lang/codegen/src/traits.rs 0.00% <0.00%> (ø)
lang/ir/src/ir/item/event.rs 68.91% <0.00%> (-3.94%) ⬇️
lang/ir/src/ir/item/storage.rs 63.63% <0.00%> (-4.11%) ⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c24f0ff...81f9dcc. Read the comment docs.

Robbepop added 29 commits July 10, 2020 20:57
This new codegen is based on the (also) new ink! IR.
…ctors

Also this implementation avoids a lot of code duplication with the similar code generation for dispatch trait impl of ink! messages.
…fault

New default is set to "false" so only contracts that actually use this feature need to specify it.
Also renamed to dynamic_storage_allocator to better carry intention.
Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WIP

Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

53/62

Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Robbepop Robbepop merged commit 5d7d855 into master Sep 28, 2020
@Robbepop Robbepop deleted the robin-implement-trait-codegen branch September 28, 2020 12:35
Copy link
Collaborator

@cmichi cmichi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ascjones ascjones mentioned this pull request May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants