fix: endow with original unstructured assert
#9514
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erights
force-pushed
the
markm-fix-endowments
branch
from
38cccd2
to
c2df9fa
Compare
Deploying agoric-sdk with
|
| Latest commit: |
887eb94
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a13539ca.agoric-sdk.pages.dev |
| Branch Preview URL: | https://markm-fix-endowments.agoric-sdk.pages.dev |
erights
changed the title
assert
erights
force-pushed
the
markm-fix-endowments
branch
2 times, most recently
from
963ba73
to
271d05e
Compare
|
Reviewers, if you see a way to refactor this change so there is less redundancy and fewer sites that need to be coordinated, please let me know. I do not like the maintenance burden produced by this PR. Or rather, produced by any naive solution to #9515 |
This was referenced Jun 16, 2024
erights
force-pushed
the
markm-fix-endowments
branch
from
0dc6ce1
to
00619c6
Compare
kriskowal
approved these changes
Jun 17, 2024
erights
added a commit
to endojs/endo
that referenced
this pull request
Jun 17, 2024
Closes: #XXXX Refs: Agoric/agoric-sdk#9515 Agoric/agoric-sdk#9514 ## Description Addresses Agoric/agoric-sdk#9515 as applied to endo, by doing for endo what Agoric/agoric-sdk#9514 does for agoric-sdk To address Agoric/agoric-sdk#5672 for endo , we should change all applicable uses of `assert` to obtain `assert` by importing it from the `@endo/errors` package. However, attempts to do so ran into the symptoms reported at Agoric/agoric-sdk#9515 for the reasons reported there -- accidentally using the imported `assert` as the endowment for the global `assert` of new Compartments. This PR prepares the ground for these fixes to Agoric/agoric-sdk#5672 for endo by unambiguously endowing with the original unstructured `globalThis.assert`, which will remain the correct endowment even when other uses of `assert` have migrated to the one imported from `@endo/errors`. By itself, this PR, preceding those fixes to Agoric/agoric-sdk#5672 for endo , is not actually fixing a bug in the sense of a behavioral change. Reviewers, let me know if you think this PR should be labeled a "refactor" because of this. ### Security Considerations none ### Scaling Considerations none ### Documentation Considerations anyone gathering endowments for a new compartment should be aware of this issue, and be sure to use `globalThis.assert` as the `assert` endowment. Fortunately, this will only be of concern to advanced developers. ### Testing Considerations Since this PR doesn't actually cause any behavioral change, it cannot be tested in place. Rather, its test will be whether #2324 still passes CI once rebased on this PR. ***Update***: #2324 is now passing CI well enough to consider this PR (#2323) to be adequately tested. ### Compatibility Considerations The point. This PR itself only prepares the ground for the equivalent of Agoric/agoric-sdk#9513 for endo. By itself it has no other effect, and so no other compat issues. ### Upgrade Considerations none
erights
force-pushed
the
markm-fix-endowments
branch
from
00619c6
to
89f599b
Compare
mhofman
pushed a commit
that referenced
this pull request
Jun 20, 2024
closes: #9515 refs: #5672 #8332 #9513 endojs/endo#2323 ## Description To address #5672 , we should change all uses of `assert` to obtain `assert` by importing it from the `@endo/errors` package. However, attempts to do so (#8332 #9513) ran into the symptoms reported at #9515 for the reasons reported there -- accidentally using the imported `assert` as the endowment for the global `assert` of new Compartments. This PR prepares the ground for these fixes to #5672 by unambiguously endowing with the original unstructured `globalThis.assert`, which will remain the correct endowment even when other uses of `assert` have migrated to the one imported from `@endo/errors`. By itself, this PR, preceding those fixes to #5672 , is not actually fixing a bug in the sense of a behavioral change. Reviewers, let me know if you think this PR should be labeled a "refactor" because of this. ### Security Considerations none ### Scaling Considerations none ### Documentation Considerations anyone gathering endowments for a new compartment should be aware of this issue, and be sure to use `globalThis.assert` as the `assert` endowment. Fortunately, this will only be of concern to advanced developers. ### Testing Considerations Since this PR doesn't actually cause any behavioral change, it cannot be tested in place. Rather, its test will be whether #9513 still passes CI once rebased on this PR. ***Update***: #9513 is now passing CI well enough to consider this PR (#9514) to be adequately tested. ### Upgrade Considerations This PR by itself does not have any dependence on @endo/errors existing, and so should be compatible even when linked against fairly ancient versions of endo.
mhofman
pushed a commit
that referenced
this pull request
Jun 22, 2024
closes: #9515 refs: #5672 #8332 #9513 endojs/endo#2323 ## Description To address #5672 , we should change all uses of `assert` to obtain `assert` by importing it from the `@endo/errors` package. However, attempts to do so (#8332 #9513) ran into the symptoms reported at #9515 for the reasons reported there -- accidentally using the imported `assert` as the endowment for the global `assert` of new Compartments. This PR prepares the ground for these fixes to #5672 by unambiguously endowing with the original unstructured `globalThis.assert`, which will remain the correct endowment even when other uses of `assert` have migrated to the one imported from `@endo/errors`. By itself, this PR, preceding those fixes to #5672 , is not actually fixing a bug in the sense of a behavioral change. Reviewers, let me know if you think this PR should be labeled a "refactor" because of this. ### Security Considerations none ### Scaling Considerations none ### Documentation Considerations anyone gathering endowments for a new compartment should be aware of this issue, and be sure to use `globalThis.assert` as the `assert` endowment. Fortunately, this will only be of concern to advanced developers. ### Testing Considerations Since this PR doesn't actually cause any behavioral change, it cannot be tested in place. Rather, its test will be whether #9513 still passes CI once rebased on this PR. ***Update***: #9513 is now passing CI well enough to consider this PR (#9514) to be adequately tested. ### Upgrade Considerations This PR by itself does not have any dependence on @endo/errors existing, and so should be compatible even when linked against fairly ancient versions of endo.
mhofman
added a commit
that referenced
this pull request
Jun 26, 2024
Rebase todo: ``` # Branch fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- label base-fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- pick bcecf52 test(vow): add test of more vow upgrade scenarios pick d7135b2 test: switch vow test to run under xs for metering pick 99fccca test(vow): add test for resolving vow to external promise pick 6d3f88c test(vow): add test for vow based infinite vat ping pong pick c78ff0e test(vow): check vow consumers for busy loops or hangs pick 3c63cba fix(vow): prevent loops and hangs from watching promises pick 188c810 chore(vat-data): remove the deprecated `@agoric/vat-data/vow.js` pick 44a6d16 fix(vow): allow resolving vow to external promise label fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- reset base-fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- merge -C 4fca040 fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- # fix(vow): make watch/when more robust against loops and hangs (#9487) # Branch ci-mergify-strip-merge-commit-HTML-comments-9499- label base-ci-mergify-strip-merge-commit-HTML-comments-9499- pick 63e21ab ci(mergify): strip merge commit HTML comments label ci-mergify-strip-merge-commit-HTML-comments-9499- reset base-ci-mergify-strip-merge-commit-HTML-comments-9499- merge -C 7b93671 ci-mergify-strip-merge-commit-HTML-comments-9499- # ci(mergify): strip merge commit HTML comments (#9499) # Pull request #9506 pick 249a5d4 fix(SwingSet): Undo deviceTools behavioral change from #9153 (#9506) # Pull request #9507 pick a19a964 fix(liveslots): promise watcher to cause unhandled rejection if no handler (#9507) # Branch feat-make-vat-localchain-resumable-9488- label base-feat-make-vat-localchain-resumable-9488- pick 76c17c6 feat: make vat-localchain resumable pick 40ccba1 fix(vow): correct the typing of `unwrap` pick 90e062c fix(localchain): work around TypeScript mapped tuple bug pick 3027adf fix(network): use new `ERef` and `FarRef` label feat-make-vat-localchain-resumable-9488- reset base-feat-make-vat-localchain-resumable-9488- merge -C 5856dc0 feat-make-vat-localchain-resumable-9488- # feat: make vat-localchain resumable (#9488) # Branch ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- label base-ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- pick 7247bd9 ci(mergify): clarify `queue_conditions` and `merge_conditions` label ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- reset base-ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- merge -C 30e56ae ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- # ci(mergify): clarify `queue_conditions` and `merge_conditions` (#9510) # Branch fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- label base-fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- pick 42ea8a3 fix(liveslots): cache.delete() does not return a useful value label fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- reset base-fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- merge -C a2e54e1 fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- # fix(liveslots): cache.delete() does not return a useful value (#9509) # Branch chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- label base-chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- pick 9930bd3 chore(swingset): re-enable test of unrecognizable orphan cleanup label chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- reset base-chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- merge -C bc53ef7 chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- # chore(swingset): re-enable test of unrecognizable orphan cleanup (#8694) # Pull request #9508 pick 513adc9 refactor(internal): move async helpers using AggregateError to node (#9508) # Branch report-bundle-sizing-in-agoric-run-9503- label base-report-bundle-sizing-in-agoric-run-9503- pick 68af59c refactor: inline addRunOptions pick a0115ed feat: writeCoreEval returns plan pick bd0edcb feat: stat-bundle and stat-plan scripts pick 0405202 feat: agoric run --verbose pick 22b43da feat(stat-bundle): show CLI to explode the bundle label report-bundle-sizing-in-agoric-run-9503- reset base-report-bundle-sizing-in-agoric-run-9503- merge -C 7b30169 report-bundle-sizing-in-agoric-run-9503- # report bundle sizing in agoric run (#9503) # Branch ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- label base-ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- pick 5f3c1d1 test(boot): use a single bundle directory for all tests pick 50229bd ci(all-packages): split tests according to AVA recipe label ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- reset base-ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- merge -C 5375019 ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- # ci(test-boot): split up test jobs via AVA recipe (#9511) # Pull request #9514 pick f908f89 fix: endow with original unstructured `assert` (#9514) # Pull request #9535 pick 989aa19 fix(swingset): log vat termination and upgrade better (#9535) # Branch types-zoe-setTestJig-param-type-optional-9533- label base-types-zoe-setTestJig-param-type-optional-9533- pick 426a3be types(zoe): setTestJig param type optional label types-zoe-setTestJig-param-type-optional-9533- reset base-types-zoe-setTestJig-param-type-optional-9533- merge -C bf9f03b types-zoe-setTestJig-param-type-optional-9533- # types(zoe): setTestJig param type optional (#9533) # Branch adopt-TypeScript-5-5-9476- label base-adopt-TypeScript-5-5-9476- pick 381b6a8 chore(deps): bump Typescript to 5.5 release label adopt-TypeScript-5-5-9476- reset base-adopt-TypeScript-5-5-9476- merge -C 0cfea88 adopt-TypeScript-5-5-9476- # adopt TypeScript 5.5 (#9476) # Branch retry-flaky-agoric-cli-integration-test-9550- label base-retry-flaky-agoric-cli-integration-test-9550- pick 2a68510 ci: retry agoric-cli integration test label retry-flaky-agoric-cli-integration-test-9550- reset base-retry-flaky-agoric-cli-integration-test-9550- merge -C c5c52ec retry-flaky-agoric-cli-integration-test-9550- # retry flaky agoric-cli integration test (#9550) # Pull Request #9556 pick 0af876f fix(vow): watcher args instead of context (#9556) # Pull Request #9561 pick a4f86eb fix(vow): handle resolution loops in vows (#9561) # Branch Restore-a3p-tests-9557- label base-Restore-a3p-tests-9557- pick d36382d chore(a3p): restore localchain test pick 5ff628e Revert "test: drop or clean-up old Tests" pick b5cf8bd fix(localchain): `callWhen`s return `PromiseVow` label Restore-a3p-tests-9557- reset base-Restore-a3p-tests-9557- merge -C c65915e Restore-a3p-tests-9557- # Restore a3p tests (#9557) # Pull Request #9559 pick 6073b2b fix(agoric): convey tx opts to `agoric wallet` and subcommands (#9559) # Branch explicit-heapVowTools-9548- label base-explicit-heapVowTools-9548- pick 100de68 feat!: export heapVowTools pick 8cb1ee7 refactor: use heapVowTools import pick 0ac6774 docs: vow vat utils pick 9128f27 feat: export heapVowE pick 3b0c8c1 refactor: use heapVowE pick 9b84bfa BREAKING CHANGE: drop V export pick 6623af5 chore(types): concessions to prepack label explicit-heapVowTools-9548- reset base-explicit-heapVowTools-9548- merge -C 4440ce1 explicit-heapVowTools-9548- # explicit heapVowTools (#9548) # Pull Request #9564 pick 44926a7 fix(bn-patch): fix bad html evasion (#9564) # Branch Fix-upgrade-behaviors-9526- label base-Fix-upgrade-behaviors-9526- pick ef1e0a2 feat: Upgrade Zoe pick e4cc97c Revert "fix(a3p-integration): workaround zoe issues" pick 84dd229 feat: repair KREAd contract on zoe upgrade pick cb77160 test: validate KREAd character purchase pick e1d961e test: move vault upgrade from test to use phase (#9536) label Fix-upgrade-behaviors-9526- reset base-Fix-upgrade-behaviors-9526- merge -C 8d05faf Fix-upgrade-behaviors-9526- # Fix upgrade behaviors (#9526) # Branch Support-for-snapshots-export-command-9563- label base-Support-for-snapshots-export-command-9563- pick 2a3976e refactor(cosmos): use DefaultBaseappOptions for newApp pick 84208e9 fix(cosmos): use dedicated dedicate app creator for non start commands pick 8c1a62d chore(cosmos): refactor cosmos command extension pick 4386f8e feat(cosmos): support snapshot export pick 2dabb52 test(a3p): add test for snapshots export and restore label Support-for-snapshots-export-command-9563- reset base-Support-for-snapshots-export-command-9563- merge -C 309c7e1 Support-for-snapshots-export-command-9563- # Support for `snapshots export` command (#9563) # Branch Swing-store-export-data-outside-of-genesis-file-9549- label base-Swing-store-export-data-outside-of-genesis-file-9549- pick f1eacbe fix(x/swingset): handle defer errors on export write pick 496a430 feat(cosmos): add hooking kv reader pick f476bd5 feat(cosmos): separate swing-store export data from genesis file pick 17a5374 test(a3p): add genesis fork acceptance test label Swing-store-export-data-outside-of-genesis-file-9549- reset base-Swing-store-export-data-outside-of-genesis-file-9549- merge -C 3aa5d66 Swing-store-export-data-outside-of-genesis-file-9549- # Swing-store export data outside of genesis file (#9549) # Branch Remove-scaled-price-authority-upgrade-9585- label base-Remove-scaled-price-authority-upgrade-9585- pick bce49e3 test: add test during upgradeVaults; vaults detect new prices pick 88f6500 test: repair test by dropping upgrade of scaledPriceAuthorities label Remove-scaled-price-authority-upgrade-9585- reset base-Remove-scaled-price-authority-upgrade-9585- merge -C 8376991 Remove-scaled-price-authority-upgrade-9585- # Remove scaled price authority upgrade (#9585) # Branch feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- label base-feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- pick 95174a2 feat(builders): non-ambient `strictPriceFeedProposalBuilder` in `priceFeedSupport.js` pick 5cc190d feat(app): establish mechanism for adding core proposals by `upgradePlan.name` pick 52f02b7 fix(builders): use proper `oracleBrand` subkey case pick ddc072d chore(cosmos): extract `app/upgrade.go` pick b3182a4 chore: fix error handling of upgrade vaults proposal pick ea568a2 fix: retry upgrade vaults price quote label feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- reset base-feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- merge -C cbe061c feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- # feat: make software upgrade `coreProposals` conditional on upgrade plan name (#9575) ``` This is followed by a commit to remove the `orchestration` and `async-flow` packages from the release, as these are not baked in yet and are not deployed anyway.
mergify bot
pushed a commit
that referenced
this pull request
Jul 3, 2024
Staged on #9514 closes: #5672 refs: #8332 #9512 #9514 Some description text below copied from #8332 ## Description Use the new `@endo/errors` and retire `@agoric/assert`. This deletes the `@agoric/assert` package but doesn't so far as to deprecate it in NPM. Fixes #5672 -- migrating all applicable uses of `assert` to import it from @endo/errors and use that one. This is a fresh attempt to do over what #8332 and #9512 tried to do. Note that this is staged on #9514 which addresses #9515 -- ensuring that the `assert` used in endowing a new compartment is not the `assert` imported from @endo/errors but is rather the original `globalThis.assert`. For the agoric-sdk repo, that should be the only needed use of `assert` that should not be changed to use the one imported from @endo/errors. ### Security Considerations Relies more directly on Endo, instead of through the `assert` package. ### Scaling Considerations none ### Documentation Considerations we should generally document @endo/errors as the only source of `assert`. However, we still need to call out the special case of Compartment endowments covered by #9514 and #9515 ### Testing Considerations If CI passes here, it not only tests the correctness of this PR, but also of #9514, since #9514 by itself does not cause any testable changes. It is only to enable the changes in this PR to work. ### Upgrade Considerations TBD. `@endo/errors` may require a newer SES than the vats have in their global env (for the `assert` global).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes: #9515
refs: #5672 #8332 #9513 endojs/endo#2323
Description
To address #5672 , we should change all uses of
assertto obtainassertby importing it from the@endo/errorspackage. However, attempts to do so (#8332 #9513) ran into the symptoms reported at #9515 for the reasons reported there -- accidentally using the importedassertas the endowment for the globalassertof new Compartments.This PR prepares the ground for these fixes to #5672 by unambiguously endowing with the original unstructured
globalThis.assert, which will remain the correct endowment even when other uses ofasserthave migrated to the one imported from@endo/errors. By itself, this PR, preceding those fixes to #5672 , is not actually fixing a bug in the sense of a behavioral change. Reviewers, let me know if you think this PR should be labeled a "refactor" because of this.Security Considerations
none
Scaling Considerations
none
Documentation Considerations
anyone gathering endowments for a new compartment should be aware of this issue, and be sure to use
globalThis.assertas theassertendowment. Fortunately, this will only be of concern to advanced developers.Testing Considerations
Since this PR doesn't actually cause any behavioral change, it cannot be tested in place. Rather, its test will be whether #9513 still passes CI once rebased on this PR.
Update: #9513 is now passing CI well enough to consider this PR (#9514) to be adequately tested.
Upgrade Considerations
This PR by itself does not have any dependence on @endo/errors existing, and so should be compatible even when linked against fairly ancient versions of endo.