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

misc fastUsdc vstorage cleanup #10661

Merged
merged 10 commits into from
Dec 10, 2024
Merged

misc fastUsdc vstorage cleanup #10661

merged 10 commits into from
Dec 10, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Dec 10, 2024

refs: #10633

Description

Miscellaneous small fixes, docs and refactorings. These are made in the course of #10633 but that has other changes that need more work.

Best reviewed by commit.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

CI

Upgrade Considerations

not yet deployed

@turadg turadg requested a review from a team as a code owner December 10, 2024 01:22
Copy link

cloudflare-workers-and-pages bot commented Dec 10, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: f77b0ec
Status: ✅  Deploy successful!
Preview URL: https://15286fd7.agoric-sdk.pages.dev
Branch Preview URL: https://10578-vstorage-misc.agoric-sdk.pages.dev

View logs

@@ -95,8 +95,8 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
marshaller,
);

const makeStatusNode = () => E(storageNode).makeChildNode(STATUS_NODE);
const statusManager = prepareStatusManager(zone, makeStatusNode);
const statusNode = E(storageNode).makeChildNode(STATUS_NODE);
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 confused about line 178: "Define all kinds above this line. Keep remote calls below."

Seems like this would go against that because it's a remote call. Does it matter? The makeStatusNode was already kind of a gray area in that it wasn't called immediately.

Copy link
Member Author

Choose a reason for hiding this comment

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

That comment really meant "keep remote call awaits below". We'll need to improve the comment and patterns for contract developers about that, but it's out of scope for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks for clarifying.

@turadg turadg requested a review from samsiegart December 10, 2024 17:57
@@ -95,8 +95,8 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
marshaller,
);

const makeStatusNode = () => E(storageNode).makeChildNode(STATUS_NODE);
const statusManager = prepareStatusManager(zone, makeStatusNode);
const statusNode = E(storageNode).makeChildNode(STATUS_NODE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks for clarifying.

@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Dec 10, 2024
@turadg turadg force-pushed the 10578-vstorage-misc branch from 90f1a97 to f77b0ec Compare December 10, 2024 18:36
@mergify mergify bot merged commit 1231911 into master Dec 10, 2024
81 checks passed
@mergify mergify bot deleted the 10578-vstorage-misc branch December 10, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants