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

feat(cosmic-swingset): Introduce inquisitor.mjs #10807

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

Conversation

gibson042
Copy link
Member

refs: #10725

Description

Introduces a tool that loads a swingstore.sqlite file and allows scripted or interactive interrogation of both the database and vats in an ephemeral environment.

Security Considerations

This adds new exports for internal use by the inquisitor and related tools, entailing some layer-crossing (e.g., returning a reference to the SwingSet controller from the chain-oriented launchAndShareInternals).

Scaling Considerations

n/a

Documentation Considerations

The script includes usage information.

Testing Considerations

This tool is for internal use, and does not have unit tests. There's a risk of bit-rot, but the alternative is excessive effort on covering rich environmental functionality that might still be insufficient protection.

Upgrade Considerations

n/a

@gibson042 gibson042 requested a review from a team as a code owner January 7, 2025 03:06
Copy link

cloudflare-workers-and-pages bot commented Jan 8, 2025

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 12e96c8
Status: ✅  Deploy successful!
Preview URL: https://f0b505e8.agoric-sdk.pages.dev
Branch Preview URL: https://gibson-2025-01-inquisitor.agoric-sdk.pages.dev

View logs

@gibson042 gibson042 force-pushed the gibson-2025-01-inquisitor branch from 773757a to e39c802 Compare February 11, 2025 03:19
packages/boot/tools/supports.ts Show resolved Hide resolved
packages/cosmic-swingset/src/chain-main.js Show resolved Hide resolved
packages/cosmic-swingset/tsconfig.json Outdated Show resolved Hide resolved
* used to replace the results of recursive picks (but not blanket permits)
* @returns {Attenuated<T, P>}
*/
export const attenuate = (specimen, permit, transform = x => x) => {
Copy link
Member

Choose a reason for hiding this comment

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

this is useful. why bucket it in "SES utils"? I see other stuff in here that's not specific to the ses package so not a big deal. Maybe we just rename this module at some point.

Copy link
Member

Choose a reason for hiding this comment

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

regardless, needs unit tests.

Copy link
Member Author

@gibson042 gibson042 Feb 14, 2025

Choose a reason for hiding this comment

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

this is useful. why bucket it in "SES utils"? I see other stuff in here that's not specific to the ses package so not a big deal. Maybe we just rename this module at some point.

I'd love to have it in an even more fundamental file, but it relies on Fail, which frustratingly depends upon SES, making this the lowest point it can go right now. At some point, I plan to introduce a package that propagates SES's redacting assert functionality when available but otherwise creates approximations like q = x => JSON.stringify(x) and Fail = (strings, ...subs) => { throw Error(String.raw(strings, ...subs.map(q))); }, at which point this function and probably a few others can move into js-utils.js.

*/
export const attenuate = (specimen, permit, transform = x => x) => {
// Fast-path for no attenuation.
if (permit === true || typeof permit === 'string') {
Copy link
Member

Choose a reason for hiding this comment

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

why allow string? I could see someone mistakenly thinking they can permit one thing by passing its name.

I see below,

serving as a grouping label for convenience and/or diagram generation

Why would you want that at the exclusion of an actual permit? Seems like an independent feature to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

@turadg

someone mistakenly thinking they can permit one thing by passing its name.

this is exactly what I thought. Thanks for articulating it.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a following a pattern established within agoric-sdk long ago, documented in deploy-script-support and used by e.g. authorityViz.js and core-eval handling. I'd defer to @dckc and/or @Chris-Hibbert for opinions on whether or not that feature should be abandoned, but I suspect we actually do want to keep it.

There's also the practical matter of TypeScript inferring { $propertyName: true } to be of type { $propertyName: boolean }, which means that const permits = { foo: 'pick' }; works but const permits = { foo: true }; does not and instead requires a type cast (and often also Prettier reformatting) to const permits = /** @type {const} */ ({ foo: true });. But for clarity in this PR, I've updated to that last form.

Copy link
Member

Choose a reason for hiding this comment

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

The core-eval extract attenuator is lazy, making a new Proxy(...) at each level. This one seems to be eager, using Object.entries(...). It's quite a different widget. So I don't see much reason to constrain the design of this one by comparison to extract.

Copy link
Member Author

Choose a reason for hiding this comment

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

extract isn't lazy; it eagerly reads from and validates the specimen just like this function. It does return a Proxy at each level, but only to implement property [[Get]] behavior that throws an error for keys outside the permit. And that's actually the reason for attenuate's optional transform parameter—so core-eval handling code can be redefined as

/**
 * @template T
 * @template {Permit<T>} P
 * @param {P} permit
 * @param {T} allPowers
 */
export const extractPowers = (permit, allPowers) => {
  if (typeof permit === 'object' && permit !== null) {
    const {
      // TODO: use these for more than just visualization.
      home: _h,
      ...effectivePermit
    } = permit;
    permit = effectivePermit;
  }
  return attenuate(allPowers, permit, (subPowers, subPermit) => {
    return new Proxy(subPowers, {
      get: (target, propName) => {
        if (typeof propName !== 'symbol') {
          propName in target ||
            Fail`${q(propName)} not permitted, only ${q(keys(subPermit))}`;
        }
        return target[propName];
      },
    });
  });
};

and thereby benefit from the typing improvements implemented here.

deleteVatTranscripts: transcriptStore.deleteVatTranscripts,
};
const transcriptStore = attenuate(transcriptStoreInternal, {
initTranscript: 'pick',
Copy link
Member

Choose a reason for hiding this comment

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

pick is misleading because it suggests a label like skip would skip but it also permits.

Any reason not to require this to be boolean?

Copy link
Member Author

Choose a reason for hiding this comment

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

Compatibility with

if (template === true || typeof template === 'string') {
, which should be reimplemented on top of this.

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.

4 participants