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

5894 durable smartwallet #6871

Merged
merged 7 commits into from
Feb 1, 2023
Merged

5894 durable smartwallet #6871

merged 7 commits into from
Feb 1, 2023

Conversation

turadg
Copy link
Member

@turadg turadg commented Jan 27, 2023

closes: #5894

Description

Makes the walletFactory contract durable. Upgrade deferred to #6878

In support, it moves some /inter-protocol helpers down to /zoe and adds a new provideStoredSubscriber. The latter will be replaced soon by TopicMeta tools (à la #6888 ) so this provider is designed to been drop-in replaced.

It also has a drive-by fix:

  • makeEphemeraProvider had a strong map for its ephemeras

In this PR I had also included d072f7f, in the spirit of POLA, but it complicated diagnostic logging so I backed it out. Looking for feedback on whether it's worth doing (and if so I'll do in a separate PR.) @dckc may have thoughts on the ocap discipline.

Security Considerations

No new assumptions. Improves POLA.

Scaling Considerations

Small additional RAM for the StoredSubscriber provider.

Documentation Considerations

--

Testing Considerations

CI and agops-oracle-smoketest.

@turadg turadg force-pushed the 5894-durable-smartwallet branch from 1692979 to f50c65a Compare January 28, 2023 01:36
@turadg turadg requested a review from Chris-Hibbert January 28, 2023 01:37
@turadg turadg force-pushed the 5894-durable-smartwallet branch 2 times, most recently from 42c9f2d to bee26f9 Compare January 28, 2023 02:25
Copy link
Contributor

@Chris-Hibbert Chris-Hibbert left a comment

Choose a reason for hiding this comment

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

One question about defineDurableExoClassKit vs. prepareExoClassKit.

LGTM

packages/smart-wallet/src/smartWallet.js Outdated Show resolved Hide resolved
@turadg turadg requested a review from Chris-Hibbert January 31, 2023 18:48
@turadg turadg marked this pull request as ready for review January 31, 2023 18:48
@turadg turadg force-pushed the 5894-durable-smartwallet branch from 8d0d2b0 to d072f7f Compare January 31, 2023 18:58
@turadg turadg force-pushed the 5894-durable-smartwallet branch from 45ada35 to 6977f73 Compare February 1, 2023 00:10
@turadg turadg added the automerge:no-update (expert!) Automatically merge without updates label Feb 1, 2023

/** @type {PublishKit<UpdateRecord>} */
const updatePublishKit = makeWalletPublishKit();
// NB: state size must not grow monotonically
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what "must not grow monotically" means. (I realize you moved it.) Do you think it's trying to say that it "must grow sub-linearly"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wrote the moved string. I mean it must not continue to grow; it must also drop information that's no longer useful.

If you propose a better way to say that I can drop it into another PR

Copy link
Contributor

Choose a reason for hiding this comment

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

"state size must not grow without bound" or "state must be limited in size". Not sure which you mean.

"must not grow monotonically" sounds to me like it can reach whatever size it wants, as long as it occasionally pares things back somewhat.

@turadg turadg mentioned this pull request Feb 1, 2023
@mergify mergify bot merged commit f8ac9b5 into master Feb 1, 2023
@mergify mergify bot deleted the 5894-durable-smartwallet branch February 1, 2023 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:no-update (expert!) Automatically merge without updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make Smart Wallet contract durable
2 participants