chore(acceptance-price-feeds): make sure old vats are quiescent and vaults receive quotes

[anilhelvaci]

closes: https://github.com/Agoric/BytePitchPartnerEng/issues/25
refs: #9928

Description

#10074 replaced existing priceFeeds and scaledPriceAuthorities with new ones that should be more efficient in terms of space usage. However, terminating those old vats is a problem that hasn't been solved as described in #9483. So it is important to make sure those old vats are actually quiescent. We also need to make sure we are able to produce quotes successfully. In this PR we are addressing these concerns.

Security Considerations

None.

Scaling Considerations

None.

Documentation Considerations

None.

Testing Considerations

Regarding observing vat details there are 2 assumptions we are relying on;

• dynamic vat ids are created incrementally (newly created vats have a higher vat id)
• number of deliveries to a vat caused by a gc sweep do not exceed 5

#9928 talks about making sure auctions receive quotes. That is no tested here due to the high fidelity of testing auctions. The auctions features are tested #10229 so I believe there will be comprehensive coverage when it is merged.

Upgrade Considerations

None.

Help from reviewers

#10074 introduced new price feeds to the system. However, f:replace-price-feeds does not activate oracles for future layers of the build. Meaning, proposals running after f:replace-price-feeds will not have oracles that received invitationMakers for new price feeds and there will not be quotes published by new price feeds. There are conflicting work to fix this issue, see;

• chore(a3p): setup the oracle and push price for ATOM and stATOM at use phase of f:replace-price-feeds #10296
• Turn priceFeed proposal into a SoftwareUpgrade #10317
• chore(a3p): setup the oracle and push price for ATOM and stATOM at use phase of f:replace-price-feeds #10296 (review)

As a work around I've implemented an init method that runs during test.before to set the stage before we start testing new price feeds. I could use some guidance from you guys how to proceed from here. One option I can think of is to have init work conditionally.

• If oracles are registered to new price feeds, do not register them
• If they did not, register them

Please let me know what you think @dckc @turadg @Chris-Hibbert

[cloudflare-workers-and-pages]

Deploying agoric-sdk with    Cloudflare Pages

Latest commit:	cbee1a5
Status:	✅  Deploy successful!
Preview URL:	https://9268afdd.agoric-sdk.pages.dev
Branch Preview URL:	https://anil-price-feed-tests.agoric-sdk.pages.dev

View logs

[anilhelvaci]

bytepitch#4 (comment)

Hey @Chris-Hibbert , so sorry about tagging you in the wrong PR. I was trying something out in the CI over there and forgot to remove the tag. I’ll answer above comment here.

Basically I wasn’t aware of #9595. I think we need a way to improve how we can avoid duplicate work. I’m open to suggestions @LuqiPan @otoole-brendan .

Assuming that this PR doesn’t add anything on top of #9595 in terms of measuring quiescent vats, what do you think can be tested in z:acceptance about replacing price feeds?

[LuqiPan]

From my quick eyeballing of this PR and #9595, it seems to me that there's still value in this PR as this PR provides tests that can be run via CI automatically. Did I misunderstand the difference?

[anilhelvaci]

From my quick eyeballing of this PR and #9595, it seems to me that there's still value in this PR as this PR provides tests that can be run via CI automatically. Did I misunderstand the difference?

No, you didn't misunderstand anything. In #9595, I don't see how the measurement happens or which vats active/inactive. The difference is this PR does not measure if abandoned vats keeps growing in terms of space usage(I believe #9595
does this). In this PR we check if SwingSet delivers any messages to abandoned vats right after we push prices to whatever is in agoricNames and assume this quiescent state will keep space used by those abandoned vats stable(meaning they will not grow).

Measuring the activity of abandoned vats is the only thing I can think of related to price feed functionality that are not already tested in auction and vaults tests. Having said that, when #8928 lands (have no idea when that will happen) and takes effect (the termination will proceed slowly), measuring abandoned vat activity will become dysfunctional and we'll have to update z:acceptance accordingly.

Above is the variables and where we've ended up on reflecting replace-price-feeds proposal into z:acceptance. Need to make a decision about how to proceed. @LuqiPan

[Chris-Hibbert]

Measuring the activity of abandoned vats is the only thing I can think of related to price feed functionality that are not already tested in auction and vaults tests. Having said that, when #8928 lands (have no idea when that will happen) and takes effect (the termination will proceed slowly), measuring abandoned vat activity will become dysfunctional and we'll have to update z:acceptance accordingly.

Above is the variables and where we've ended up on reflecting replace-price-feeds proposal into z:acceptance. Need to make a decision about how to proceed. @LuqiPan

I can see that this is an interesting second approach to ensuring that the abandoned vats are indeed quiescent, but it doesn't seem useful as a regression test to be kept around until we kill those vats. The interesting question about continuing activity on-chain has to do with the way the Oracles behave and how the chain responds, and this doesn't help with that question.

My suggestion would be to say it was a helpful observation, but I don't think there's net positive value in merging it.