Skip to content

Pallet Supersig - Milestone 2#719

Merged
Noc2 merged 5 commits intow3f:masterfrom
decentration:master
Feb 7, 2023
Merged

Pallet Supersig - Milestone 2#719
Noc2 merged 5 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

@0xCaso 0xCaso self-assigned this Feb 3, 2023
@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 3, 2023

Hi @decentration, thanks a lot for the delivery. I started the evaluation, I'll post my notes the next week!

@decentration
Copy link
Copy Markdown
Contributor Author

hey @0xCaso awesome, 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.

Here I am, I leave you the evaluation here. Let me know if you have any questions!

@decentration
Copy link
Copy Markdown
Contributor Author

Here I am, I leave you the evaluation here. Let me know if you have any questions!

Fantastic, let me go through your points... : )

@decentration
Copy link
Copy Markdown
Contributor Author

Great feedback @0xCaso thank you.

0c

  • Medium ✅ ah yes, sorry if that slowed you down, the pesky Medium platform is prone to mess up the formatting, but that's fixed now.
  • RPC tests, great catch will add those in for Milestone 3 (feedback and improvements milestone), which will follow this review as its 95% done as well.
  • Testing leaveSupersig and deleteSupersig:

leaveSupersig is very simple, if you are a member you just select your account, go to supersig > leaveSupersig, and select the supersig account you would like to leave and that should be good.

deleteSupersig, needs to be a proposal, so you need to wrap a deleteSupersig call within a submitCall function:

  • first select your account, then go to supersig > submitCall,
  • then select the supersig you would like to propose to delete,
  • then you go to select supersig > deleteSupersig,
  • then you need to vote for it by going to supersig > approveCall (do not wrap it in a submitCall). and make sure >51% of members vote for it, so that the call will be executed.

Substrate module: pallet_supersig

The functionalities should work, I just noted one thing: if I try to perform a tx with a Supersig through a member, and it's not funded, after the proposal passes the tx fails, but there is no way to see the failure. This because the last voting, which makes the proposal pass, gets through (and it's visible from the UI), but then the tx itself fails and the UI won't show it (neither in the Network page). Is there a way to let this see to the user?

Yes, this is a well considered scenario, it is true that failed calls do not show an error or log an event that shows that the vote passed but the call itself was not a success. This will also be added in for Milestone 3. Hopefully you can review m3 too, because we've added some very nice and usable and visual interactions for voting.

Let me know if you have any other queries, if not we can wrap this up and I will submit m3 in the coming days.

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 a lot for the answers @decentration!

I managed to test the two functions, for some reason leaveSupersig didn't work yesterday but I think (i've read now the error) it was because the only remaining member was leaving it (in that case, one needs to use deleteSupersig). Maybe you could add a note in the docs for the next milestone to clarify this, but it's pretty logic!

For the other adjustments thanks! And yes, I can definitely evaluate also the next milestone, so I can see the progress :)

@0xCaso 0xCaso mentioned this pull request Feb 7, 2023
@Noc2 Noc2 merged commit b3cd420 into w3f:master Feb 7, 2023
@decentration
Copy link
Copy Markdown
Contributor Author

Fab thanks @0xCaso, I will indeed update the supersig wiki with that detail. And have just changed the error message from InvalidNumberOfMembers, to MustHaveAtLeastOneMemberToDelete, which will be available on the next release, once we deal with some bug fixes brought on by polkadot-v0.9.37. All the best 👍

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 7, 2023

@decentration Great, looking forward!

May you add the VAT amount in the invoice and resend it please?

@decentration
Copy link
Copy Markdown
Contributor Author

@0xCaso Oh, the company is registered in a no VAT jurisdiction, which is why it’s not on the invoice.

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 7, 2023

@0xCaso Oh, the company is registered in a no VAT jurisdiction, which is why it’s not on the invoice.

Yes sorry, I was meaning to put the VAT amount to 0 ;)

@decentration
Copy link
Copy Markdown
Contributor Author

That is resubmitted now @0xCaso

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 7, 2023

That is resubmitted now @0xCaso

Perfect thanks, I forwarded it internally. The payment should take place within 14 days!

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 7, 2023

Sorry @decentration, in the details there is written "Milestone 3 .... ", but actually it's for Milestone 2. Could you change that field (I'm sorry to make you resend it, didn't notice it)

@decentration
Copy link
Copy Markdown
Contributor Author

3rd time’s a charm @0xCaso

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 7, 2023

I think there is still written "Milestone 3 - Supersig UI completion" instead of "Milestone 2 - Supersig UI completion" 😬

@decentration
Copy link
Copy Markdown
Contributor Author

decentration commented Feb 7, 2023

Oh my goodness! Sorry about that. Ok this one should be correct. Thanks for your patience @0xCaso

@0xCaso
Copy link
Copy Markdown
Contributor

0xCaso commented Feb 7, 2023

No worries and thanks! This time it should be perfect

@RouvenP
Copy link
Copy Markdown

RouvenP commented Feb 10, 2023

hi @decentration we transferred the payment today.

@decentration
Copy link
Copy Markdown
Contributor Author

Hey @RouvenP, received and thank you.

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