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

fix: get permissions for the extension for the payment domain instead of motion domain #3365

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

bassgeta
Copy link
Contributor

Description

O N E L I N E R

Basically, the lookup for the voting reputation's permission proofs was done for the motion domain instead of the domain the funds are getting sent from.

Testing

  1. Install the voting reputation extension
  2. Create a simple payment motion sending some CREDS to leela from Andromeda, but create the motion in General (you can note how many CREDS she has)
    image
  3. Fully support it and run npm run forward-time 1 to get to the finalization step
    image
    image
  4. Try finalizing it, it should pass (double check if the CREDS came to leela's wallet
    image
  5. For a sanity check, create a simple payment motion paying some CREDS from Serenity and create the motion in Serenity, support it, forward time, finalize it
    image
    image

Diffs

Changes 🏗

  • use domainId instead of motionId for permission proofs lookup of the voting reputation extension

Resolves #3287

@bassgeta bassgeta self-assigned this Oct 16, 2024
@bassgeta bassgeta requested review from a team as code owners October 16, 2024 14:52
@mmioana mmioana self-requested a review October 17, 2024 12:15
Copy link
Contributor

@mmioana mmioana left a comment

Choose a reason for hiding this comment

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

Lovely what a simple fix this was @bassgeta 🎉

Created a payment motion in General with funds from Andromeda

Screenshot 2024-10-17 at 14 20 12
Screenshot 2024-10-17 at 14 21 13
Screenshot 2024-10-17 at 14 21 34
Screenshot 2024-10-17 at 14 22 01
Screenshot 2024-10-17 at 14 22 18
Screenshot 2024-10-17 at 14 22 21

And then one having Serenity as both the created in domain and from domain
Screenshot 2024-10-17 at 14 23 24
Screenshot 2024-10-17 at 14 23 34
Screenshot 2024-10-17 at 14 23 40

Let's get this fix merged 🦾

Copy link
Member

@rdig rdig left a comment

Choose a reason for hiding this comment

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

Short and sweet, nicely done!

Screenshot from 2024-10-17 15-37-31
Screenshot from 2024-10-17 15-38-18
Screenshot from 2024-10-17 15-39-00
Screenshot from 2024-10-17 15-45-33
Screenshot from 2024-10-17 15-45-50

Copy link
Contributor

@Nortsova Nortsova left a comment

Choose a reason for hiding this comment

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

Great one line solution 🥳

Tested for both Andromeda created in General
Tested for both Serenity created in Serenity

Creds for Leela increase as expected:
image

@bassgeta bassgeta merged commit 4de8673 into master Oct 17, 2024
4 of 5 checks passed
@bassgeta bassgeta deleted the fix/3287-finalize-in-parent-team branch October 17, 2024 13:01
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.

Not able to finalize motion created in a parent team
4 participants