Skip to content

supersig m3 completion#761

Merged
Noc2 merged 2 commits intow3f:masterfrom
decentration:master
Feb 28, 2023
Merged

supersig m3 completion#761
Noc2 merged 2 commits intow3f:masterfrom
decentration:master

Conversation

@decentration
Copy link
Copy Markdown
Contributor

Milestone Delivery Checklist

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

@decentration
Copy link
Copy Markdown
Contributor Author

@0xCaso Hey Matteo, if you're available, M3 is ready for your perusal! Some nice improvements have been made which will greatly improve user experience.

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 23, 2023

Hi @decentration great, and thanks for the delivery! Actually I have a bit of backlog, but for sure I can work on it early next week.

@0xCaso 0xCaso self-assigned this Feb 23, 2023
@decentration
Copy link
Copy Markdown
Contributor Author

sounds good @0xCaso look forward!

Copy link
Copy Markdown
Contributor

@0xCaso 0xCaso left a comment

Choose a reason for hiding this comment

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

I started the evaluation :)
In the meantime, could you fix this minor thing in the delivery doc?

Copy link
Copy Markdown
Contributor

@0xCaso 0xCaso left a comment

Choose a reason for hiding this comment

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

I'm testing the pallet, and I followed the tutorial article and the loom video (which was really clear, thanks!).

Btw, I'm trying to execute proposals that will success (transfer x < supersig balance) and not (>).

In every case, I got the same output, but I've seen in the video that actually you changed this (you can see the screenshots below: proposal 0 was successful, proposal 2 wasn't).

Screenshot 2023-02-27 at 11 33 54

Screenshot 2023-02-27 at 11 33 33

I'm using substrate-supersig-template (branch v0.9.28-new) and apps (branch page-supersig-apps-new).

I've just noticed that the specified branch of substrate-supersig-template has not been updated recently. Could this be the issue?

@decentration
Copy link
Copy Markdown
Contributor Author

decentration commented Feb 27, 2023

hey @0xCaso, great stuff i'm glad it was clear!

Re: your above issue:

I've just noticed that the specified branch of substrate-supersig-template has not been updated recently.

This shouldn't be an issue because the changes are in the pallet-supersig and apps. However make sure to cargo update before recompiling, so that it pulls in the changes from pallet-supersig.

Having said that, if you didn't recompile, and the transaction was not a success, then you would see nothing instead of seeing Ok.... Ok comes from CallExecutionAttempted(T::AccountId, CallId, DispatchResultWithPostInfo), from the pallet. It's very unlikely that if the transaction was not a success that it would show Ok because this is a primitive from substrate's FRAME.

There is a chance that the balance you've sent is like 500 / 1_000_000_000_000 instead of 500. I think this is the most likely issue.

Are you using the docker for apps? There is a chance i may have not pushed the newest version which may have fixed the decimals issue i just mentioned. (Update I can see i pushed to docker hub 5 days ago, so its the recent version).

p.s. if you could also share the proposal details that would help understand the issue. But to conclude its very unlikely it would output a false Ok.

@decentration
Copy link
Copy Markdown
Contributor Author

Screenshot 2023-02-27 at 11 22 17

I just tried to send 5000 from SUPERSIG which only has a balance of 4500, and got the above error. And i am very sure all this is the same version, because i don't think changes have been made recently to affect the decimal issue.

@decentration
Copy link
Copy Markdown
Contributor Author

decentration commented Feb 27, 2023

Screenshot 2023-02-27 at 11 25 42

@0xCaso Yes it looks like you are on an older version of pallet-supersig.

Screenshot 2023-02-27 at 11 26 25

The result should display the above whereas your result only shows <...SpRuntimeDispactchError>.

So yes you will need to cargo update substrate-supersig-template, which will pull the updates from the pallet. Sorry that was confusing i should have mentioned, given its on the same release branch!

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 27, 2023

Thanks for the fast response! Yes, the problem was that I didn't do cargo update before building. Now I can see the new messages.

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 27, 2023

Also, I'm trying to run pallet_supersig tests but I have the following 5 errors, do you have idea of the reason?

error[E0308]: mismatched types
   --> src/tests/approve_call.rs:113:8
    |
113 |                 Ok(Ok(()))
    |                 -- ^^^^^^ expected struct `hidden_include::dispatch::PostDispatchInfo`, found enum `Result`
    |                 |
    |                 arguments to this enum variant are incorrect
    |
    = note: expected struct `hidden_include::dispatch::PostDispatchInfo`
                 found enum `Result<(), _>`
note: tuple variant defined here
   --> /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/result.rs:508:5

error[E0308]: mismatched types
   --> src/tests/approve_call.rs:181:8
    |
181 |                 Ok(Ok(()))
    |                 -- ^^^^^^ expected struct `hidden_include::dispatch::PostDispatchInfo`, found enum `Result`
    |                 |
    |                 arguments to this enum variant are incorrect
    |
    = note: expected struct `hidden_include::dispatch::PostDispatchInfo`
                 found enum `Result<(), _>`
note: tuple variant defined here
   --> /rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/result.rs:508:5

error[E0599]: no variant or associated item named `InvalidNumberOfMembers` found for enum `pallet::Error` in the current scope
   --> src/tests/create_supersig.rs:136:19
    |
136 |             Error::<Test>::InvalidNumberOfMembers
    |                            ^^^^^^^^^^^^^^^^^^^^^^ variant or associated item not found in `pallet::Error<mock::Test>`
    |
   ::: src/lib.rs:202:5
    |
202 |     pub enum Error<T> {
    |     ----------------- variant or associated item `InvalidNumberOfMembers` not found for this enum

error[E0599]: no variant or associated item named `InvalidNumberOfMembers` found for enum `pallet::Error` in the current scope
   --> src/tests/leave_supersig.rs:93:19
    |
93  |             Error::<Test>::InvalidNumberOfMembers
    |                            ^^^^^^^^^^^^^^^^^^^^^^ variant or associated item not found in `pallet::Error<mock::Test>`
    |
   ::: src/lib.rs:202:5
    |
202 |     pub enum Error<T> {
    |     ----------------- variant or associated item `InvalidNumberOfMembers` not found for this enum

error[E0599]: no variant or associated item named `InvalidNumberOfMembers` found for enum `pallet::Error` in the current scope
   --> src/tests/remove_members.rs:112:19
    |
112 |             Error::<Test>::InvalidNumberOfMembers
    |                            ^^^^^^^^^^^^^^^^^^^^^^ variant or associated item not found in `pallet::Error<mock::Test>`
    |
   ::: src/lib.rs:202:5
    |
202 |     pub enum Error<T> {
    |     ----------------- variant or associated item `InvalidNumberOfMembers` not found for this enum

@decentration
Copy link
Copy Markdown
Contributor Author

@0xCaso ah yes, the tests were updated in the later version (0.9.37), i've now pulled them into 0.9.28, just fetch and git pull and try again, and you should have working tests.

Copy link
Copy Markdown
Contributor

@0xCaso 0xCaso left a comment

Choose a reason for hiding this comment

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

Thanks again, I posted my evaluation here. Basically I just mention to fix a little thing on the Medium guide.

@decentration
Copy link
Copy Markdown
Contributor Author

Great stuff, i hope it wasn't too painful of a review for you @0xCaso. The changes have been made with the correct branch name.

Copy link
Copy Markdown
Contributor

@0xCaso 0xCaso left a comment

Choose a reason for hiding this comment

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

Thanks! And no, it was straightforward ;)

@0xCaso 0xCaso mentioned this pull request Feb 28, 2023
@Noc2 Noc2 merged commit db412b1 into w3f:master Feb 28, 2023
@github-actions
Copy link
Copy Markdown

We noticed that this is the last milestone of your project. Congratulations on completing your grant!

So, where to from here? The main goal of the W3F grants program is to support research as well as early-stage technical projects. So, if your project still falls under one of those categories, you might want to apply for a follow-up grant. Depending on your goals and project status, there are other support programs in our ecosystem that might be better suited as the next step, for example:

Project with a Business Case/Token: Substrate Builders Program or VC Funding

Common Good Projects: Treasury Funding

For a more comprehensive list, see our Alternative Funding page.
Let us know if you have any questions regarding the above. We are more than happy to point you to additional resources and help you determine the best course of action.

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 28, 2023

Thanks again @decentration for the contribution and congrats! I've forwarded internally the invoice, the payment should happen within 14 days.

@decentration
Copy link
Copy Markdown
Contributor Author

Thank you @0xCaso pleasure to interact with you : )

@RouvenP
Copy link
Copy Markdown

RouvenP commented Mar 3, 2023

hi @decentration we transferred the payment today

@decentration
Copy link
Copy Markdown
Contributor Author

hi @decentration we transferred the payment today

Hey Rouven, thanks and received 👍

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