-
Notifications
You must be signed in to change notification settings - Fork 227
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
base: master
Are you sure you want to change the base?
Conversation
Deploying agoric-sdk with
|
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 |
We never consume any affected value as an already-realized iterator
* from file (vs. from swingstore.sqlite in a directory) * read-only (vs. read-write)
773757a
to
e39c802
Compare
…rsive `attenuate`
* used to replace the results of recursive picks (but not blanket permits) | ||
* @returns {Attenuated<T, P>} | ||
*/ | ||
export const attenuate = (specimen, permit, transform = x => x) => { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regardless, needs unit tests.
There was a problem hiding this comment.
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') { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
someone mistakenly thinking they can permit one thing by passing its name.
this is exactly what I thought. Thanks for articulating it.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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', |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compatibility with
agoric-sdk/packages/vats/src/core/utils.js
Line 123 in 8ef3fde
if (template === true || typeof template === 'string') { |
…rsive `attenuate`
…ers from test-kit.js
…ers from test-kit.js
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