-
Notifications
You must be signed in to change notification settings - Fork 215
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(run-protocol): complete permits for startRewardDistributor, makeAnchorAsset #5213
Conversation
- needs zoe, installation.mintHolder
Thanks for verifying that it works, @arirubinstein . @michaelfig , @turadg , I'm still interested in feedback on the testing considerations above. |
Do we have a ticket for cleanups we've identified during this bootstrap? I would non-blocking request a) such a ticket and b) that testing this be included. |
@turadg and I have plans to work toward such a test on Thu. |
well, we have 1 now: #5217 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
closes: ##5212
Description / Security Considerations
When we wired up runStake to rewards, we neglected to update the permit of startRewardDistributor to give it access to the runStake creator facet.
Another incomplete permit that came up in testing ( #5062 (comment) ) was
makeAnchorAsset
. Since it usesmintHolder
for the anchor asset, so it needs access themintHolder
installation and tozoe
to start a copy of it.Testing Considerations
These permits used to be tested by
vats/test/test-boot.js
but since we moved them topackages/run-protocol
, they are no longer covered by tests that run in ci.While these fixes have been tested manually ( #5062 (comment) ) ideally, this PR would include a regression test that runs in ci. Reviewers will please help judge whether it's expedient to proceed without such a test.