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

vaultFactory proposal shapes #7094

Merged
merged 3 commits into from
Mar 17, 2023
Merged

Conversation

turadg
Copy link
Member

@turadg turadg commented Mar 1, 2023

refs: #7041

Description

assertProposalShape has deprecated and the replacement is to use a new argument to makeInvitations, which provides better protection for contracts against malformed offers. When an offer's proposal violates the contract's required proposalShape pattern, it is rejected in Zoe and the contract never even sees the malformed offer.

This does that for vaultFactory. While touching these it also specifies the brand in the shape guard. To do this without an E(brand).getAmountShape() remote call it uses a new helper makeNatAmountShape() that can be safely used in Inter Protocol because the amount kinds are known. I put it in inter-protocol's contractSupport so not endorse the usage elsewhere.

Security Considerations

Improves guarding by testing earlier and including the brand.

Scaling Considerations

The shape is now constructed on each makeInvitation instead of once at module scope. If this is a problem it could be memoized.

Documentation Considerations

--

Testing Considerations

CI

Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

/**
* @param {Brand} brand
* @param {AssetKind} assetKind
* @param {Matcher} elementShape
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param {Matcher} elementShape
* @param {Pattern} elementShape

For example, harden({ x: M.bigint() }) is a Pattern but not a Matcher, whereas M.bigint() is both a Pattern and a Matcher.

@turadg turadg force-pushed the ta/vaultFactory-proposal-shapes branch from 0a5c7de to 3ea332d Compare March 1, 2023 18:01
@turadg turadg added automerge:no-update (expert!) Automatically merge without updates bypass:integration Prevent integration tests from running on PR and removed bypass:integration Prevent integration tests from running on PR labels Mar 1, 2023
@Chris-Hibbert
Copy link
Contributor

Can this wait until after #7047?

@turadg turadg removed the automerge:no-update (expert!) Automatically merge without updates label Mar 1, 2023
@turadg turadg mentioned this pull request Mar 9, 2023
@turadg turadg force-pushed the ta/vaultFactory-proposal-shapes branch from 3ea332d to dfbf8c5 Compare March 17, 2023 15:13
@datadog-full-agoric
Copy link

datadog-full-agoric bot commented Mar 17, 2023

Datadog Report

Branch report: ta/vaultFactory-proposal-shapes
Commit report: 1dd942b

agoric-sdk: 0 Failed, 0 New Flaky, 3756 Passed, 53 Skipped, 16m 22.14s Wall Time

@turadg turadg added the automerge:no-update (expert!) Automatically merge without updates label Mar 17, 2023
@mergify mergify bot merged commit 4bd83a6 into master Mar 17, 2023
@mergify mergify bot deleted the ta/vaultFactory-proposal-shapes branch March 17, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:no-update (expert!) Automatically merge without updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants