feat: add the ability to override param values #9829
Conversation
Chris-Hibbert
added
Governance
Chris-Hibbert
force-pushed
the
9596-vaultDirParams
branch
from
6aa71aa to
e0a0cdc
Compare
Deploying agoric-sdk with
|
| Latest commit: |
ef2963a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://99b9e963.agoric-sdk.pages.dev |
| Branch Preview URL: | https://9596-vaultdirparams.agoric-sdk.pages.dev |
Chris-Hibbert
force-pushed
the
9584-upgradeVaults
branch
from
fc11081 to
0442ba7
Compare
There was a problem hiding this comment.
This says "closes: #9596"; the test plan there says "Test if a3p-integration. Verify that the values are restored." I don't see that here. Did that turn out to be prohibitively expensive?
Also, I need to know more about (what looks like) busy-waiting before approving.
| while (AmountMath.isEmpty(value.current.MinInitialDebt.value)) { | ||
| ({ value, updateCount } = await notifier.getUpdateSince(updateCount)); | ||
| trace( | ||
| `minInitialDebt was empty, retried`, | ||
| value.current.MinInitialDebt.value, | ||
| ); | ||
| } |
There was a problem hiding this comment.
this looks busy-waiting. it that on purpose? add a comment about why that's ok?
There was a problem hiding this comment.
It's not quite busy waiting. Each time through, it asks for the next update (getUpdateSince(updateCount)). The expectation is that the zero'th might be empty, but the successive update should have a value. It might be better to back off just once and then give up, rather than waiting in a loop.
| @@ -85,6 +85,32 @@ export const upgradeVaults = async (powers, { options }) => { | |||
| } | |||
| } | |||
|
|
|||
| const restoreDirectorParams = async () => { | |||
There was a problem hiding this comment.
This seems to get/query the params, not restore them. Or am I mis-reading it?
There was a problem hiding this comment.
You are correct. I'll rename this to readCurrentDirectorParams. Restoring them happens because they're included in newPrivateArgs, which is passed to upgradeContract.
I did test manually once by changing the |
Chris-Hibbert
added
the
automerge:rebase
Chris-Hibbert
force-pushed
the
9596-vaultDirParams
branch
from
8539ce5 to
0d8b1c7
Compare
|
@Mergifyio refresh |
✅ Pull request refreshed |
frazarshad
added
automerge:rebase
When upgrading a contract, some parameter values originally defined based on terms must be overridden by values supplied via privateArgs. This provides a path to supply those overrides when creating the new paramManager. The vaultFactory uses this new feature to pass values through. The proposal to upgrade vaultFactory gets the current values as updated by governance and passes them in when upgrading.
Chris-Hibbert
force-pushed
the
9596-vaultDirParams
branch
from
0d8b1c7 to
ef2963a
Compare
Chris-Hibbert
added
the
automerge:rebase
|
@Mergifyio refresh |
✅ Pull request refreshed |
closes: #9596
refs: #9748
Description
When upgrading VaultFactory, restore the VaultDirector's parameters to the values that exist on chain.
Security Considerations
No particular security implications.
Scaling Considerations
Run only on upgrade. No impact.
Documentation Considerations
No user visible impact.
Testing Considerations
being tested manually thoroughly. The change to
typeParamManager.jshas a unit test.Upgrade Considerations
Make it possible to preserve values that have been updated on chain. It would have been bad to lose the updated value of
ReferencedUI.