Skip to content
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

ink! Analyzer #1615

Merged
merged 1 commit into from
Mar 22, 2023
Merged

ink! Analyzer #1615

merged 1 commit into from
Mar 22, 2023

Conversation

davidsemakula
Copy link
Contributor

Project Abstract

ink! analyzer is a collection of modular and reusable libraries and tools for semantic analysis of ink! smart contract code.

ink! analyzer aims to improve ink! language support in integrated development environments (IDEs), source code editors and other development tools by providing modular and reusable building blocks for implementing features like diagnostic errors, code completion suggestions, code/intent actions and hover content for the ink! programming language which is used for writing smart contracts for blockchains built on Substrate.

Relying on only generic Rust language support in IDEs, code editors and other development tools has some significant limitations for the developer experience including:

To solve the above challenges and improve ink! language support in IDEs, code editors and other development tools, this project will create two main components:

  • a modular domain-specific semantic analysis library for ink!.
  • a Language Server Protocol (LSP) implementation built on top of the aforementioned semantic analysis library.

Grant level

  • Level 1: Up to $10,000, 2 approvals
  • Level 2: Up to $30,000, 3 approvals
  • Level 3: Unlimited, 5 approvals (for >$100k: Web3 Foundation Council approval)

Application Checklist

  • The application template has been copied and aptly renamed (project_name.md).
  • I have read the application guidelines.
  • Payment details have been provided (bank details via email or BTC, Ethereum (USDC/DAI) or Polkadot/Kusama (USDT) address in the application).
  • The software delivered for this grant will be released under an open-source license specified in the application.
  • The initial PR contains only one commit (squash and force-push if needed).
  • The grant will only be announced once the first milestone has been accepted (see the announcement guidelines).
  • I prefer the discussion of this application to take place in a private Element/Matrix channel. My username is: @_______:matrix.org (change the homeserver if you use a different one)

@davidsemakula davidsemakula changed the title Add ink! analyzer ink! Analyzer Mar 15, 2023
@semuelle semuelle self-assigned this Mar 16, 2023
@davidsemakula
Copy link
Contributor Author

Hi @semuelle 👋
I'm just checking in ... anything I can add/clarify to help the review along?🙂

@keeganquigley keeganquigley added the ready for review The project is ready to be reviewed by the committee members. label Mar 20, 2023
Copy link
Contributor

@keeganquigley keeganquigley left a comment

Choose a reason for hiding this comment

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

Hi @davidsemakula thanks for the interesting application. I think this is a very useful project and would be happy to support it. Regarding static analysis, there is also substrace, and saft for frame pallets. Just wanted to mention these for you to check out, however I believe both are smaller research projects and don't have a VS-Code extension.

@davidsemakula
Copy link
Contributor Author

Hi @keeganquigley
Thanks for the review 🙂.
I actually checked out substrace and SAFT during my research. I didn't mention either of them because, as you noted, they're targeted at FRAME and Substrate and don't implement anything ink! or smart contract specific AFAICT.
I used the term "smart contract static code analyzer" when evaluating relevant projects in the ecosystem to narrow down the scope and so I only mention Vanguard. I'm happy to add a note about them if its necessary, I just didn't think they were relevant to this project's objective.

@keeganquigley
Copy link
Contributor

thanks for the explanation @davidsemakula yes I figured that would be the case considering you are focusing on ink! specifically. So no need to include them. However, it's possible that other committee members may ask more questions during the approval process.

@Noc2 Noc2 requested a review from bhargavbh March 21, 2023 07:53
Copy link
Collaborator

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

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

Looks interesting to me, and I'm happy to approve it. You might want to take a look at this RFP: https://github.com/w3f/Grants-Program/blob/master/docs/RFPs/Open/Static-Analysis-for-Runtime-Pallets.md Ideally, at some point, there are tools for semantic as well as static analysis for ink, and I believe you mentioned this already as part of your future roadmap.

@davidsemakula
Copy link
Contributor Author

davidsemakula commented Mar 21, 2023

Hi @Noc2
Thanks for the review 🙂

You might want to take a look at this RFP: https://github.com/w3f/Grants-Program/blob/master/docs/RFPs/Open/Static-Analysis-for-Runtime-Pallets.md

Will do, thanks for sharing.

Ideally, at some point, there are tools for semantic as well as static analysis for ink, and I believe you mentioned this already as part of your future roadmap.

Yes, it's definitely something I'll be adding in the future, I just wanted to keep the initial scope relatively small.

Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

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

Great project and great application. I would be happy to see this come to life.
Just one question: Do Docker deliverables really make sense in this project? Wouldn't a Makefile for building/testing/installing locally make more sense?

@davidsemakula
Copy link
Contributor Author

Hi @semuelle
Thanks for the review 🙂

Do Docker deliverables really make sense in this project? Wouldn't a Makefile for building/testing/installing locally make more sense?

So specifically for milestone 4, VS Code has this feature called Dev Containers that allows you to use a docker container as an isolated development environment that I think will be a neat and easy way for testing the VS Code extension. So for me this and the "apparent" preference for Docker testing environments in the application template made Docker look like the obvious choice.
I personally have no strong preference between Dockerfiles or Makefiles for this. LMK if Makefiles are a strong preference on your side and I'll update accordingly.

@davidsemakula davidsemakula requested review from semuelle and removed request for bhargavbh March 22, 2023 16:45
@davidsemakula
Copy link
Contributor Author

@Noc2 Looks like me requesting a re-review from Sebastian removed your review request from @bhargavbh 🙈
Sorry about that, please free to re-add the request 🙂

@semuelle semuelle merged commit 40c75b5 into w3f:master Mar 22, 2023
@github-actions
Copy link
Contributor

Congratulations and welcome to the Web3 Foundation Grants Program! Please refer to our Milestone Delivery repository for instructions on how to submit milestones and invoices, our FAQ for frequently asked questions and the support section of our README for more ways to find answers to your questions.

Before you start, take a moment to read through our announcement guidelines for all communications related to the grant or make them known to the right person in your organisation. In particular, please don't announce the grant publicly before at least the first milestone of your project has been approved. At that point or shortly before, you can get in touch with us at grantsPR@web3.foundation and we'll be happy to collaborate on an announcement about the work you’re doing.

Lastly, please remember to let us know in case you run into any delays or deviate from the deliverables in your application. You can either leave a comment here or directly request to amend your application via PR. We wish you luck with your project! 🚀

@keeganquigley
Copy link
Contributor

@davidsemakula Congrats on the acceptance of the grant! Just to let you know, there is an ink! security meetup happening in Berlin next week. You can find details here.

Not sure where you are based out of but just wanted to forward the info in case you are able to attend :) Looking forward to the deliveries.

@davidsemakula
Copy link
Contributor Author

Thanks @keeganquigley,
I'm based in Uganda so not likely than I can attend on such short notice unfortunately.
Still good to know though 🙂

Super excited to work on the deliveries for this as well. 🚀

@davidsemakula
Copy link
Contributor Author

davidsemakula commented Mar 22, 2023

Thanks for taking the time to review and approve @keeganquigley, @Noc2 and @semuelle 🙂

@bhargavbh
Copy link
Contributor

Hi all. Sorry for the late reply as i was OoO. Even though the grant has been approved, i would still like to pitch in to get some clarity, since this is a very relevant project and would love to see it succeed.

  1. what is the source for coming up with the semantic rules of the ink eDSL? you did mention a few examples but would be nice to know what methodology is used for extracting such rules. It would be a pity to build all this machinery and pipeline without a decent number of concrete rules which are ink specific. It makes sense to formulate such precise rules, which would also help in testing and benchmarking.
  2. the choice for using rust-analysers parser over ink_IR is well justified (resilient and lossless properties make sense). Does the semantic analyser boil down to generating ink's IR and AST using rust_ap_syntax and rust_ap_ide, instead of syn? For sanity check, you would want to compare the IR/AST generated by rust_ap_syntax with the IR/AST generated by syn, and it should match for well-formed programs, right?
  3. will all of the analysis be performed at the ink-IR level, without unwrapping the macro? would the information in ink-AST subsume the information of unwrapped rust AST?

@davidsemakula
Copy link
Contributor Author

Hi @bhargavbh




Thanks for taking the time to review 🙂.
This is a great set of questions.

  1. what is the source for coming up with the semantic rules of the ink eDSL? you did mention a few examples but would be nice to know what methodology is used for extracting such rules. It would be a pity to build all this machinery and pipeline without a decent number of concrete rules which are ink specific. It makes sense to formulate such precise rules, which would also help in testing and benchmarking.

I briefly mentioned in the architecture section that the ink_ir crate will be used as a reference implementation for ink's semantic rules but I agree that it makes sense to share the methodology for extracting the semantic rules.


So for each ink! macro defined in the ink_macro crate, we'll be extracting these rules from its corresponding ink_ir type, the types and functions it uses, as well as related unit tests.

Using the #[ink::contract] attribute-macro as an example, from it's implementation in the ink_macro crate
, we can trace it's corresponding ink_ir type as the Contract struct in the contracts module of the ink_ir crate.
We can then extract the semantic rules by recursively analyzing the types and functions used in the Contract struct's constructor as well as related unit tests with the following being the main modules of interest
https://github.com/paritytech/ink/blob/master/crates/ink/ir/src/ir/item_mod.rs
https://github.com/paritytech/ink/blob/master/crates/ink/ir/src/ir/config.rs
https://github.com/paritytech/ink/blob/master/crates/ink/ir/src/ir/attrs.rs
(above is not meant to be an exhaustive list)

I will definitely document this for testers and other contributors.

  1. the choice for using rust-analysers parser over ink_IR is well justified (resilient and lossless properties make sense). Does the semantic analyser boil down to generating ink's IR and AST using rust_ap_syntax and rust_ap_ide, instead of syn?

At the IR/AST level, yes. But the public interface has a wider feature set that will resemble (not just simply use) rust_ap_ide, so it's a bit more than just generating an IR/AST. The IR is more like a lower level dependency for the higher level semantic analyzer.

For sanity check, you would want to compare the IR/AST generated by rust_ap_syntax with the IR/AST generated by syn, and it should match for well-formed programs, right?

At a high-level yes, at a lower level the internal architectures for syn and ra_ap_syntax are sufficiently different that it would not be a trivial comparison but certainly doable.

  1. will all of the analysis be performed at the ink-IR level, without unwrapping the macro?

At this stage, Yes. In general, like ink! itself, eDSL validation is happening before/without any macro code generation.


In ink!'s case, all eDSL validation is performed in the ink_ir crate with public interface level type constructors returning Result<T, syn::Error>, and any Rust level errors from syn returned as is.
Errors, if present, are then reported back to the compiler in the ink_macro crate, while a well-formed IR is passed to the ink_codegen crate which generates the final output Rust code.
As an example, you can see this in the implementation of the #[ink::contract] attribute-macro at https://github.com/paritytech/ink/blob/master/crates/ink/macro/src/contract.rs#L21-L29

In the future, with more ambitious goals for the analysis, this may change, but at this stage, it will all be ink IR level analysis.

would the information in ink-AST subsume the information of unwrapped rust AST?

Not 100% sure I follow, but I think this a no, if I understand you correctly. I think my answer above implicitly answers this as well though, otherwise some more context may help get us on the same page 🙂

@bhargavbh
Copy link
Contributor

bhargavbh commented Mar 24, 2023

Hi @davidsemakula thanks for the response!

  1. Documenting the process would be very helpful indeed.
  2. ok, if the architectures and parsed formats of syn and rust_ap_syntax are quite different, then there is no point in matching the two.
  3. most rust static analysis engines ingest MIR-level representations, but this work with ink_ir level might complement it very well, specially for generating provenance and meaningful error messages.
    Cheers, good luck!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review The project is ready to be reviewed by the committee members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants