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(ERTP,vats): fix 9407 AmountPatternShape #9863

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

erights
Copy link
Member

@erights erights commented Aug 7, 2024

Staged on #9410

closes: #9407
refs: #9410

Description

Originally #9410 was supposed to fix #9407 , by relaxing an incorrect rejection into a correct acceptance. But we are not yet ready to deploy such a fix on-chain, due to upgrade constraints, so fixing it in agoric-sdk master would be misleading. It might lead people to write other code that depends on this bug being fixed when tested against agoric-sdk master, only to have that code fail later on-chain when invoking the non-upgraded buggy code.

So we unfixed #9410 to restore the buggy behavior, but with a better error message, and refactored to make fixing it again easier. This PR is that fix. It will remain draft until we are confident we can upgrade the relevant on-chain vat (the vbank) in a timely manner. Nevertheless, though we'll keep it in draft until then as a safety measure, this PR is reviewable. Reviewers, please review it.

Security Considerations

Fixing a bug is generally good for security. Otherwise, none.

Scaling Considerations

none

Documentation Considerations

Would remove the need to document the #9407 buggy behavior

Testing Considerations

Restores the test.failing from #9410 to a test.

Upgrade Considerations

as above.

@erights erights self-assigned this Aug 7, 2024
@erights erights requested a review from mhofman August 7, 2024 22:39
Copy link

cloudflare-workers-and-pages bot commented Aug 7, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: f1e91c7
Status: ✅  Deploy successful!
Preview URL: https://c5d74605.agoric-sdk.pages.dev
Branch Preview URL: https://markm-amount-shape-shape-2.agoric-sdk.pages.dev

View logs

Base automatically changed from markm-amount-shape-shape to master August 8, 2024 22:12
@erights erights force-pushed the markm-amount-shape-shape-2 branch 2 times, most recently from 3a34f42 to 41b4317 Compare September 5, 2024 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

faulty amountShape param in interface guard
2 participants