replace ROLE with 3 bootstrap modules; audit contents #7049
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
This was referenced Feb 24, 2023
dckc
force-pushed
the
dc-boot-check
branch
3 times, most recently
from
6aa1dde to
a57cc44
Compare
dckc
force-pushed
the
dc-boot-check
branch
5 times, most recently
from
b43084f to
3194c47
Compare
dckc
force-pushed
the
dc-boot-check
branch
3 times, most recently
from
b2d2f00 to
37c10aa
Compare
Datadog ReportBranch report: ✅ |
|
@michaelfig in Is that what you would expect? Is this where the trick with the old wallet package comes in somehow? I'm looking at |
dckc
commented
Mar 7, 2023
dckc
changed the title
|
@warner this PR includes a unit test to audit bundles similar to our Jan discussion. There's more to do: currently the prohibition on It checks every module in each bundle for a This does enumerate all the bundles, both directly in the swingset config and indirectly via @michaelfig @turadg please take a look. I wonder about breaking it into pieces, but I've been smushing the parts around for so long that I think I need other eyes on it to make useful progress. |
turadg
reviewed
Mar 7, 2023
| @@ -345,7 +347,7 @@ export const PSM_GOV_MANIFEST = { | |||
| }; | |||
|
|
|||
| export const INVITE_PSM_COMMITTEE_MANIFEST = harden( | |||
| /** @type {import('@agoric/vats/src/core/manifest.js').BootstrapManifest} */ | |||
| /** @type {import('@agoric/vats/src/core/lib-boot.js').BootstrapManifest} */ | |||
There was a problem hiding this comment.
defined up top
Suggested change
| /** @type {import('@agoric/vats/src/core/lib-boot.js').BootstrapManifest} */ | |
| /** @type {BootstrapManifest} */ |
| export const PSM_MANIFEST = harden({ | ||
| /** @type {import('@agoric/vats/src/core/manifest.js').BootstrapManifestPermit} */ | ||
| /** @type {import('@agoric/vats/src/core/lib-boot.js').BootstrapManifestPermit} */ |
There was a problem hiding this comment.
implied by being a property of BootstrapManifest
Suggested change
| /** @type {import('@agoric/vats/src/core/lib-boot.js').BootstrapManifestPermit} */ |
There was a problem hiding this comment.
looks like the BootstrapManifest declaration has to go inside harden() too.
Comment on lines
7
to
8
| import { makeAgoricNamesAccess } from '@agoric/vats/src/core/utils.js'; | ||
| import { makePromiseSpace } from '@agoric/vats'; |
There was a problem hiding this comment.
Suggested change
| import { makeAgoricNamesAccess } from '@agoric/vats/src/core/utils.js'; | |
| import { makePromiseSpace } from '@agoric/vats'; | |
| import { makeAgoricNamesAccess, makePromiseSpace } from '@agoric/vats'; |
| makeAgoricNamesAccess, | ||
| makePromiseSpace, | ||
| } from '@agoric/vats/src/core/utils.js'; | ||
| import { makeAgoricNamesAccess } from '@agoric/vats/src/core/utils.js'; |
|
|
||
| /** @type {import('./lib-boot').BootstrapManifest} */ | ||
| export const BASIC_BOOTSTRAP_PERMITS = harden({ | ||
| /** @type {import('./lib-boot').BootstrapManifestPermit} */ |
packages/vats/src/core/boot-chain.js
Outdated
Comment on lines
11
to
16
| const { | ||
| BASIC_BOOTSTRAP_PERMITS: _5, | ||
| PowerFlags: _3, | ||
| makeMyAddressNameAdminKit: _4, | ||
| ...basicBehaviors | ||
| } = basicBehaviorsPlus; |
There was a problem hiding this comment.
the numbers seem significant but aren't. consider:
Suggested change
| const { | |
| BASIC_BOOTSTRAP_PERMITS: _5, | |
| PowerFlags: _3, | |
| makeMyAddressNameAdminKit: _4, | |
| ...basicBehaviors | |
| } = basicBehaviorsPlus; | |
| const { | |
| BASIC_BOOTSTRAP_PERMITS: _B, | |
| PowerFlags: _P, | |
| makeMyAddressNameAdminKit: _m, | |
| ...basicBehaviors | |
| } = basicBehaviorsPlus; |
Comment on lines
180
to
184
| /** | ||
| * WARNING: not for production use | ||
| * |
There was a problem hiding this comment.
the import and the line below seem sufficient:
notForProductionUse();
dckc
commented
Mar 7, 2023
- misc static types - note TODO ideas - make executable
- toward test for non-upgradeable vats
No change to the runtime names, for now: vatParams.argv
- test: sim/demo config launches Vaults as expected by loadgen - demo-config: add missing proposals: - add-collateral-core for IbcATOM - price-feed-core - test: fill out expected home properties - installSimEgress: make hardcodedClientAddresses optional - type: prune governanceActions (obsolete in favor of coreProposals) - type: prune bootstrapManifest from BootstrapVatParams - no longer dynamic - rename USDC -> DAI in connectFaucet() - add missing harden()s in nameHub, connectFaucet() - factor out makeHomeFor()
- move makeMyAddressNameAdminKit to utils
- bonus: avoids importing all of basic-behaviors
into places such as provisionPool.js
- move PowerFlags to walletFlags.js
- avoid redundant imports in boot-chain.js, -sim, -client
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.
refs: #6687, #7044
Description
boot.jsincluded everything for production chain, sim, and solo and dispatched at runtime based onROLEarg. Instead, we choose betweenboot-chain.js,boot-sim.js,boot-solo.jsmodules in the swingset config, so thatboot-chain.jshas only production code.This introduces a
notForProductionUse()"poison pill" and any bundle that includes its module is flagged.Tasks
Some items in the list of non-upgradeable vats are commented out; follow-on PRs should address them.
Security Considerations
Eliminates need to evaluate bootstrap logic to determine that test code is never run.
Scaling Considerations
Small code size reduction in production?
Documentation Considerations
Documentation about which bootstrap config files are used for what is still pretty messy.
Testing Considerations
Figuring out all the constraints on bootstrap and all the different bootstrap configs is challenging. I added tests for some of them, I think.