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

Implement new factory #106

Draft
wants to merge 31 commits into
base: handle-contract-versions
Choose a base branch
from
Draft

Implement new factory #106

wants to merge 31 commits into from

Conversation

contracts/CPKFactory.sol Outdated Show resolved Hide resolved
contracts/CPKFactory.sol Outdated Show resolved Hide resolved
contracts/CPKFactory.sol Outdated Show resolved Hide resolved
contracts/CPKFactory.sol Outdated Show resolved Hide resolved
contracts/CPKFactory.sol Outdated Show resolved Hide resolved
contracts/CPKFactory.sol Outdated Show resolved Hide resolved
contracts/CPKFactory.sol Outdated Show resolved Hide resolved
@cag
Copy link
Contributor Author

cag commented Dec 29, 2020

Ugh, this is still not ready, and generalizing setup straight up made an obvious security vulnerability, which I realized while thinking about the implication of this design, which would in any case require a signature and a transaction authorization on MM (so two user interactions per batch instead of the one that is the case right now). Thought about tx.origin but that opens up a can of phish.


Okay, I'll resolve this by requiring the owner to either be the msg.sender or to have signed the creation params.


Note to self: rename from facade to adapter because that contract now has a more complicated interface and it's only used to ease working with the Safe relay.

@cag cag marked this pull request as ready for review January 6, 2021 18:58
@cag cag requested a review from rmeissner January 6, 2021 18:58
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.

2 participants