pallet-verifier (milestone 1 & 2)

[davidsemakula]

Milestone Delivery Checklist

• The milestone-delivery-template.md has been copied and updated.
• This pull request is being made by the same account as the accepted application.
• I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
• In case of acceptance, invoices must be submitted and payments will be transferred to the Polkadot AssetHub and/or fiat account provided in the application.
• The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: w3f/Grants-Program#2234 < please fill this in with the PR number of your application.

[PieWol]

Hey @davidsemakula ,
thanks for submitting your delivery. I'd like to ask you to either submit the deliveries for individual milestones separately or point out which work has been done to achieve each milestone. In it's current format it's hard to differentiate the work that went towards each milestone. This would make it easier for me to review. Best ✌️

[davidsemakula]

Hi @PieWol

I'd like to ask you to either submit the deliveries for individual milestones separately or point out which work has been done to achieve each milestone.

I'll do the latter and ping you. 🙂

[PieWol]

Great, thank you :)

[davidsemakula]

Hi @PieWol

point out which work has been done to achieve each milestone.

I've added the details to the delivery document.

[PieWol]

Great, thanks. I'll let you know once I made progress on the review.

[davidsemakula]

@PieWol I pushed a few updates, please pull the latest from master before reviewing 🙂

[PieWol]

Thanks for the update @davidsemakula :)

[PieWol]

Hey @davidsemakula ,
sadly when I try to follow the readme and install pallet-verifier I run into the issue that it's using the authenticated ssh request for getting additional code from github. What do you think about switching this to the http option that doesn't need authentication?

See

  Installing pallet-verifier v0.1.0 (/home/ubuntu/pallet-verifier)
    Updating crates.io index
    Updating git submodule `git@github.com:microsoft/vcpkg.git`
git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
error: failed to get `mirai` as a dependency of package `pallet-verifier v0.1.0 (/home/ubuntu/pallet-verifier)`

Caused by:
  failed to load source for dependency `mirai`

Caused by:
  Unable to update https://github.com/davidsemakula/MIRAI.git?branch=pallet-verifier-2024-02-08#7f2d3cee

Caused by:
  failed to update submodule `vcpkg`

Caused by:
  failed to fetch submodule `vcpkg` from git@github.com:microsoft/vcpkg.git

Caused by:
  process didn't exit successfully: `git fetch --tags --force --update-head-ok 'git@github.com:microsoft/vcpkg.git' '+refs/heads/*:refs/remotes/origin/*' '+HEAD:refs/remotes/origin/HEAD'` (exit status: 128)```

[davidsemakula]

Hi @PieWol
Technically, the vcpkg git submodule affected by this is from upstream MIRAI, so before I make a change that I will eventually have to upstream (I've already started upstreaming a few other things, so that's not a issue), can you try to comment out this line in .cargo/config.toml and LMK if you still get an error?

[davidsemakula]

Pinging @PieWol 🙂

[davidsemakula]

@PieWol The suggested cargo config update appears to fix the issue in my testing, so I've pushed an update to master with the fix applied (along with a few other general improvements).

[PieWol]

Thanks, I'll update you later today.

[PieWol]

@davidsemakula
I think the issue lies in this file. The url should be using the https url. Would you mind changing this? ^^
Thanks.

[davidsemakula]

@PieWol should be fixed, please pull latest Cargo.lock from master

[PieWol]

Hey @davidsemakula ,
everything looks good so far and I'm expecting to have both milestones evaluated by Monday if nothing goes wrong :) I'm just doing some additional testing now.

I have put up a draft pr already for the milestone 1 evaluation.

Thanks

[PieWol]

Hey @davidsemakula ,
when I try to run cargo verify-pallet on this pallet then I get this error:

error: Failed to generate tractable entry points for any dispatchable function in public interface
  |
  = help: Add some unit tests for dispatchable functions.

error: could not compile `pallet-proofs-dealer` (lib) due to 1 previous error

Whats the reason for this error? Is it because the testing functions have different names than the actual calls of the pallet? 🤔

[davidsemakula]

@PieWol Checking it out ... the names of the tests shouldn't matter though 🙂

[davidsemakula]

Hi @PieWol

The issue was this pallet has a cyclic transitive dependency on itself in test mode (via some of its dev dependencies - see also). So our custom rustc driver was being prematurely invoked on this cyclic dependency (instead of the "primary" package - for the compiler, these are different artifacts because of different compiler flags, configs, features et.c). So no unit tests where being found because the test harness (and hence the tests module in this case) is NOT built/compiled for dependencies (only for the "primary" package).

I've updated the cargo subcommand to detect these cases and only invoke our custom rustc driver for the right artifact, so it should work fine now (i.e. after pulling and installing latest changes from master).

Thanks for finding/reporting the issue 🙂

[PieWol]

Hey @davidsemakula ,
great job figuring this out and fixing it so quickly.

[github-actions]

🪙 Please fill out the invoice form in order to initiate the payment process. Thank you!

[PieWol]

Congratulations on finishing this grant 🎉 . Nice work!
I really hope this will be maintained and refined so it becomes a must-have tool for pallet development 🙏

[davidsemakula]

Thanks for the swift and thorough review @PieWol , and for the positive feedback as well!

I really hope this will be maintained and refined so it becomes a must-have tool for pallet development 🙏

This is 100% the plan 🙂

[davidsemakula]

I've also submitted the invoices 🙂