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.

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

chore: prune non-upgradeable network vats from production config #7165

Closed
wants to merge 63 commits into from

Conversation

dckc
Copy link
Member

@dckc dckc commented Mar 14, 2023

closes: #7044

Description

reorg network / IBC bootstrap to avoid including vat-network in production config

Security Considerations

facilitates audit of production config

Scaling, Documentation Considerations

none AFAICT

Testing Considerations

TODO:

  • finish WIP commits
  • try the smoke tests

dckc added 30 commits March 13, 2023 11:01
 - confine ambient authority to test set-up region
 - fix some static typing
  - update boot-psm.js now that manifest.js is gone
  - move BootstrapManifest type to lib-boot.js

chore(vats): reorg boot-chain, chain-behaviors

 - restore test-boot.js tests

 - static types for test-boot.js and boot-*.js
   - prune argvByRole
 - no more export *
 - lib-boot: early check for missing behavior

 - basic-behaviors.js:
   - organize imports a bit
   - all behaviors return Promise<void>
     use await rather than return
   - export BASIC_BOOTSTRAP_PERMITS
 - boot-chain.js:
   - destructure non-behaviors out of import *
   - hoist modules
 - chain-behaviors: factor BASIC_BOOTSTRAP_PERMITS out
   of SHARED_CHAIN_BOOTSTRAP_MANIFEST
With an empty `modules` bootstrap power,

`Cannot read properties of undefined (reading 'runModuleBehaviors')`

... was thrown in the code generated by makeWriteCoreProposal()
in code-gen.js

The initial symptoms were pretty obscure:

```
KERNEL PANIC: kp40.policy panic: rejected {"body":"#{\"#error\":\"behavior: cannot coerce undefined to object\"
```

These debug settings gave us a useful stack trace:

`packages/solo/test$ DEBUG=SwingSet:ls,SwingSet:vat,track-turns ./startsolo.sh`

The offending line is referring to `runModuleBehaviors` as a module global.
So `Cannot read properties of undefined` seems to be SES-shim-speak for
"Reference Error".
Patterned after devnet config, but uses boot-sim.js bootstrap vat.

```diff
$ diff -wu ./decentral-devnet-config.json ./decentral-demo-config.json
--- ./decentral-devnet-config.json      2023-02-24 14:02:56.015581229 -0600
+++ ./decentral-demo-config.json        2023-02-24 14:59:18.991084488 -0600
@@ -106,7 +106,7 @@
   ],
   "vats": {
     "bootstrap": {
-      "sourceSpec": "@agoric/vats/src/core/boot-chain.js",
+      "sourceSpec": "@agoric/vats/src/core/boot-sim.js",
       "creationOptions": {
         "critical": true
       }
```
 - export manifest from sim-behaviors.js
 - core(inter-protocol): static types for sim-behaviors
   - return Promise<void>
 - 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()
@datadog-full-agoric
Copy link

datadog-full-agoric bot commented Mar 14, 2023

Datadog Report

Branch report: dc-net-not-prod
Commit report: 52a0e91

agoric-sdk: 8 Failed (0 Known Flaky), 0 New Flaky, 2222 Passed, 25 Skipped, 16m 52.12s Wall Time

❌ Failed Tests (8)

This report shows up to 5 failed tests.

  • gov-collateral › assets are in Vaults - agoric.inter-protocol - Details

    Expand for error
     ---
         name: AssertionError
         message: Rejected promise returned by test
         values:
           'Rejected promise returned by test. Reason:': |-
             TypeError {
               message: 'Cannot read properties of undefined (reading \'consume\')',
             }
         at: |-
           addBankAssets (packages/vats/src/core/basic-behaviors.js:263:21)
     ...
    
  • gov-collateral › Benefactor can add to reserve - agoric.inter-protocol - Details

    Expand for error
     ---
         name: AssertionError
         message: Rejected promise returned by test
         values:
           'Rejected promise returned by test. Reason:': |-
             TypeError {
               message: 'Cannot read properties of undefined (reading \'consume\')',
             }
         at: |-
           addBankAssets (packages/vats/src/core/basic-behaviors.js:263:21)
     ...
    
  • gov-collateral › Committee can raise debt limit - agoric.inter-protocol - Details

    Expand for error
     ---
         name: AssertionError
         message: Rejected promise returned by test
         values:
           'Rejected promise returned by test. Reason:': |-
             TypeError {
               message: 'Cannot read properties of undefined (reading \'consume\')',
             }
         at: |-
           addBankAssets (packages/vats/src/core/basic-behaviors.js:263:21)
     ...
    
  • gov-collateral › voters get invitations - agoric.inter-protocol - Details

    Expand for error
     ---
         name: AssertionError
         message: Rejected promise returned by test
         values:
           'Rejected promise returned by test. Reason:': |-
             TypeError {
               message: 'Cannot read properties of undefined (reading \'consume\')',
             }
         at: |-
           addBankAssets (packages/vats/src/core/basic-behaviors.js:263:21)
     ...
    
  • addAsset › before hook - agoric.smart-wallet - Details

    Expand for error
     ---
         name: AssertionError
         message: Rejected promise returned by test
         values:
           'Rejected promise returned by test. Reason:': |-
             TypeError {
               message: 'Cannot read properties of undefined (reading \'consume\')',
             }
         at: |-
           makeBoard (packages/vats/src/core/basic-behaviors.js:134:23)
     ...
    

@@ -317,14 +285,15 @@ export const addBankAssets = async ({
bldIssuerKit.resolve(bldKit);

const assetAdmin = E(agoricNamesAdmin).lookupAdmin('vbankAsset');
// XXX interposes bootstrap between agoricNames and vat-bank
Copy link
Member

Choose a reason for hiding this comment

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

@michaelfig: I'll be able to fix this one with the callback abstraction (a closureless bound method descriptor).

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to me that the simpler fix is to just pass assetAdmin and not bother with a separate updater. The only risk would be that vat-bank abuses .reserve().

Comment on lines 123 to 126
installations: {
provisionPool: restoreRef(installKeys.provisionPool),
walletFactory: restoreRef(installKeys.walletFactory),
},
Copy link
Member

Choose a reason for hiding this comment

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

I'm baffled as to why these installations are needed. Was this a typo?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, typo

@dckc
Copy link
Member Author

dckc commented May 6, 2023

obsolete in favor of #7623

@dckc dckc closed this May 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants