-
Notifications
You must be signed in to change notification settings - Fork 207
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.
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
pipelined requests through fake-chain linkage can fail #1400
Comments
I've tracked this down to a comms-vat bug that probably got introduced when we added The sequence is:
|
I'm working on a test to demonstrate this within swingset/comms directly. I don't think it involves capTP, but I'll prove that with the test before removing the CapTP label and upating the title. |
For (mostly my own) reference, here's my annotated trace of the original expression of the bug:
|
I found a way to test the fix. A key difference between the swingset unit tests ( In the swingset unit tests, a turn or crank that produces multiple messages will deliver them to the "loopbox" device, which delivers them back into the same swingset machine (but to a second instance of vat-tp, which delivers them to a second instance of the comms vat). When the left vat sends When they arrive at vat-tp, the old loopbox device immediately pushed them onto the run-queue as separate messages to the right-side vat-tp vat. Then right-vattp pushes both messages to right-comms. Right-comms sees The overall order of delivery is:
So the The bug here only happens when right-comms sees "D" ( If "D" happens first, right-comms dutifully updates its internal promise table to record the fact that The failing delivery order is "A B D C". This happens because fake-chain uses a queue that lives above the kernel's run-queue, and cross-machine deliveries are made one message at a time. The first cross-machine delivery makes the right side do:
and the second makes it do:
The quick fix that @dtribble found was to add an To exercise the "A B D C" order in our unit tests, I'm changing (#1425) the loopbox device to have a higher-level queue, just like the fake-chain delivery code does. |
* refactor: refactor Zoe to use seat objects rather than offerHandles * refactor: update seat namings, add seats.md to docs with diagrams * refactor: move up init of instance, instanceAdmin * fix: minting allocations in zcf (#1382) * refactor: tests for sellItems and mintAndSellNFT pass - need to be reworked for new synchronous mint still * refactor: fix jsdoc to recognize makeZCFMint as property * fix: register the new minty issuer on zcf side (#1392) * refactor: redirect main in package.json * refactor: apply zoe renames - `getInviteIssuer` to `getInvitationIssuer` - `inviteDesc` to `description` * fix: solve some easy types * fix: use local amountMath (#1408) * fix(swingset): check promise resolution table during comms.inbound refs #1400 * refactor: multipoolAutoswap * refactor: remove BrandName parameter from ERTP types * feat: add the ability for a contract to get a synchronous seat zcf.addEmptySeat() would return a zcfSeat that the contract could use to hold allocations. untested * refactor: cleanup synchronous ZCF seat creation drop keyword parameter in addEmptySeat() extract makeZoeSeatAdminKit, use it for emptySeat, too. rename zcf seat construction to use 'Kit' * refactor: use updateFromNotifier from library * refactor: add types to reveal the wallet changes needed for Zoe * refactor: post review cleanups; mostly type info improved type annotations rename makeEmptySeat to makeOfferlessSeat don't return the zcfSeatAdmin to the contract * fix: make Zoe typesafe again * refactor: bring autoswap up to date on the new Zoe spike branch Based on the synchronous seat work. The unit tests for autoswap pass. I haven't yet done the swingset tests. * fix: update autoswap from addEmptySeat() to makeEmptySeatKit() * chore: add publicFacet and instance to MakeInstanceResult (#1418) * chore: fix typing of MakeInstanceResult * chore: add hasExited, getNotifier to UserSeat * Update more unit tests (#1422) * chore: update tests * chore: finish automaticRefund tests * chore: update barterExchange and brokenContract tests. Small changes to Zoe and types (#1427) * fix: make the wallet pass unit tests * refactor: clear up some more types * fix(dapp-svelte-wallet): minor cleanups * fix: always use the published wallet payment facet * test: invite -> creatorInvitation * fix: noticed bug in exit code while doing (chore: update coveredCall tests) (#1428) * chore: update tests for escrowToVote, grifter, multipoolAutoswap, publicAuction, sellTickets, simpleExchange * chore: make more unit tests pass, linting * chore: update tests for escrowToVote, grifter, multipoolAutoswap, publicAuction, sellTickets, simpleExchange (#1430) * fix: minor wallet cleanup * test: fix all the zoe swingSet tests * chore: find usages of .getBrand() and make sure that brand and issuer match (#1443) * refactor: rename makeInstance to startInstance (#1444) * test(dapp-svelte-wallet): fix test log for determinism The prior golden test log relied on a race between deposit and the new purse values. Correct that ambiguity. * fix: generate unique petnames, and use them * fix: only uniquify suffixes for petnames that are actually paths * chore: fix brokenContracts tests, zoe-metering tests, skip ZoeHelpers tests for now (need to be rewritten entirely, I think using the real ZCF, not a mock) (#1435) * fix: minor typing issue * chore: clean up vestiges of encouragement dapp * chore: remove contract/deploy.js from the wallet spec * refactor: prevent the same seat being an argument multiple times in reallocate (#1452) * refactor: remove makeEmptyOffer from zoeHelpers, use zcf.makeEmptySeatKit instead (#1451) * refactor: move issuerKeywordRecord and brandKeywordRecord to terms (#1459) * refactor: add issuers, brands, maths to terms * refactor: rename NonCustomTerms in types * chore: address PR comments * refactor: move AmountMath from names to Kinds; easier local creation Issuer no longer has getAmountMath(), instead exports makeLocalAmountMath(); added an enum for the three kinds of AmountMath getAmountMathHelpersName replaced with getAmountMathKind Add MathKind as an enum containing the legal values of AmountKind Made progress on correct typing * fix: review suggestions and merge conflicts. * fix: clear up the types * chore: export all of ERTP from index.js. Update all imports. A few other review-suggested cleanups * refactor: add Zoe methods for getting assured invitation values The type declarations seem to work. * refactor: drop unnecessary invitations in the metering tests. * refactor: review of the following files in the form of code changes (#1475) * refactor: review of: packages/zoe/src/contracts/autoswap.js packages/zoe/src/contracts/barterExchange.js packages/zoe/src/contracts/simpleExchange.js packages/zoe/test/swingsetTests/zoe/test-zoe.js packages/zoe/test/swingsetTests/zoe/vat-alice.js packages/zoe/test/swingsetTests/zoe/vat-bob.js packages/zoe/test/swingsetTests/zoe/vat-carol.js packages/zoe/test/swingsetTests/zoe/vat-dave.js packages/zoe/test/swingsetTests/zoe/vat-zoe.js packages/zoe/test/unitTests/contracts/test-autoswap.js packages/zoe/test/unitTests/contracts/test-barter.js packages/zoe/test/unitTests/contracts/test-simpleExchange.js packages/zoe/test/unitTests/installFromSource.js packages/zoe/test/unitTests/setupBasicMints.js packages/zoe/test/unitTests/setupMixedMints.js packages/zoe/test/unitTests/setupNonFungibleMints.js packages/zoe/test/unitTests/test-cleanProposal.js packages/zoe/test/unitTests/test-offerSafety.js packages/zoe/test/unitTests/test-rightsConservation.js packages/zoe/test/zoeTestHelpers.js in PR form rather than comments * fix: fix dapp-svelte-wallet issues * fix: lint-fix * fix: fix errors * chore: review in the form of PR w/ changes (#1484) * fix: resolve semantic merge conflict The `@agoric/zoe/src/contractFacet` moved to: `@agoric/zoe/src/contractFacet/contractFacet.js`. * refactor: add typings for some sample Zoe contracts * refactor: loosen unknown typings * chore: expose '@agoric/zoe/contractFacet' * docs: address review comments * test(swingset-runner): update demo/exchangeBenchmark for new Zoe * chore: follow #1194: take fresh copy of zoeTests from zoe * fix: don't use monorepo-relative paths to @agoric/zoe * chore: complete #1194 Problem 1 * fix: zcf is unused, so prepend an underscore lint fix * chore: complete #1194 Problem 2b * chore: complete #1194 Problem 2c * chore: move global harden declaration to package.json * fix: undo invalid Zoe change * fix: minor cleanups * chore: rename userSeat.exit() to tryExit() (#1493) * chore: rename userSeat.exit() to tryExit() * Update packages/zoe/src/objArrayConversion.js Co-authored-by: Mark S. Miller <erights@users.noreply.github.com> * chore: address small PR comments * chore: remove objToArray because unused * chore: move cleanProposal.js (#1495) * chore: remove more unused functions * chore: remove escrowAndAllocateTo * chore: remove unused type * chore: rename invite to invitation, forever more. begone! * Use flatMap Co-authored-by: Mark S. Miller <erights@users.noreply.github.com> * chore: change areRightsConserved to assertRightsConserved * chore: address PR comments * fix: reintroduce makeZoe's zcfBundleName argument; lost in merge * Update packages/zoe/src/zoeService/zoeSeat.js Co-authored-by: Chris Hibbert <Chris-Hibbert@users.noreply.github.com> * chore: address PR comments * chore: fix the lint * chore: address PR comments about kick out messages (#1529) * chore: address PR comments about exit behavior (#1527) * chore: address PR comments about exit behavior * Update packages/zoe/src/zoeService/zoeSeat.js Co-authored-by: Mark S. Miller <erights@users.noreply.github.com> * chore: fix lint, slight errors from merge Co-authored-by: Mark S. Miller <erights@users.noreply.github.com> Co-authored-by: Mark S. Miller <erights@users.noreply.github.com> Co-authored-by: Dean Tribble <tribble@agoric.com> Co-authored-by: Michael FIG <mfig@agoric.com> Co-authored-by: Brian Warner <warner@lothar.com> Co-authored-by: Chris Hibbert <hibbert@agoric.com> Co-authored-by: Chris Hibbert <Chris-Hibbert@users.noreply.github.com>
Describe the bug
When sending pipelined messages from the api deploy script via
ag-solo
to objects on the chain, the comm internal fail. See below.To Reproduce
agoric start --reset -v
agoric deploy contract/depoy.js api/deploy.js
Expected behavior
Contract and API server should complete installation
Platform Environment
WSL
agoric-sdk: new-zoe-spike-2
dapp-encouragement: new-zoe
Additional context
Specific exampel pipelining code that causes this:
results in:
Adding an
await
to avoid pipelining also avoids the problem.The text was updated successfully, but these errors were encountered: