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

fix: When setting a new zcfBundleCap, store it in baggage #8806

Merged
merged 3 commits into from
Jan 29, 2024

Conversation

Chris-Hibbert
Copy link
Contributor

@Chris-Hibbert Chris-Hibbert commented Jan 24, 2024

closes: #8805

Description

While reviewing a PR to push code to a release branch, @erights noticed that we don't save new zcfBundleCaps persistently.

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

None.

Testing Considerations

As we're testing zoe upgrades in a3p, we'll verify that the newly set value survives zoe upgrade.

Upgrade Considerations

It's all about upgrade.

This will be immediately copied to a release staging branch to be included when we upgrade Zoe.

@Chris-Hibbert Chris-Hibbert added bug Something isn't working Zoe package: Zoe labels Jan 24, 2024
@Chris-Hibbert Chris-Hibbert requested a review from erights January 24, 2024 01:30
@Chris-Hibbert Chris-Hibbert self-assigned this Jan 24, 2024
Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

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

I notice that saveBundleCap does an init. Thus, it should be called at most once. (Given its purpose, more like exactly once.) Skimming the code around this change, I was not able to convince myself it could not be called multiple times. I don't understand the interlock

  const setVatAdminService = Far('setVatAdminService', lateVatAdminSvc => {
    vatAdminSvc = lateVatAdminSvc;
    saveBundleCap();
  });

  if (vatAdminSvc) {
    saveBundleCap();
  } else if (zoeBaggage.has('zcfBundleCap')) {
    zcfBundleCap = zoeBaggage.get('zcfBundleCap');
    // in this case, it'll be known to have been resolved, but that's fine
    vatAdminSvc = zoeBaggage.get('vatAdminSvc');
  }

@Chris-Hibbert
Copy link
Contributor Author

I was not able to convince myself it could not be called multiple times.

I changed it check if init has already been called, and call set() in that case.

Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Chris-Hibbert Chris-Hibbert added the automerge:squash Automatically squash merge label Jan 29, 2024
@mergify mergify bot merged commit 1f4333e into master Jan 29, 2024
66 checks passed
@mergify mergify bot deleted the 8805-zoeSaveZcfBundleCap branch January 29, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge bug Something isn't working Zoe package: Zoe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

when updateZcfBundleId() is called, save it in baggage
2 participants