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

fix(zoe): Fix OfferHandlerI.handle returns guard #8748

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

erights
Copy link
Member

@erights erights commented Jan 13, 2024

closes: #XXXX
refs: #8745

Description

At #8745 (comment) @dckc suggests extracting this one fix into a separate change. This PR is that.

An OfferResult is the value returned by a offer handler. E(userSeat).getOfferResult() returns a promise for an OfferResult.

By design, an OfferResult can be any Passable. Indeed, its static type is often the OR of

 * @template {object} [OR=unknown]

which is the type parameter of UserSeat.

However, the OfferResult is so often a string, that the incorrect assumption that it is a string has crept into our code in at least the two places fixed by this PR. One is an incorrect returns guard. The other is an incorrect promise type parameter.

Security Considerations

More accurate types help security review. The too-narrow results guard would only cause inappropriate failures, which at least usually preserve integrity while sacrificing availability. The incorrect promise type arg is unsound in a way that might cause a reviewer to miss a loss of integrity: The @returns {Promise<string>} statically makes a claim that might silently be violated by runtime behavior. (Note though that neither has yet resulted in any known such problem.)

Scaling Considerations

None.

Documentation Considerations

The documentation at https://docs.agoric.com/reference/zoe-api/user-seat.html#e-userseat-getofferresult is already correct. This brings the code into agreement with the existing docs.

More accurate types will also help generate more accurate docs.

Testing Considerations

None.

Upgrade Considerations

None, I think.

@erights erights requested a review from dckc January 13, 2024 23:44
@erights erights self-assigned this Jan 13, 2024
@erights erights force-pushed the markm-fix-offer-handler-interface-guard branch from 9649cd9 to f9202a0 Compare January 13, 2024 23:45
@erights erights added the automerge:squash Automatically squash merge label Jan 13, 2024
@mergify mergify bot merged commit 87e4ef4 into master Jan 14, 2024
74 checks passed
@mergify mergify bot deleted the markm-fix-offer-handler-interface-guard branch January 14, 2024 00:23
anilhelvaci pushed a commit to Jorge-Lopes/agoric-sdk that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants