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

makeContractGovernorKit is missing input validation #10879

Open
gibson042 opened this issue Jan 23, 2025 · 0 comments
Open

makeContractGovernorKit is missing input validation #10879

gibson042 opened this issue Jan 23, 2025 · 0 comments
Labels
bug Something isn't working

Comments

@gibson042
Copy link
Member

Describe the bug

makeContractGovernorKit is defined with ContractGovernorKitI, which requires that facet creator method getCreatorFacet returns a remotable, and likewise for methods getAdminFacet and getPublicFacet. But its initializer just accepts (startedInstanceKit, limitedCreatorFacet) and populates state like { ...startedInstanceKit, limitedCreatorFacet, … }, which means that the values those methods attempt to return might not actually be remotables. The interface guards will produce errors in that case, but there is no indication of a problem until that time, which is potentially far removed from the problem with initial creation.

To Reproduce

This problem already exists within our tests:

  1. governedContract.js returns makeGovernorFacet({}, { governanceApi }) as its creatorFacet.
  2. makeGovernorFacet(originalCreatorFacet, governedApis = {}) wraps makeFarGovernorFacet to return a "governorFacet" Far object whose getLimitedCreatorFacet method returns the input originalCreatorFacet, which for this test is an empty CopyRecord.
  3. contractGovernor.js starts the governed contract (which for this test is governedContract.js) to get startedInstanceKit, calls getLimitedCreatorFacet() on startedInstanceKit.creatorFacet to get a limitedCreatorFacet (which for this test is that empty CopyRecord), and passes both on to makeContractGovernorKit(startedInstanceKit, limitedCreatorFacet).
  4. The tests that start governed instances of governedContract.js don't call getCreatorFacet(), but any attempt to do so would result in an error.

Expected behavior

Because makeContractGovernorKit interface guards constrain methods that return values from original input, those values should be validated at creation time in the makeContractGovernorKit initializer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant