fix(zoe): Fix guards to accurately guard args#8642
Conversation
b7ed892 to
331944c
Compare
9b8878b to
331944c
Compare
kriskowal
left a comment
There was a problem hiding this comment.
A test that would have previously failed would hedge against regression.
331944c to
4c935b5
Compare
Done. Since this PR is currently tested against an endo prior to endojs/endo#1765 , the test's difference in behavior is in what error message is generated. This same test, prior to the rest of this PR while still prior to endojs/endo#1765 emits an error message from the operation itself. Whereas with the rest of this PR, we're getting the error from the guard mismatch. After agoric-sdk is updated to current endo, i.e., one post endojs/endo#1890 , then this test without the rest of this PR would still fail from a guard mismatch, but still with a different error: that there were too many arguments. |
|
@erights - This PR appears to be stuck. It's had a merge label for > 24 hours |
1 similar comment
|
@erights - This PR appears to be stuck. It's had a merge label for > 24 hours |
4c935b5 to
3016a9a
Compare
closes: #XXXX
refs: #8641 #8514 endojs/endo#1765 endojs/endo#1890
Description
#8641 throws errors when a call has more arguments than are declared in a method guard. These throws have diagnosed several errors that were otherwise undiagnosed, even after endojs/endo#1765 . After endojs/endo#1765 these mismatches would have been genuine observable runtime bugs that remained undiagnosed by CI, as seen by the clean CI run at #8514 .
This PR reproduces the bug fixes from #8641 by themselves, without the endo-branch directive or the rest of #8514 , testing that these fixes are also compat with current agoric-sdk and should be merged as is.
Security Considerations
More accurate arg guarding supports better input validation.
Scaling Considerations
None
Documentation Considerations
For the methods whose guards are fixed here, we should also check whether the documentation of those methods include those args.
Testing Considerations
The fact that the bugs fixed in this PR were not detected by #8514 shows that endojs/endo#1765 was too hazardous: It would introduce changes in behavior that could be unlikely to be detected in tests, but be genuine dynamic bugs.
Upgrade Considerations
See the Upgrade Considerations of endojs/endo#1890