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

Excise reallocate() #10871

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Excise reallocate() #10871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bd7ae9e
refactor!: remove reallocate and support for staged allocations
Chris-Hibbert Jan 22, 2025
7854c0a
refactor: satisfies() no longer needs zcf param
Chris-Hibbert Jan 22, 2025
24475c6
test: drop zcfProbe test
Chris-Hibbert Jan 22, 2025
e46d102
docs: update references to zcf.reallocate() in docs
Chris-Hibbert Jan 22, 2025
da28af3
docs: drop obsolete comment references to incrementBy, decrementBy
Chris-Hibbert Jan 22, 2025
b6231b6
test: update obsolete reallocate() call in test
Chris-Hibbert Jan 22, 2025
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
124 changes: 0 additions & 124 deletions packages/boot/test/bootstrapTests/zcf-upgrade.test.ts
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

I suppose we have coverage of basic upgrade functionality through some other test (preferably something lighter than an integration test)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we don't.

In order to test upgrading a contract from one version of ZCF to another, we'd need to be able to do what I call time-travel testing, or maintain two different versions of ZCF to install within a single test. We can do that in agoric-3-proposals to represent the change from a past version to the current version, or in a3p-integration to represent the change from the current-on-chain version to this new one. But otherwise, any unit test or bootstrap test would be testing a move the the current version to itself, which is no change at all.

The test it would be straightforward to add to a3p-integration would call E(zoeConfigFacet).updateZcfBundleId(newBundleId), early in the upgrade process, and then any vat that gets upgraded later will use the new ZCF. The tests in p::upgrade-19 exercise ZCF enough to show that the upgrade will be successful.

I'll add that to the tests, and also add the update to the staged changes in upgrade.go.

Copy link
Member

@mhofman mhofman Jan 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I thought we had the ability to do a fake livelsots test of a single contract? I suppose any interaction with Zoe is a problem for such a test?

This file was deleted.

154 changes: 0 additions & 154 deletions packages/boot/test/bootstrapTests/zcfProbe.contract.js
View file
Open in desktop

This file was deleted.

9 changes: 6 additions & 3 deletions packages/smart-wallet/test/gameAssetContract.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,13 @@ export const start = async zcf => {

// We use the deprecated stage/reallocate API
// so that we can test this with the version of zoe on mainnet1B.
playerSeat.decrementBy(gameSeat.incrementBy(give));
const tmp = mint.mintGains(want);
playerSeat.incrementBy(tmp.decrementBy(want));
zcf.reallocate(playerSeat, tmp, gameSeat);
zcf.atomicRearrange(
harden([
[playerSeat, gameSeat, give],
[tmp, playerSeat, want],
]),
);
playerSeat.exit(true);
return 'welcome to the game';
};
Expand Down
2 changes: 2 additions & 0 deletions packages/zoe/docs/AttackGuide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ The main focus of most threats would be a breach of one of Zoe's core invariants

## Reallocation

THIS SECTION IS OBSOLETE. We've converted all code to use attomicRearrange
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
THIS SECTION IS OBSOLETE. We've converted all code to use attomicRearrange
THIS SECTION IS OBSOLETE. We've converted all code to use atomicRearrange


The current approach (staging, incrementBy/decrementBy and the fact that all seats must
be included in realloc) has led to a few bugs. It's probably worth looking for other cases
that create new stagings or presume there are none outstanding. We plan to replace this
Expand Down
10 changes: 0 additions & 10 deletions packages/zoe/docs/seats.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ __ZCFSeat.exit() Flow:__

![ZCFSeat Exit Flow](./zcf-seat-exit-flow.png)

__ZCF.reallocate() Flow:__

![ZCF Reallocate Flow](./zcf-reallocate-flow.png)


## UserSeat

The `UserSeat` is what is returned when a user calls
Expand Down Expand Up @@ -67,12 +62,7 @@ The type of the ZCFSeat is:
* @property {() => ProposalRecord} getProposal
* @property {ZCFGetAmountAllocated} getAmountAllocated
* @property {() => Allocation} getCurrentAllocation
* @property {() => Allocation} getStagedAllocation
* @property {() => boolean} hasStagedAllocation
* @property {(newAllocation: Allocation) => boolean} isOfferSafe
* @property {(amountKeywordRecord: AmountKeywordRecord) => AmountKeywordRecord} incrementBy
* @property {(amountKeywordRecord: AmountKeywordRecord) => AmountKeywordRecord} decrementBy
* @property {() => void} clear
*/
```

Expand Down
Binary file removed packages/zoe/docs/zcf-reallocate-flow.png
View file
Open in desktop
Binary file not shown.
26 changes: 0 additions & 26 deletions packages/zoe/docs/zcf-reallocate-flow.puml
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions packages/zoe/docs/zoe-catalog-of-storage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ seats.

### sumsByBrand - Store from `@agoric/store`

Two of these are created in every call to `zcf.reallocate`, and then
are immediately dropped.
One of these is created in every call to `zcf.atomicRearrange`, and then
is immediately dropped.

**Expected cardinality**: One key per brand of allocation reallocated over.

Expand Down
Loading
Loading