-
Notifications
You must be signed in to change notification settings - Fork 206
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
9303 orchestrate upgrade #9719
Draft
turadg
wants to merge
9
commits into
master
Choose a base branch
from
9303-orchestrate-upgrade
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
9303 orchestrate upgrade #9719
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
ba0e92c
feat(vow): abandoned errors are retriable
mhofman 7bb54b5
WIP feat(vow): retriable tools
mhofman 816952d
WIP fix(orchestration): chainHub accepts zone
mhofman 57d8f9c
WIP Update test snapshots
mhofman 10ddcc6
chore(types): relax RetriableFunc generic
turadg 146884b
fixups
turadg dd61e16
feat(examples): logging progress
turadg 11879f7
test: upgrade from buggy sendAnywhere
turadg 78fb22d
fixup! test: upgrade from buggy sendAnywhere
turadg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
/** @file Bootstrap test of restarting contracts using orchestration */ | ||
import { test as anyTest } from '@agoric/zoe/tools/prepare-test-env-ava.js'; | ||
import { TestFn } from 'ava'; | ||
|
||
import { | ||
makeWalletFactoryContext, | ||
type WalletFactoryTestContext, | ||
} from '../bootstrapTests/walletFactory.js'; | ||
|
||
const test: TestFn<WalletFactoryTestContext> = anyTest; | ||
test.before(async t => { | ||
t.context = await makeWalletFactoryContext( | ||
t, | ||
'@agoric/vm-config/decentral-itest-orchestration-config.json', | ||
); | ||
}); | ||
test.after.always(t => t.context.shutdown?.()); | ||
|
||
/** | ||
* This test core-evals a buggy installation of the sendAnywhere contract by | ||
* giving it a faulty `agoricNames` service with a lookup() function returns a | ||
* promise that never resolves. | ||
* | ||
* Because the send-anywhere flow requires a lookup(), it waits forever. This | ||
* gives us a point at which we can upgrade the vat with a working agoricNames | ||
* and see that the flow continues from that point. | ||
*/ | ||
test('resume', async t => { | ||
const { walletFactoryDriver, buildProposal, evalProposal, storage } = | ||
t.context; | ||
|
||
const { IST } = t.context.agoricNamesRemotes.brand; | ||
|
||
t.log('start sendAnywhere'); | ||
await evalProposal( | ||
buildProposal( | ||
'@agoric/builders/scripts/testing/start-buggy-sendAnywhere.js', | ||
), | ||
); | ||
|
||
t.log('making offer'); | ||
const wallet = await walletFactoryDriver.provideSmartWallet('agoric1test'); | ||
// no money in wallet to actually send | ||
const zero = { brand: IST, value: 0n }; | ||
// send because it won't resolve | ||
await wallet.sendOffer({ | ||
id: 'send-somewhere', | ||
invitationSpec: { | ||
source: 'agoricContract', | ||
instancePath: ['sendAnywhere'], | ||
callPipe: [['makeSendInvitation']], | ||
}, | ||
proposal: { | ||
// @ts-expect-error XXX BoardRemote | ||
give: { Send: zero }, | ||
}, | ||
offerArgs: { destAddr: 'cosmos1whatever', chainName: 'cosmoshub' }, | ||
}); | ||
|
||
// XXX golden test | ||
const getLogged = () => | ||
JSON.parse(storage.data.get('published.sendAnywhere.log')!).values; | ||
|
||
// This log shows the flow started, but didn't get past the name lookup | ||
t.deepEqual(getLogged(), ['sending {0} from cosmoshub to cosmos1whatever']); | ||
|
||
t.log('upgrade sendAnywhere with fix'); | ||
await evalProposal( | ||
buildProposal('@agoric/builders/scripts/testing/fix-buggy-sendAnywhere.js'), | ||
); | ||
|
||
t.deepEqual(getLogged(), [ | ||
'sending {0} from cosmoshub to cosmos1whatever', | ||
// XXX this denom list may be wrong | ||
'got info for denoms: ubld, uist', | ||
'transfer complete, seat exited', | ||
]); | ||
}); |
142 changes: 142 additions & 0 deletions
142
packages/builders/scripts/testing/fix-buggy-sendAnywhere.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
/** | ||
* @file This is for use in tests in a3p-integration | ||
* Unlike most builder scripts, this one includes the proposal exports as well. | ||
*/ | ||
import { | ||
deeplyFulfilledObject, | ||
makeTracer, | ||
NonNullish, | ||
} from '@agoric/internal'; | ||
import { E, Far } from '@endo/far'; | ||
|
||
/// <reference types="@agoric/vats/src/core/types-ambient"/> | ||
/** | ||
* @import {Installation, Instance} from '@agoric/zoe/src/zoeService/utils.js'; | ||
*/ | ||
|
||
const trace = makeTracer('FixBuggySA', true); | ||
|
||
/** | ||
* @import {start as StartFn} from '@agoric/orchestration/src/examples/send-anywhere.contract.js'; | ||
*/ | ||
|
||
/** | ||
* @param {BootstrapPowers & { | ||
* instance: { | ||
* consume: { | ||
* sendAnywhere: Instance<StartFn>; | ||
* }; | ||
* }; | ||
* }} powers | ||
* @param {...any} rest | ||
*/ | ||
export const fixSendAnywhere = async ( | ||
{ | ||
consume: { | ||
agoricNames, | ||
board, | ||
chainStorage, | ||
chainTimerService, | ||
contractKits, | ||
cosmosInterchainService, | ||
localchain, | ||
}, | ||
instance: instances, | ||
}, | ||
{ options: { sendAnywhereRef } }, | ||
) => { | ||
trace(fixSendAnywhere.name); | ||
|
||
const saInstance = await instances.consume.sendAnywhere; | ||
trace('saInstance', saInstance); | ||
const saKit = await E(contractKits).get(saInstance); | ||
|
||
const marshaller = await E(board).getReadonlyMarshaller(); | ||
|
||
// This apparently pointless wrapper is to maintain structural parity | ||
// with the buggy core-eval's wrapper to make lookup() hang. | ||
const agoricNamesResolves = Far('agoricNames that resolves', { | ||
lookup: async (...args) => { | ||
return E(agoricNames).lookup(...args); | ||
}, | ||
}); | ||
|
||
const privateArgs = await deeplyFulfilledObject( | ||
harden({ | ||
agoricNames: agoricNamesResolves, | ||
localchain, | ||
marshaller, | ||
orchestrationService: cosmosInterchainService, | ||
storageNode: E(NonNullish(await chainStorage)).makeChildNode( | ||
'sendAnywhere', | ||
), | ||
timerService: chainTimerService, | ||
}), | ||
); | ||
|
||
trace('upgrading...'); | ||
await E(saKit.adminFacet).upgradeContract( | ||
sendAnywhereRef.bundleID, | ||
privateArgs, | ||
); | ||
|
||
trace('done'); | ||
}; | ||
harden(fixSendAnywhere); | ||
|
||
export const getManifestForValueVow = ({ restoreRef }, { sendAnywhereRef }) => { | ||
console.log('sendAnywhereRef', sendAnywhereRef); | ||
return { | ||
manifest: { | ||
[fixSendAnywhere.name]: { | ||
consume: { | ||
agoricNames: true, | ||
board: true, | ||
chainStorage: true, | ||
chainTimerService: true, | ||
cosmosInterchainService: true, | ||
localchain: true, | ||
|
||
contractKits: true, | ||
}, | ||
installation: { | ||
consume: { sendAnywhere: true }, | ||
}, | ||
instance: { | ||
consume: { sendAnywhere: true }, | ||
}, | ||
}, | ||
}, | ||
installations: { | ||
sendAnywhere: restoreRef(sendAnywhereRef), | ||
}, | ||
options: { | ||
sendAnywhereRef, | ||
}, | ||
}; | ||
}; | ||
|
||
/** @type {import('@agoric/deploy-script-support/src/externalTypes.js').CoreEvalBuilder} */ | ||
export const defaultProposalBuilder = async ({ publishRef, install }) => | ||
harden({ | ||
// Somewhat unorthodox, source the exports from this builder module | ||
sourceSpec: '@agoric/builders/scripts/testing/fix-buggy-sendAnywhere.js', | ||
getManifestCall: [ | ||
'getManifestForValueVow', | ||
{ | ||
sendAnywhereRef: publishRef( | ||
install( | ||
'@agoric/orchestration/src/examples/send-anywhere.contract.js', | ||
), | ||
), | ||
}, | ||
], | ||
}); | ||
|
||
export default async (homeP, endowments) => { | ||
// import dynamically so the module can work in CoreEval environment | ||
const dspModule = await import('@agoric/deploy-script-support'); | ||
const { makeHelpers } = dspModule; | ||
const { writeCoreEval } = await makeHelpers(homeP, endowments); | ||
await writeCoreEval(fixSendAnywhere.name, defaultProposalBuilder); | ||
}; |
140 changes: 140 additions & 0 deletions
140
packages/builders/scripts/testing/start-buggy-sendAnywhere.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
/** | ||
* @file This is for use in tests in a3p-integration | ||
* Unlike most builder scripts, this one includes the proposal exports as well. | ||
*/ | ||
import { | ||
deeplyFulfilledObject, | ||
makeTracer, | ||
NonNullish, | ||
} from '@agoric/internal'; | ||
import { E, Far } from '@endo/far'; | ||
|
||
/// <reference types="@agoric/vats/src/core/types-ambient"/> | ||
/** | ||
* @import {Installation} from '@agoric/zoe/src/zoeService/utils.js'; | ||
*/ | ||
|
||
const trace = makeTracer('StartBuggySA', true); | ||
|
||
/** | ||
* @import {start as StartFn} from '@agoric/orchestration/src/examples/send-anywhere.contract.js'; | ||
*/ | ||
|
||
/** | ||
* @param {BootstrapPowers & { | ||
* installation: { | ||
* consume: { | ||
* sendAnywhere: Installation<StartFn>; | ||
* }; | ||
* }; | ||
* }} powers | ||
*/ | ||
export const startSendAnywhere = async ({ | ||
consume: { | ||
agoricNames, | ||
board, | ||
chainStorage, | ||
chainTimerService, | ||
cosmosInterchainService, | ||
localchain, | ||
startUpgradable, | ||
}, | ||
installation: { | ||
consume: { sendAnywhere }, | ||
}, | ||
instance: { | ||
// @ts-expect-error unknown instance | ||
produce: { sendAnywhere: produceInstance }, | ||
}, | ||
}) => { | ||
trace(startSendAnywhere.name); | ||
|
||
const marshaller = await E(board).getReadonlyMarshaller(); | ||
|
||
const privateArgs = await deeplyFulfilledObject( | ||
harden({ | ||
agoricNames, | ||
localchain, | ||
marshaller, | ||
orchestrationService: cosmosInterchainService, | ||
storageNode: E(NonNullish(await chainStorage)).makeChildNode( | ||
'sendAnywhere', | ||
), | ||
timerService: chainTimerService, | ||
}), | ||
); | ||
|
||
const agoricNamesHangs = Far('agoricNames that hangs', { | ||
lookup: async () => { | ||
trace('agoricNames.lookup being called that will never resolve'); | ||
// BUG: this never resolves | ||
return new Promise(() => {}); | ||
}, | ||
}); | ||
|
||
const { instance } = await E(startUpgradable)({ | ||
label: 'sendAnywhere', | ||
installation: sendAnywhere, | ||
privateArgs: { | ||
...privateArgs, | ||
agoricNames: agoricNamesHangs, | ||
}, | ||
}); | ||
produceInstance.resolve(instance); | ||
trace('done'); | ||
}; | ||
harden(startSendAnywhere); | ||
|
||
export const getManifestForValueVow = ({ restoreRef }, { sendAnywhereRef }) => { | ||
trace('sendAnywhereRef', sendAnywhereRef); | ||
return { | ||
manifest: { | ||
[startSendAnywhere.name]: { | ||
consume: { | ||
agoricNames: true, | ||
board: true, | ||
chainStorage: true, | ||
chainTimerService: true, | ||
cosmosInterchainService: true, | ||
localchain: true, | ||
|
||
startUpgradable: true, | ||
}, | ||
installation: { | ||
consume: { sendAnywhere: true }, | ||
}, | ||
instance: { | ||
produce: { sendAnywhere: true }, | ||
}, | ||
}, | ||
}, | ||
installations: { | ||
sendAnywhere: restoreRef(sendAnywhereRef), | ||
}, | ||
}; | ||
}; | ||
|
||
/** @type {import('@agoric/deploy-script-support/src/externalTypes.js').CoreEvalBuilder} */ | ||
export const defaultProposalBuilder = async ({ publishRef, install }) => | ||
harden({ | ||
// Somewhat unorthodox, source the exports from this builder module | ||
sourceSpec: '@agoric/builders/scripts/testing/start-buggy-sendAnywhere.js', | ||
getManifestCall: [ | ||
'getManifestForValueVow', | ||
{ | ||
sendAnywhereRef: publishRef( | ||
install( | ||
'@agoric/orchestration/src/examples/send-anywhere.contract.js', | ||
), | ||
), | ||
}, | ||
], | ||
}); | ||
|
||
export default async (homeP, endowments) => { | ||
// import dynamically so the module can work in CoreEval environment | ||
const dspModule = await import('@agoric/deploy-script-support'); | ||
const { makeHelpers } = dspModule; | ||
const { writeCoreEval } = await makeHelpers(homeP, endowments); | ||
await writeCoreEval(startSendAnywhere.name, defaultProposalBuilder); | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a comment of what exactly will hang in the test? I tracked down that
chainHub
usesagoricNames
withretriable
, so I assume something in the test will trigger usage of chainHub and its retriable vow to not be resolved before upgrade, but I lost track of how exactly that happens.Also we should provide a little more details as to why this is a valid test (and equivalent to the previous one, which had the pending promise explicitly created in the vat being upgraded). Here the buggy
agoricNames
is external to the vat being upgraded, which means upgrading the vat will not reject the result promise for theE(agoricNames).lookup()
call. However that call is simply awaited in aretriable
async function, which means there is a local promise created and watched (the result of the async function), which is pending at vat upgrade, and which will be rejected on upgrade.Interestingly, the lookup result promise will remain pending forever, causing a "leak" of the promise subscription (in the real world until the decider of the promise upgrades) even though the subscriber is actually no longer interested in the promise. This is a liveslots "limitation" that I created an issue about (#10101)