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

Apply governor mutation based on existing datum value #238

Merged
merged 9 commits into from
Apr 24, 2023

Conversation

danielfarrelly
Copy link
Contributor

@danielfarrelly danielfarrelly commented Mar 29, 2023

Describe your changes

The governor mutation effect as written currently can not be run due to its dependence on the out ref of the governor. The proposal creation step consumes the pinned out ref, preventing it from being consumed later as a part of the effect's execution. This change instead pins the existing datum value of the governor, causing validation of the effect to fail if the value does not match.

Relevant issues

Checklist before requesting a review.

  • I have ensured documentation and testing are thorough.
  • I have updated the changelog.
  • I have read CONTRIBUTING.md
  • I have made sure the CI checks run using nix run .#ci.
  • I have followed the code standards to the best of my ability or have documented carefully where and why I haven't.

Copy link
Contributor

@chfanghr chfanghr left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

@nini-faroux
Copy link
Contributor

So I added that small change - tested it out with the off-chain and it's working fine it seems.

@emiflake emiflake force-pushed the df/governor-mutation-fix branch from 3ebc4ad to 7c475a4 Compare April 24, 2023 14:45
@emiflake emiflake enabled auto-merge April 24, 2023 14:48
@emiflake emiflake merged commit 939e8b8 into staging Apr 24, 2023
@danielfarrelly danielfarrelly deleted the df/governor-mutation-fix branch April 24, 2023 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants