remove reallocate from ZCF

[Chris-Hibbert]

What is the Problem Being Solved?

#6577 Added a new reallocation API for Zoe, but didn't convert all the uses in existing contracts to the new version.

Description of the Design

#7136 convered all the uses of reallocate, incrementBy, and decrementBy in agoric-sdk, except for the one in atomicRearrange and tests.

Left to do is :

• remove from dapps
• inform third-parties that the old version is going away
• move atomicRearrange onto ZCF
• remove realllocate from ZCF

Security Considerations

The new API is more robust. Version migration has to be done carefully.

Test Plan

This ends by deleting the old APIs, which is sufficient test that the methods are gone.

[dckc]

How is this a contract-api-change?

Oh... I guess the label wasn't clear. It doesn't mean changes to things like the Zoe API. It means changes to the external API of contracts such that clients (such as web UIs) are affected.

I updated the description.

[turadg]

I think Mergify got confused by "This doesn't close #6678" in the PR.

[erights]

I think Mergify got confused by "This doesn't close #6678" in the PR.

I've had this problem several times. I hate "guess my syntax" user interfaces. Do we know what the actual syntax is for a PR to say that it would close an Issue, so that we can reliably avoid saying that when it is not what we want to say?

[turadg]

I was wrong about Mergify. GitHub attributes the action to the account that merged the PR. The magic syntax is https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

[erights]

Thanks, that explains it. The cases I recently ran into were at #5903 (comment)

due to the text

Would enable us to fix #5903

Apparently, the use of the word "fix" followed by the bug number was an adequate signal.

[Chris-Hibbert]

Next steps:

• move atomicRearrange inside Zoe
• deprecate pervasively: reallocate, getStagedAllocation, and hasStagedAllocation.
• remove uses of those methods from our code.
• wait
• remove reallocate, et. al.

The first three can be in a single release, or successive releases. There should be at least a short period between those and removing the older approach.

[dckc]

update docs too

[Chris-Hibbert]

update docs too

Already in progress.

[Chris-Hibbert]

@turadg,
The next step in this process involves making a change to ZCF, to add atomicRearrange, and a bunch of tooling to actually make it possible to upgrade zoe and zcf to install those changes on chain. The first part of that is #7900, which makes the change to ZCF. That is accompanied by #7901, #7903, and #7904, which upgrade three of the core contracts to make use of that change. None of those are urgent, so we'll say no more about them at this point.

In order to install the change to ZCF on chain, we have to modify Zoe to support a mutable version of ZCF (#7946), and then we have to provide scripts that can be invoked by core-eval (#7966). This is complicated by the fact that 7966 will be merged into a state of the tree that already includes the new Zoe & ZCF, so its accompanying test doesn't actually verify that an upgrade from the status quo ante to the goal state will succeed.

In order to validate that, we have a separate PR (#7969). It includes a copy of the same set of tooling, and installs bundles built by 7966 representing the target state of Zoe & ZCF. The test that validates the upgrade has some minor validations between 7966 and 7969, since the latter installs pre-built bundles (and the test results are expected to be slightly different.)

My expectation is that that we'll now proceed to review and merge #7900, #7946, and #7966.

The test in #7969 is intended to be convincing validation that the code in 7900 and 7946 will succeed when they are applied on-chain. A follow-up task is to build an upgrade test based on 7969 that applies the upgrade in the docker test environment.

After that, the remaining steps of urging developers to migrate and removing support tor zcf.reallocate() and stagings can be taken after a short pause.

[Chris-Hibbert]

After #7900, #7946, and #7966, the following work will remain:

• Actually merge the changes to the chain. I presume the main prerequisite for that is
  • Build a docker test showing that Zoe and ZCF can be upgraded like test: a test framework for verifying upgrade of Zoe and ZCF #7966. (Build a docker test to validate Zoe/ZCF upgrade #7980)
• remove stagings from ZCF (reallocate, incrementBy, decrementBy, hasStagedAllocations, and clear) (Remove stagings from ZCF #7981)
• merge refactor(vaultFactory!): vaultFactory use zcf.atomicRearrange #7901, refactor(PSM!): make PSM use zcf.atomicRearrange() #7903, and refactor(Auction!): make Auction use zcf.atomicRearrange() #7904. There's no hurry on these; they should each be done as other changes are made to those contracts.

[turadg]

Thanks for writing that up @Chris-Hibbert . I put the "get onto chain" requirements into #7982