WalletFactory survive upgrades and repair outstanding purses and offers #8773

Chris-Hibbert · 2024-01-18T21:22:38Z

closes: #8488
closes: #8292
closes: #8293
closes: #8286
refs: #8445
refs: #8609
refs: #8445
refs: #8557
refs: a3p#34

Description

Fix the WalletFactory to survive upgrade, and repair all outstanding offers and purses.

Security Considerations

Ensure survival of outstanding SmartWallets.

Scaling Considerations

Some work is done for existing purses and offers when WalletFactory is upgraded to incarnation 2. It's a small amount of work for each, and there aren't many on the chain at this point.

Documentation Considerations

No changes to user-visible behavior are intended.

Testing Considerations

Some unit tests are included. Some tests that the upgrade succeeds in A3P are here. Further tests from #8557 will be added separately. Some manual tests in the A3P environment will be automated and also added separately.

Upgrade Considerations

Oh, yeah. It's all about upgrading.

dckc

more after a short meeting...

dckc · 2024-01-18T23:22:59Z

packages/builders/scripts/smart-wallet/build-wallet-factory2-upgrade.js

@@ -0,0 +1,29 @@
+import { makeHelpers } from '@agoric/deploy-script-support';


I don't see a builders package on the branch that this is going into.
I think this needs to be somewhere with a package.json to work.

It's a duplicate of one in vats anyway. Gone.

dckc · 2024-01-18T23:24:24Z

packages/inter-protocol/test/smartWallet/test-oracle-integration.js

- * @param {*} wallet
+ * @param {import('@agoric/smart-wallet/src/smartWallet.js').SmartWallet} wallet


MH usually avoids changes that have no runtime impact on the release branch. I'm not sure whether that matters here.

not a problem in test files

dckc · 2024-01-18T23:25:25Z

packages/smart-wallet/package.json

@@ -26,6 +27,7 @@
  "dependencies": {
    "@agoric/assert": "^0.6.1-u11wf.0",
    "@agoric/casting": "^0.4.3-u13.0",
+    "@agoric/deploy-script-support": "^0.10.4-u13.0",


I wonder if that could be moved to devDependencies.
not sure it matters, much, though...

Good question, which we can punt on for now.

dckc · 2024-01-18T23:27:37Z

packages/smart-wallet/src/proposals/upgrade-wallet-factory2-proposal.js

+      walletFactoryStartResult,
+      provisionPoolStartResult,
+      chainStorage,
+      walletBridgeManager: walletBridgeManagerP,


👏 This is a pretty clean set of bootstrap powers to be using.

dckc

looks like there's an extra copy of a file. not fatal

mhofman

Some tests that the upgrade succeeds in A3P are here. Further tests from #8557 will be added separately. Some manual tests in the A3P environment will be automated and also added separately.

I am confused. I do not see any a3p-integration changes here, so what exactly is related to A3P? I understand the wallet factory work may not be complete, but from what I understand, the upgrade of the wallet factory vat with the core proposal included in this PR should go through, right? Can we at least add that to a3p-integration in this PR, something along the lines of 65d8c5b

mhofman · 2024-01-19T02:24:05Z

packages/inter-protocol/test/smartWallet/test-oracle-integration.js

- * @param {*} wallet
+ * @param {import('@agoric/smart-wallet/src/smartWallet.js').SmartWallet} wallet


not a problem in test files

mhofman · 2024-01-19T02:24:45Z

packages/smart-wallet/package.json

@@ -26,6 +27,7 @@
  "dependencies": {
    "@agoric/assert": "^0.6.1-u11wf.0",
    "@agoric/casting": "^0.4.3-u13.0",
+    "@agoric/deploy-script-support": "^0.10.4-u13.0",


Good question, which we can punt on for now.

mhofman · 2024-01-19T03:46:24Z

Can we at least add that to a3p-integration in this PR

I have done just that in 016da83, and a crude test seems to pass!

https://github.com/Agoric/agoric-sdk/actions/runs/7578792312/job/20641968845?pr=8773#step:6:1012

https://github.com/Agoric/agoric-sdk/actions/runs/7578792312/job/20641968845?pr=8773#step:7:196

Chris-Hibbert · 2024-01-19T17:30:08Z

Some tests that the upgrade succeeds in A3P are here. Further tests from #8557 will be added separately. Some manual tests in the A3P environment will be automated and also added separately.

I am confused. I do not see any a3p-integration changes here, so what exactly is related to A3P? I understand the wallet factory work may not be complete, but from what I understand, the upgrade of the wallet factory vat with the core proposal included in this PR should go through, right? Can we at least add that to a3p-integration in this PR, something along the lines of 65d8c5b

I ran manuall tests in a3p-integration based on a proposals I wrote myself. I didn't have a version of a3p-integration that added upgrade-14, so it didn't make sense to check them in. Since you have that, it's a good addition. Your simple test is the same thing I used as a first cut. I'm working on converting the manual tests to ones I can script (shell vs. js invocations). My expectation was that they would go in master, but I can bring them here as well once there's a here to target.

mhofman · 2024-01-19T17:33:53Z

I ran manuall tests in a3p-integration based on a proposals I wrote myself. I didn't have a version of a3p-integration that added upgrade-14, so it didn't make sense to check them in. Since you have that, it's a good addition. Your simple test is the same thing I used as a first cut. I'm working on converting the manual tests to ones I can script (shell vs. js invocations). My expectation was that they would go in master, but I can bring them here as well once there's a here to target.

Sorry for the miscommunication, the whole point of the tmp-upgrade-14 was to provide that working a3p-integration environment, supporting core-proposal during upgrade. The same should work in master since it's based off master code I had to cherry-pick. Similarly, I would expect most of this PR to be an adaptation of code in master or about to be in master.

pulled offers.js and payments.js into smartWallet.js as they shared plenty of state that needs to be durable in order to be callable from the watchedPromise. build an upgrade proposal; tested in Agoric/agoric-3-proposals#34

observeNotifier() would handle upgrade disconnects if the notifiers were durable. But in @agoric/ertp, they're ephemeral, so we open-code the loop. When the offerWatchers detect an upgrade, they reschedule the watchers. We needed to ensure the wallet itself doesn't do its cleanup in that case. The test updates purse balance across upgrade - provision a smartWallet for an oracle operator - upgrade zoe; reproduce smartWallet bug - check for new invitation

feat: include invitation purses in repairUnwatchedPurses

covered better by a3p integration tests

Chris-Hibbert · 2024-01-19T19:39:49Z

I would expect most of this PR to be an adaptation of code in master or about to be in master.

#8775 will merge all the new code back to master.

Chris-Hibbert added wallet contract-upgrade next-release about next agoric-sdk or endo release labels Jan 18, 2024

Chris-Hibbert self-assigned this Jan 18, 2024

Chris-Hibbert requested review from turadg, mhofman and dckc January 18, 2024 21:23

Chris-Hibbert force-pushed the 8445-upgradeWF-a3p-test branch from 2bf69c3 to 51b96eb Compare January 18, 2024 23:07

dckc reviewed Jan 18, 2024

View reviewed changes

dckc approved these changes Jan 19, 2024

View reviewed changes

Chris-Hibbert mentioned this pull request Jan 19, 2024

fix: smartWallet watch ERTP purse balances across zoe upgrades #8775

Merged

mhofman reviewed Jan 19, 2024

View reviewed changes

mhofman added the force:integration Force integration tests to run on PR label Jan 19, 2024

mhofman force-pushed the 8618-fakeVirtual-watchedPromises-14 branch from f31ccb0 to c02c973 Compare January 19, 2024 18:30

Base automatically changed from 8618-fakeVirtual-watchedPromises-14 to dev-upgrade-14 January 19, 2024 19:04

turadg and others added 10 commits January 19, 2024 19:18

feat: smartWallet verstion 2 with watchedPromises

24527bb

pulled offers.js and payments.js into smartWallet.js as they shared plenty of state that needs to be durable in order to be callable from the watchedPromise. build an upgrade proposal; tested in Agoric/agoric-3-proposals#34

feat: repairUnwatchedPurses

be1b237

feat: include invitation purses in repairUnwatchedPurses

chore: ignore ts error for type of watchPromise

9ef5861

chore: move test-wallet-upgrade from boot back to vats

d262617

chore: update imports in test-wallet-upgrade

ce2bf57

chore: punt other another wallet test file

5d0194e

covered better by a3p integration tests

chore: repair changed dropped from smartWallet

03ef418

chore: drop duplicate file

dfe4d21

ci: test wallet-factory core proposal

d58948e

mhofman force-pushed the 8445-upgradeWF-a3p-test branch from 016da83 to d58948e Compare January 19, 2024 19:19

This was referenced Jan 19, 2024

fix: watch ERTP purse balances across zoe upgrades (release branch) #8557

Closed

fix: watch ERTP purse balances across zoe upgrades #8573

Closed

Chris-Hibbert merged commit 81d2080 into dev-upgrade-14 Jan 20, 2024
63 checks passed

Chris-Hibbert deleted the 8445-upgradeWF-a3p-test branch January 20, 2024 00:08

This was referenced Jan 22, 2024

Rebase smartWallet (#8488) to release-mainnet1B branch #8609

Closed

test that walletFactory upgrade continue tracking existing offers #8788

Closed

test that smartWallet offer notifications survive Zoe upgrade #8789

Closed

mhofman mentioned this pull request Jan 24, 2024

smartWallet stops updating purse balance on upgrade #8293

Closed

7 tasks

mhofman mentioned this pull request Feb 8, 2024

Upgrade wallet-factory and zoe in handler #8872

Merged

This was referenced Feb 20, 2024

chore(release): publish upgrade-14 #8959

Closed

chore(release): publish upgrade-14 #9003

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WalletFactory survive upgrades and repair outstanding purses and offers #8773

WalletFactory survive upgrades and repair outstanding purses and offers #8773

Chris-Hibbert commented Jan 18, 2024

dckc left a comment

dckc Jan 18, 2024

Chris-Hibbert Jan 19, 2024

dckc Jan 18, 2024

mhofman Jan 19, 2024

dckc Jan 18, 2024

mhofman Jan 19, 2024

dckc Jan 18, 2024

dckc left a comment

mhofman left a comment

mhofman Jan 19, 2024

mhofman Jan 19, 2024

mhofman commented Jan 19, 2024

Chris-Hibbert commented Jan 19, 2024

mhofman commented Jan 19, 2024

Chris-Hibbert commented Jan 19, 2024

		@@ -0,0 +1,29 @@
		import { makeHelpers } from '@agoric/deploy-script-support';

		* @param {*} wallet
		* @param {import('@agoric/smart-wallet/src/smartWallet.js').SmartWallet} wallet

WalletFactory survive upgrades and repair outstanding purses and offers #8773

WalletFactory survive upgrades and repair outstanding purses and offers #8773

Conversation

Chris-Hibbert commented Jan 18, 2024

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

mhofman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhofman commented Jan 19, 2024

Chris-Hibbert commented Jan 19, 2024

mhofman commented Jan 19, 2024

Chris-Hibbert commented Jan 19, 2024