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

add eip-zodiac #5005

Merged
merged 17 commits into from
May 28, 2022
Merged

add eip-zodiac #5005

merged 17 commits into from
May 28, 2022

Conversation

auryn-macmillan
Copy link
Contributor

@auryn-macmillan auryn-macmillan commented Apr 14, 2022

This PR adds an EIP for Zodiac, a standard for composable DAO tooling.

@eth-bot
Copy link
Collaborator

eth-bot commented Apr 14, 2022

All tests passed; auto-merging...

(pass) eip-5005.md

classification
updateEIP
  • passed!

EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
Copy link
Contributor

@wschwab wschwab left a comment

Choose a reason for hiding this comment

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

Very excited to see this! I've made some suggestions cleaning up the header, and made a couple of other points.

EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
EIPS/eip-zodiac.md Outdated Show resolved Hide resolved
Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

I get the sense that this shouldn't be an EIP. Can you elaborate on the benefit of standardizing this ERC would be vs. developing the philosophy as a standalone thing? Also, you will need to remove the external links to Zodiac. EIPs should be able to stand on their own.

@auryn-macmillan
Copy link
Contributor Author

Can you elaborate on the benefit of standardizing this ERC would be vs. developing the philosophy as a standalone thing?

ERCs are a Schelling point in the Ethereum ecosystem. If you're building something that benefits from composibility (most things in web3), then it makes sense to build it in such a way that it conforms to whatever most relevant standards are. So the motivation for standardizing this as an ERC really just to help solidify it as a credibly neutral standard in the Ethereum ecosystem. It feels much more likely to be widely used, and thus beneficial to the ecosystem, as an ERC than as a standalone standard.

Also, you will need to remove the external links to Zodiac. EIPs should be able to stand on their own.

No worries, will remove those links.

@MicahZoltu
Copy link
Contributor

The litmus test I usually use for deciding whether something should be a standard or not is whether or not there is a many-to-many relationship between components of a system. Twitter's API doesn't need to be standardized because there is a single provider and many clients (one to many). Browsers on the other hand need standards because there are many web pages and many browsers (many to many).

What are the components on each side of a particular communication channel in the proposed system?

@auryn-macmillan
Copy link
Contributor Author

The litmus test I usually use for deciding whether something should be a standard or not is whether or not there is a many-to-many relationship between components of a system.

This seems like a reasonable litmus test.

What are the components on each side of a particular communication channel in the proposed system?

IAvatar.sol is the core interface. It was prototyped on the Gnosis Safe's ModuleManager.sol, simply because that was the natural choice being the most widely used contract wallet. That said, it was a conscious decision to label it Avatar, rather than something explicitly Gnosis Safe related, because we wanted to make it explicit that this is an interface we fully expect other programmable accounts to leverage.

On the other side of the interface, modules can be any contract that can call execTransactionFromModule() or execTransactionFromModuleReturnData() on an Avatar. This could be just about any of the existing DAO frameworks or any of a number of modules that have been written to comply with the standard already.

The intent is definitely for it to be a many-to-many relationship between components, where each component should be mostly agnostic the specific implementation of the other components.

EIPS/eip-5005.md Outdated Show resolved Hide resolved
Co-authored-by: Pandapip1 <[email protected]>
EIPS/eip-5005.md Outdated Show resolved Hide resolved
EIPS/eip-5005.md Outdated Show resolved Hide resolved
@@ -0,0 +1,225 @@
---
eip: 5005
title: Zodiac
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
title: Zodiac
title: Zodiac Avatar Accounts and Extensions

I feel like the core contribution of this EIP is Avatar accounts, and I think "extensions" covers modifiers, modules, and guards well enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer not to introduce additional terminology that isn't used elsewhere.

If we were to add anything to the title, I'd be inclined to go with something like "Zodiac Avatar Accounts and Mods".

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, how about just Zodiac Avatar Accounts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That seems reasonable as well.
One thing I have been conscious of lately is that this pattern is useful beyond just DAOs. So I wonder if it would be worthwhile changing the rest of the language to be less opinionated about how it is used / what it is used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this PR to generalize the language. This feels like the right direction to me.
https://github.com/auryn-macmillan/EIP-Zodiac/pull/1

EIPS/eip-5005.md Outdated Show resolved Hide resolved
EIPS/eip-5005.md Outdated Show resolved Hide resolved
EIPS/eip-5005.md Outdated Show resolved Hide resolved
EIPS/eip-5005.md Outdated Show resolved Hide resolved
EIPS/eip-5005.md Outdated Show resolved Hide resolved
EIPS/eip-5005.md Outdated Show resolved Hide resolved
Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

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

I think this is good to merge as a draft. I think the new title and generalization PR is a good idea, but that can get merged after.

If the bot doesn't merge this, try opening and closing the PR.

@eth-bot eth-bot enabled auto-merge (squash) May 27, 2022 18:56
EIPS/eip-5005.md Outdated Show resolved Hide resolved
@eth-bot eth-bot merged commit f480f73 into ethereum:master May 28, 2022
nachomazzara pushed a commit to nachomazzara/EIPs that referenced this pull request Jan 13, 2023
* add eip-zodiac

* Add security considerations and some minor fixes

* Add ETHMagicians discussion link

* Update EIPS/eip-zodiac.md

Co-authored-by: William Schwab <[email protected]>

* Update EIPS/eip-zodiac.md

Co-authored-by: William Schwab <[email protected]>

* Update EIPS/eip-zodiac.md

Co-authored-by: William Schwab <[email protected]>

* Update EIPS/eip-zodiac.md

Co-authored-by: William Schwab <[email protected]>

* Remove external links to Zodiac & external imports

* Rename EIP file to add ERC number

* Update EIPS/eip-5005.md

Co-authored-by: Pandapip1 <[email protected]>

* Update EIPS/eip-5005.md

Co-authored-by: Sam Wilson <[email protected]>

* Update EIPS/eip-5005.md

Co-authored-by: Sam Wilson <[email protected]>

* Remove external links

Co-authored-by: Sam Wilson <[email protected]>

* Fix gramma

* Small sentance structure fix

* Remove license and external links

* Update EIPS/eip-5005.md

Co-authored-by: William Schwab <[email protected]>
Co-authored-by: Pandapip1 <[email protected]>
Co-authored-by: Sam Wilson <[email protected]>
Co-authored-by: Micah Zoltu <[email protected]>
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.

8 participants