Upgrading scaled price authority causes vaults upgrade to not complete #9584
Assignees
Labels
bug
Something isn't working
Comments
mergify bot
added a commit
that referenced
this issue
Jun 26, 2024
refs: #9382 refs: #9584 ## Description Add a test that was supposed to be in #9283, where it says > A3P tests that verify that vaultFactory has been upgraded, that a new Auctioneer is running and is receiving prices. Verify that when prices drop, assets are sold via the auction, the bidder gets the proceeds, and the vaults are liquidated or reconstituted appropriately. It was too hard to verify the results of the auction, because of the timing of vault liquidations and auction runs, so the actual check was dropped. The subsequent PR (#9371) that upgraded scaledPriceAuthorities seems to have broken the upgrade, and the missing test failed to warn us. Here we test that vaultFactory was actually upgraded by verifying that it's getting prices from the new price feeds, and drop the upgrade of scaledPriceAuthority until we can figure out how to make that upgrade compatible. ### Security Considerations Not relevant ### Scaling Considerations Drops the upgrade of scaledPriceAuthority, which fixed part of the memory growth. This was the smaller portion of the growth, so it's more important to get the rest of the fixes in than to also include this. ### Documentation Considerations None. ### Testing Considerations Replaces a missing test. ### Upgrade Considerations Repairs upgrade.
mhofman
pushed a commit
that referenced
this issue
Jun 26, 2024
refs: #9382 refs: #9584 ## Description Add a test that was supposed to be in #9283, where it says > A3P tests that verify that vaultFactory has been upgraded, that a new Auctioneer is running and is receiving prices. Verify that when prices drop, assets are sold via the auction, the bidder gets the proceeds, and the vaults are liquidated or reconstituted appropriately. It was too hard to verify the results of the auction, because of the timing of vault liquidations and auction runs, so the actual check was dropped. The subsequent PR (#9371) that upgraded scaledPriceAuthorities seems to have broken the upgrade, and the missing test failed to warn us. Here we test that vaultFactory was actually upgraded by verifying that it's getting prices from the new price feeds, and drop the upgrade of scaledPriceAuthority until we can figure out how to make that upgrade compatible. ### Security Considerations Not relevant ### Scaling Considerations Drops the upgrade of scaledPriceAuthority, which fixed part of the memory growth. This was the smaller portion of the growth, so it's more important to get the rest of the fixes in than to also include this. ### Documentation Considerations None. ### Testing Considerations Replaces a missing test. ### Upgrade Considerations Repairs upgrade.
mergify bot
added a commit
that referenced
this issue
Oct 9, 2024
closes: #9584 closes: #9928 refs: #9827 refs: #9748 refs: #9382 closes: #10031 ## Description We added upgrading the scaledPriceAuthority to the steps in upgrading vaults, auctions, and priceFeeds, and didn't notice that it broke things. The problem turned out to be that the "priceAuthority" object registered with the priceFeedRegistry was an ephemeral object that was not upgraded. This fixes that by re-registering the new priceAuthority. Then, to simplify the process of cleaning up the uncollected cycles reported in #9483, we switched to replacing the scaledPriceAuthorities rather than upgrading them. We also realized that we would need different coreEvals in different environments, since the Oracle addresses and particular assets vary for each (test and mainNet) chain environment. #9748 addressed some of the issues in the original coreEval. #9999 showed what was needed for upgrading priceFeeds, which was completed by #9827. #10021 added some details on replacing scaledPriceAuthorities. ### Security Considerations N/A ### Scaling Considerations Addresses one of our biggest scaling issues. ### Documentation Considerations N/A ### Testing Considerations Thorough testing in a3p, and our testnets. #9886 discusses some testing to ensure Oracles will work with the upgrade. ### Upgrade Considerations See above
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
The vaults upgrade is a 2 step process where first new price feeds and auction contracts are installed, then the vault factory is upgraded only after a price quote on a new feed was received.
For some reason upgrading the scaled price authority before vaults factory completes its upgrade ends up failing the vaults upgrade.
Furthermore integration tests of vaults upgrade did not fail when this happened.
To Reproduce
run a3p tests on master
Expected behavior
No error, and an integration test catching vaults upgrade failures
Platform Environment
N/A
Additional context
Add any other context about the problem here.
Screenshots
If applicable, add screenshots to help explain your problem, especially for UI interactions.
Tasks
The text was updated successfully, but these errors were encountered: