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

6007 test pause PSM #6047

Merged
merged 12 commits into from
Aug 27, 2022
Merged

6007 test pause PSM #6047

merged 12 commits into from
Aug 27, 2022

Conversation

Chris-Hibbert
Copy link
Contributor

refs: #6007

Description

A test showing that governance can set offer filters for the PSM and prevent trading

Changes to contractHelper to get the parameters right in non- and virtual- governorFacets.

Security Considerations

minimal

Documentation Considerations

None

Testing Considerations

This is a mainly test.

@Chris-Hibbert Chris-Hibbert added test Inter-protocol Overarching Inter Protocol pso labels Aug 24, 2022
@Chris-Hibbert Chris-Hibbert added this to the Mainnet 1 RC0 milestone Aug 24, 2022
@Chris-Hibbert Chris-Hibbert self-assigned this Aug 24, 2022
@Chris-Hibbert Chris-Hibbert requested a review from turadg as a code owner August 24, 2022 21:14
@Chris-Hibbert Chris-Hibbert marked this pull request as draft August 24, 2022 21:14
@Chris-Hibbert Chris-Hibbert changed the base branch from master to 5796-governPause August 24, 2022 21:15
).getAmountOf(poserInvitationP);

/** @type {import('@agoric/vats/tools/storage-test-utils.js').MockChainStorageRoot} */
// @ts-expect-error cast
Copy link
Member

Choose a reason for hiding this comment

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

you could get this directly from setupPsmBootstrap where it has the type MockChainStorageRoot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand what you're suggesting.

is it a better source for the type info, or calling makeMockChainStorageRoot() here rather than in setupPsmBootstrap?

Copy link
Member

Choose a reason for hiding this comment

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

in short, better source for the type info. If you exported the value from setupPsmBootstrap then the type would come along with it and you wouldn't need the annotation/cast here.

I think you could call makeMockChainStorageRoot() again here, but I don't see a benefit. It's important that each test have its own chain storage, but each test is calling setupPsm and each of those is calling setupPsmBootstrap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

* @param {TimerService} timer
* @param {FarZoeKit} [farZoeKit]
*/
export const setupPsmBootstrap = async (
Copy link
Member

Choose a reason for hiding this comment

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

this isn't specific to PSM. I wonder if we can DRY it out with other tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was copied from AMM, and and apparently didn't require any changes. I don't see other contracts that do the same thing, and I don't know how to generalize what's happening. I'd prefer to do the cleanup as a separate task.

@@ -0,0 +1,62 @@
// @ts-check

// eslint-disable-next-line import/no-extraneous-dependencies
Copy link
Member

Choose a reason for hiding this comment

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

is this suppression necessary? @agoric/zoe is declared as a dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy-pasta

@Tartuffo Tartuffo removed this from the Mainnet 1 RC0 milestone Aug 24, 2022
@Chris-Hibbert Chris-Hibbert mentioned this pull request Aug 24, 2022
@Chris-Hibbert Chris-Hibbert force-pushed the 6007-testPausePsm branch 2 times, most recently from 94325c2 to 6dd4e21 Compare August 25, 2022 18:03
@Chris-Hibbert Chris-Hibbert marked this pull request as ready for review August 26, 2022 20:06
@Chris-Hibbert Chris-Hibbert added the automerge:squash Automatically squash merge label Aug 26, 2022
Base automatically changed from 5796-governPause to master August 26, 2022 20:52
@mergify mergify bot merged commit 8d5bd3f into master Aug 27, 2022
@mergify mergify bot deleted the 6007-testPausePsm branch August 27, 2022 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge Inter-protocol Overarching Inter Protocol test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants