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

feat(cosmos): upgrade provisioning vat #8901

Merged
merged 3 commits into from
Feb 17, 2024
Merged

Conversation

mhofman
Copy link
Member

@mhofman mhofman commented Feb 13, 2024

refs: #8821 #8311 #8786
closes: #8113

Stacked on #8902

Description

Core proposal to upgrade the provisioning vat (follow-up from #8821) and a3p integration test to verify namesByAddress works as expected (adapted from #8786)

Security Considerations

Same as #8821

Scaling Considerations

None

Documentation Considerations

None

Testing Considerations

Adds a3p integration test

Upgrade Considerations

Core proposal to be executed during chain software upgrade. Tested in a3p. Has equivalent PR on the release branch.

@mhofman mhofman changed the title Mhofman/provisioning upgrade Provisioning core proposal and namesByAddress a3p test Feb 13, 2024
@mhofman mhofman added the force:integration Force integration tests to run on PR label Feb 13, 2024
@mhofman mhofman changed the base branch from master to mhofman/8887-fix-governance February 13, 2024 01:29
@mhofman
Copy link
Member Author

mhofman commented Feb 13, 2024

FYI @dckc

Base automatically changed from mhofman/8887-fix-governance to master February 13, 2024 04:29
@mhofman mhofman marked this pull request as ready for review February 13, 2024 22:47
@mhofman mhofman requested a review from dckc February 13, 2024 22:47
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.

looks like this closes #8113!

@@ -0,0 +1,53 @@
// @ts-check
Copy link
Member

Choose a reason for hiding this comment

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

hm. another copy of this file? we can't get the relevant stuff from the synthetic chain package?

Copy link
Member Author

Choose a reason for hiding this comment

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

If there's a way, I'm open to it. I just extracted what you had in #8786. I'm open to alternatives!

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 think I found a way. PTAL

Comment on lines +852 to +853
// Then, upgrade the provisioning vat
vm.CoreProposalStepForModules("@agoric/builders/scripts/vats/replace-provisioning.js"),
Copy link
Member

Choose a reason for hiding this comment

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

this is more than a test... ah... the commit message says feat. The PR title threw me off a bit.

@@ -335,6 +335,16 @@ __metadata:
languageName: node
linkType: hard

"brace-expansion@npm:^1.1.7":
Copy link
Member

Choose a reason for hiding this comment

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

all these changes to yarn.lock come from adding tmp?

I guess this is not our main yarn.lock, so there's not much supply-chain risks.

Copy link
Member Author

Choose a reason for hiding this comment

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

removed

@mhofman
Copy link
Member Author

mhofman commented Feb 13, 2024

I will likely extract the core-eval-support part into a separate stand alone PR so that #8909 can stand on it too.

@mhofman mhofman changed the title Provisioning core proposal and namesByAddress a3p test feat(cosmos): upgrade provisioning vat Feb 14, 2024
@mhofman mhofman requested a review from dckc February 14, 2024 02:08
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.

even nicer

@mhofman
Copy link
Member Author

mhofman commented Feb 14, 2024

Ran into a synthetic-chain bug: Agoric/agoric-3-proposals#104

@mhofman mhofman added force:integration Force integration tests to run on PR and removed force:integration Force integration tests to run on PR labels Feb 16, 2024
@mhofman mhofman added automerge:squash Automatically squash merge and removed force:integration Force integration tests to run on PR labels Feb 16, 2024
@mhofman mhofman force-pushed the mhofman/provisioning-upgrade branch 2 times, most recently from eba0d8b to 6316acf Compare February 17, 2024 00:28
@mhofman mhofman removed the automerge:squash Automatically squash merge label Feb 17, 2024
@mhofman mhofman added the force:integration Force integration tests to run on PR label Feb 17, 2024
@mhofman mhofman force-pushed the mhofman/provisioning-upgrade branch 4 times, most recently from 84e7127 to 9b526d3 Compare February 17, 2024 03:15
@mhofman mhofman added automerge:squash Automatically squash merge and removed force:integration Force integration tests to run on PR labels Feb 17, 2024
@mergify mergify bot merged commit 174e37d into master Feb 17, 2024
64 checks passed
@mergify mergify bot deleted the mhofman/provisioning-upgrade branch February 17, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

can't send to account by address: namesByAddress is empty
2 participants