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: upgrade v7-board and test it #10516

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: upgrade v7-board and test it #10516

wants to merge 1 commit into from

Conversation

Chris-Hibbert
Copy link
Contributor

closes: #10394

Description

Upgrade v7-board.

Security Considerations

The Board is a critical vat.

Scaling Considerations

v7-Board is consuming more resources than it should. restarting it should clean up a fair amount of space. ...checking

Documentation Considerations

None

Testing Considerations

Verify that the board doesn't lose anything.

Upgrade Considerations

Can go out in Upgrade 19.

@Chris-Hibbert Chris-Hibbert added contract-upgrade force:integration Force integration tests to run on PR labels Nov 18, 2024
@Chris-Hibbert Chris-Hibbert self-assigned this Nov 18, 2024
@Chris-Hibbert Chris-Hibbert requested a review from a team as a code owner November 18, 2024 19:16
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.

It seems "restarting it should clean up a fair amount of space." is not mentioned in the test plan of #10394

Would you please clarify that this is on purpose? Do we have other plans (mainfork?) for testing that?


const { adminNode } = await E(vatStore).get('board');

await E(adminNode).upgrade(boardBundleCap, {});
Copy link
Member

Choose a reason for hiding this comment

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

I suggest logging completion of upgradeBoard.

@@ -0,0 +1,35 @@
import { E, Far } from '@endo/far';
import { Stable } from '@agoric/internal/src/tokens.js';

Copy link
Member

Choose a reason for hiding this comment

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

How about testing that an inventory with getIDs() looks the same before and after the upgrade?

@Chris-Hibbert
Copy link
Contributor Author

Would you please clarify that this is on purpose? Do we have other plans (mainfork?) for testing that?

Sorry, I wrote "restarting it should clean up a fair amount of space. ...checking", meaning that I intend to see if approaches like I used in #9595 would show some improvement. I'll do that next.

@Chris-Hibbert
Copy link
Contributor Author

Well I get a count of 422 before in a3p-integration, and 432 after, using a query like this (with vatID = 7).

    sql.get([`SELECT COUNT(*) from kvStore WHERE key LIKE '${vatId}.c.%'`]);

I don't know what objects are soaking up space on mainnet, but they're not in evidence here.

@mhofman @warner Is there anything else worth checking in a3p-integration?

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contract Upgrade: v7-board
2 participants