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

repair ZCF to unblock contract upgrade #9254

Merged
merged 2 commits into from
Apr 18, 2024
Merged

Conversation

Chris-Hibbert
Copy link
Contributor

closes: #9246

Description

#8911 fixes an issue with ZCF on the master branch. That issue effectively blocks contract upgrade. This ports that fix to the release branch, and adds a proposal that tells Zoe to use the new ZCF.

Security Considerations

Zoe and ZCF and the ability to upgrade contracts are crucial.

Scaling Considerations

N/A

Documentation Considerations

N/A

Testing Considerations

After installing the fix, we will test contract upgrades in a3p and the dev chains. The fix itself is tested in #8911.

Upgrade Considerations

It's all about upgrade.

dckc
dckc previously approved these changes Apr 18, 2024
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

This looks like what we discussed.

@gibson042 , I presume it's OK to duplicate rather than cherry-pick 73e0bb8 from #9250

@dckc dckc dismissed their stale review April 18, 2024 18:08

need to double-check target branch

@dckc
Copy link
Member

dckc commented Apr 18, 2024

Should the target branch be dev-upgrade-15?

Copy link
Member

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

The changes from app.go are missing. The 2 commits from #9250 should be a simple cherry pick, with the one changing app.go requiring minor conflict resolution

@Chris-Hibbert
Copy link
Contributor Author

The changes from app.go are missing. The 2 commits from #9250 should be a simple cherry pick, with the one changing app.go requiring minor conflict resolution

attempting to cherry-pick gets

CONFLICT (file location): packages/builders/scripts/vats/upgrade-zcf.js added in 73e0bb830 (feat: add upgrade zcf only proposal) inside a directory that was renamed in HEAD, suggesting it should perhaps be moved to packages/vats/scripts/upgrade-zcf.js.
error: could not apply 73e0bb830... feat: add upgrade zcf only proposal

How would you recommend I copy that change and maintain the history? (I presume that's why a cherry pick would be preferred to a new commit)

@mhofman
Copy link
Member

mhofman commented Apr 18, 2024

A cherry-pick does not keep the history any more than a new commit. I forgot about the move. I had split the commit to make the cherry pick easier, but that didn't help. You could indicate in the commit message what original commit was picked, but that only helps humans.

Copy link

cloudflare-pages bot commented Apr 18, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7debf1d
Status:🚫  Build failed.

View logs

@Chris-Hibbert
Copy link
Contributor Author

Should the target branch be dev-upgrade-15?

Presumably that or something related. I'm waiting for a signal from @gibson042 that it's ready, at which point, I'll rebase this to it.

@gibson042
Copy link
Member

You could indicate in the commit message what original commit was picked, but that only helps humans.

But that help is valuable! A reference to 72c7574 may very well reduce future release work.

@Chris-Hibbert
Copy link
Contributor Author

A reference to 72c7574 may very well reduce future release work.

I think the important part of that is to note that the change to Zoe in that commit hasn't been merged to the chain yet. It should be included before the next time we're going to upgrade Zoe, or the next time we're going to upgrade multiple contracts.

Base automatically changed from gibson-init-upgrade-15 to dev-upgrade-15 April 18, 2024 21:21
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

I defer to RG / MH

@mhofman mhofman added the force:integration Force integration tests to run on PR label Apr 18, 2024
@Chris-Hibbert Chris-Hibbert merged commit 74662d7 into dev-upgrade-15 Apr 18, 2024
71 of 73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contract-upgrade force:integration Force integration tests to run on PR Zoe package: Zoe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants