-
Notifications
You must be signed in to change notification settings - Fork 212
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
Sync Endo: 2023-08-08 #8314
Sync Endo: 2023-08-08 #8314
Conversation
d6cd5ff
to
9792a58
Compare
9792a58
to
2300eca
Compare
2e39409
to
62e0168
Compare
62e0168
to
5037da2
Compare
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.
I've reviewed (and approve) all of your @kriskowal commits. Can you comment on my commits, or delegate to somebody else to do a review of them before we merge?
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.
Your commits all look good to me @michaelfig.
@@ -59,7 +59,7 @@ const runSetMathHelpersTests = (t, [a, b, c], a2) => { | |||
if (a2 !== undefined) { | |||
t.throws( | |||
() => m.make(mockBrand, harden([a, a2])), | |||
{ message: /value has duplicates/ }, | |||
{ message: /value has duplicate(| key)s/ }, |
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.
I don’t think we need another reviewer, but it would be good for @gibson042 or @erights to know that this change created a complication for synchronizing Endo. It would be good for this frustration to have been discovered earlier and for a change to land in Agoric or Endo to iron it out before we attempted integration.
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.
to have been discovered earlier
Would that be done by a process change or new tooling? If the latter, is there a ticket?
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.
I have used #7937 on occasion to detect and fix such problems. I think this is similar to some automation @michaelfig already did?
@@ -14,7 +14,7 @@ export const makeNodeBundleCache = async (dest, options, loadModule) => { | |||
styles.dim.close, | |||
); | |||
}; | |||
return wrappedMaker(dest, { log, ...options }, loadModule); | |||
return wrappedMaker(dest, { log, ...options }, loadModule, pid); |
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.
And, this is a complication that I introduced that I should have addressed in Agoric before we had attempted to sync the repos.
@michaelfig has created some tools to make these aberrations more visible. I haven’t internalized them well enough in my own process to recommend a workflow for the rest of the team.
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.
LGTM.
One question about a possible unintentional change, but it's innocuous and can be improved later.
@@ -125,7 +129,7 @@ runs: | |||
if test -e ~/endo; then | |||
# Stage the redirected `yarn install` consequences. | |||
git add package.json yarn.lock | |||
rm -rf ~/endo | |||
${{ inputs.keep-endo }} || rm -rf ~/endo |
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.
nifty
@@ -417,7 +417,7 @@ jobs: | |||
|
|||
test-boot: | |||
# BEGIN-TEST-BOILERPLATE | |||
timeout-minutes: 30 | |||
timeout-minutes: 40 |
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.
😞
@@ -305,12 +307,14 @@ harden(prepareAttenuator); | |||
/** | |||
* Prepare an attenuator whose methodNames are derived from the interfaceGuard. | |||
* | |||
* @template {import('@endo/patterns').InterfaceGuard} G |
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 isn't being used as a type parameter. did you mean to use it for the return type?
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.
It's being used as the type of the input interfaceGuard
, and it (via inference) is also used as part of the return type.
This change brings Agoric and Endo in sync.
ses
v0.18.8 (2023-09-11)repairIntrinsics(options)
andhardenIntrinsics()
from thebehavior of
lockdown(options)
so vetted shims can run between thesecalls.
Any modifications to shared intrinsics survive if applied after
repairIntrinsics()
.get the same safe
Date
constructor, that does not provide the ability tomeasure duration.
It used to do this by having
Date.now()
returnNaN
, and to have calls onthe constructor that would normally have returned an indication of the
current date, instead return the corresponding invalid date indication.
Now, all of these throw a
TypeError
whose message begins with'secure mode'
.This aligns with the XS implementation of HardenedJS.
compartments get the same safe
Math
namespace object that does not providea working
random()
function.It used to do that by omitting the
random
property from the safeMath
namespace object.
Now, the safe shared
Math
namespace object has aMath.random()
functionthat throws a
TypeError
whose message begins with'secure mode'
.This again aligns with the XS implementation of HardenedJS.
ses
v0.18.6 (2023-08-07){...import(specifier)
}.We previously censored
import(specifier)
and expressly allowedobject.import(specifier)
.The relaxation for the latter form in version 0.13.0 inadvertently allowed
import with the spread operator.
@endo/patterns
v0.2.6 (2023-09-11)matches(specimen, makeCopyMap([]))
).@endo/exo
v0.2.6 (2023-09-11)@endo/bundle-source
v2.8.0 (2023-09-11)@endo/bundle-source/cache.js
maker now accepts additional optionalarguments:
pid
andnonce
, with reasonable defaults.Providing the
pid
can make it easier to clean up scratch files ifthe bundler is interrupted, since deleting any scratch file with the PID of a
non-running process is safe.