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.

Sign up for GitHub

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

Jump to bottom

feat: make software upgrade coreProposals conditional on upgrade plan name #9575

Merged
merged 6 commits into from
Jun 26, 2024
Merged

feat: make software upgrade coreProposals conditional on upgrade plan name #9575

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
95174a2
feat(builders): non-ambient `strictPriceFeedProposalBuilder` in `pric…
michaelfig Jun 24, 2024
5cc190d
feat(app): establish mechanism for adding core proposals by `upgradeP…
michaelfig Jun 24, 2024
52f02b7
fix(builders): use proper `oracleBrand` subkey case
michaelfig Jun 25, 2024
ddc072d
chore(cosmos): extract `app/upgrade.go`
michaelfig Jun 25, 2024
b3182a4
chore: fix error handling of upgrade vaults proposal
mhofman Jun 25, 2024
ea568a2
fix: retry upgrade vaults price quote
mhofman Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions packages/inter-protocol/src/proposals/upgrade-vaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { E } from '@endo/far';
import { makeNotifierFromAsyncIterable } from '@agoric/notifier';
import { AmountMath } from '@agoric/ertp/src/index.js';
import { makeTracer } from '@agoric/internal/src/index.js';
import { isUpgradeDisconnection } from '@agoric/internal/src/upgrade-api.js';

const trace = makeTracer('upgrade Vaults proposal');

Expand Down Expand Up @@ -150,9 +151,28 @@ export const upgradeVaults = async (powers, { options }) => {
// Wait for at least one new price feed to be ready before upgrading Vaults
void E.when(
any(
Object.values(vaultBrands).map(brand =>
E(priceAuthority).quoteGiven(AmountMath.make(brand, 10n), istBrand),
),
Object.values(vaultBrands).map(async brand => {
const getQuote = async lastRejectionReason => {
await null;
try {
return await E(priceAuthority).quoteGiven(
AmountMath.make(brand, 10n),
istBrand,
);
} catch (reason) {
if (
isUpgradeDisconnection(reason) &&
(!lastRejectionReason ||
reason.incarnationNumber >
lastRejectionReason.incarnationNumber)
) {
return getQuote(reason);
}
throw reason;
}
};
return getQuote(null);
}),
Comment on lines +155 to +175
Copy link
Collaborator

@anilhelvaci anilhelvaci Jul 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What is the reason for waiting for a quote when upgrading vaultFactory? @mhofman @Chris-Hibbert

My guess is that in vaultDirector's makePriceLockWaker;

agoric-sdk/packages/inter-protocol/src/vaultFactory/vaultDirector.js

Lines 447 to 450 in ac88da5

makePriceLockWaker() {
return makeWaker('priceLockWaker', () => {
allManagersDo(vm => vm.lockOraclePrices());
});

allManagersDo iterates over all vault managers to invoke their lockOraclePrices sequentially. If there's a case such as no price updates are pushed for a certain vault manager when priceLockWaker is awake, then lockOraclePrices for that certain vault manager will fail. Which will result with vault managers added later than the failing one will not be able to lock their prices because allManagersDo invokes lockOraclePrices sequentially, see:

agoric-sdk/packages/inter-protocol/src/vaultFactory/vaultDirector.js

Lines 267 to 273 in ac88da5

const allManagersDo = fn => {
for (const managerIndex of collateralManagers.values()) {
const vm = vaultManagers.get(managerIndex).self;
fn(vm);
}
};

I have already opened an issue about this, please see #9706. In the issue I have a possible fix which worked for me quite nicely.

However, I am very curios to find out what other reasons there may be for waiting a price feed update before upgrading vaultFactory?

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason for waiting for a quote when upgrading vaultFactory? @mhofman @Chris-Hibbert

When we run the coreEval to do the upgrade, we'll be upgrading vaultFactory, and replacing auctions, as well as replacing all the priceAuthorities. After replacing the priceAuthorities, the oracle operators have to manually intervene in order to accept new invitations, and do some reconfiguration in order to start feeding information to the new priceAuthorities. I don't have any indication of whether that will take a half hour or a couple of days.

During the interim, it's best that the vaults and auctions continue to listen to the old priceAuthorities. The indication that the switchover is complete is that any price starts coming via the new priceAuthorities. At that point, we upgrade the vaultFactory, which will then start paying attention to the new priceAuthorities.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks Chris!

),
async price => {
trace(`upgrading after delay`, price);
Expand Down
Loading