Skip to content

chore: Sync Endo#8651

Merged
mergify[bot] merged 17 commits into
masterfrom
kris-sync-endo-2023-12-12-20-31-15
Dec 21, 2023
Merged

chore: Sync Endo#8651
mergify[bot] merged 17 commits into
masterfrom
kris-sync-endo-2023-12-12-20-31-15

Conversation

@kriskowal

@kriskowal kriskowal commented Dec 12, 2023

Copy link
Copy Markdown
Member

This change brings Agoric SDK in sync with the latest versions from Endo as of December 12, 2023.

This includes syncing up breaking changes with interface guards. Where invalid call guards would silently allow an invocation previously, they now throw. Several interface guards needed fixing.

Some accommodations were necessary for the new __getMethodNames__ and __getInterfaceGuard__ protocols.

The algorithm for performing a speculative integration with an Endo branch required an adjustment to ensure that types were generated correctly.

For reasons opaque to the integrator, this required:

  • many type imports to qualify the .js extension. The only relevant change from the Endo side is that many packages provide generated .d.ts types now.
  • @agoric/timeto have atypesfield pointing atindex.js`, which by dint of fortunate coïncidence is a valid TypeScript definition file.
  • a type error to be suppressed in @agoric/zone that only causes a type error in integration with @agoric/vats.

@kriskowal kriskowal mentioned this pull request Dec 12, 2023
@kriskowal kriskowal added the force:integration Force integration tests to run on PR label Dec 12, 2023
@kriskowal kriskowal marked this pull request as ready for review December 12, 2023 21:23
Comment thread packages/zoe/test/unitTests/test-zoe.js Outdated
getRoundData: M.call(M.any()).returns(M.promise()),
getRoundStatus: M.call(M.any()).returns(M.record()),
oracleRoundStateSuggestRound: M.call(M.any()).returns(M.record()),
authenticateQuote: M.call().rest(M.any()).returns(M.any()),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should take one quote,

/** @param {PriceQuoteValue} quote */
const authenticateQuote = quote => {
const quoteAmount = AmountMath.make(brand, harden(quote));
const quotePayment = quoteMint.mintPayment(quoteAmount);
return harden({ quoteAmount, quotePayment });
};
const calcAmountOut = amountIn => {

I'll push accurate guards.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Better guards would certainly be nice, but can happen after this PR. Thus, I won't withhold approval waiting for this conversation to be resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

refactor: Make all TS import extensions explicit

Why?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Tsc made me do it. I do not know why tsc insisted, but it did.

Comment on lines +30 to +31
* @typedef {import('@agoric/time').RelativeTime} RelativeTime // TODO: use
* RelativeTime, not RelativeTimeValue

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this TODO is odd. perhaps it's meant to be a deprecated type?

/** @type {import('.').Zone['exoClass']} */
const exoClass = (...args) => prepareExoClass(baggage, ...args);
/** @type {import('.').Zone['exoClassKit']} */
// @ts-ignore This type check regressed inexplicably with the release

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'll take a look at fixing this before approving

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Likewise, although it would be nice to get this resolved now, it can happen after this PR, so I won't withhold approval waiting for it.

Comment thread package.json Outdated
"packageManager": "yarn@1.22.19",
"devDependencies": {
"@endo/eslint-plugin": "^0.5.1",
"@endo/eslint-plugin": "^1.0.0",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

these 1.0s, will they all be semver? Are Endo maintainers committed to major very bump for all breaking changes? do deep imports count as the package API?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Deep imports are expressly forbidden by an "exports" directive in every public Endo package except captp, far, lockdown, and marshal, and these must be fixed.

Endo has for some time committed to backward-compatibility for dependents using the default carat (^) range, which is more strict than semver.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Tracking the broader issue for Agoric SDK #8679

@erights

erights commented Dec 12, 2023

Copy link
Copy Markdown
Member

This change brings Agoric SDK in sync with the latest versions from Endo as of December 12, 2003.

Wow, that's a long time ago!

@kriskowal kriskowal force-pushed the kris-sync-endo-2023-12-12-20-31-15 branch 8 times, most recently from 3d3a6f3 to 89c2b4b Compare December 20, 2023 21:41
@kriskowal kriskowal requested a review from erights December 20, 2023 22:25

@erights erights left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!!!

getRoundData: M.call(M.any()).returns(M.promise()),
getRoundStatus: M.call(M.any()).returns(M.record()),
oracleRoundStateSuggestRound: M.call(M.any()).returns(M.record()),
authenticateQuote: M.call().rest(M.any()).returns(M.any()),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Better guards would certainly be nice, but can happen after this PR. Thus, I won't withhold approval waiting for this conversation to be resolved.

/** @type {import('.').Zone['exoClass']} */
const exoClass = (...args) => prepareExoClass(baggage, ...args);
/** @type {import('.').Zone['exoClassKit']} */
// @ts-ignore This type check regressed inexplicably with the release

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Likewise, although it would be nice to get this resolved now, it can happen after this PR, so I won't withhold approval waiting for it.

@kriskowal kriskowal force-pushed the kris-sync-endo-2023-12-12-20-31-15 branch from 89c2b4b to 69884dd Compare December 21, 2023 01:34
@kriskowal kriskowal added the automerge:rebase Automatically rebase updates, then merge label Dec 21, 2023
@mergify mergify Bot merged commit 4f72580 into master Dec 21, 2023
@mergify mergify Bot deleted the kris-sync-endo-2023-12-12-20-31-15 branch December 21, 2023 02:08
This was referenced Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge:rebase Automatically rebase updates, then merge force:integration Force integration tests to run on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants