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

ERC-3000: Optimistic enactment governance standard #3000

Merged
merged 8 commits into from
Oct 14, 2020

Conversation

izqui
Copy link
Contributor

@izqui izqui commented Sep 24, 2020

🚧 Under construction 🚧

Check spec

@izqui izqui marked this pull request as draft September 25, 2020 03:10
@izqui izqui changed the title EIP X: On-chain enactment for off-chain voting ERC-3000: On-chain enactment for off-chain voting Sep 25, 2020
@izqui izqui changed the title ERC-3000: On-chain enactment for off-chain voting ERC-3000: On-chain enactment for off-chain votes Sep 25, 2020
@izqui izqui changed the title ERC-3000: On-chain enactment for off-chain votes ERC-3000: On-chain enactment of off-chain votes standard Sep 25, 2020
@izqui izqui changed the title ERC-3000: On-chain enactment of off-chain votes standard ERC-3000: Binding off-chain voting standard Sep 25, 2020
@MicahZoltu
Copy link
Contributor

@izqui Discussed this in EIPIP meeting. Please add content to this EIP (i.e., get it into draft) ASAP if you want to keep this number.

@izqui
Copy link
Contributor Author

izqui commented Oct 7, 2020

@MicahZoltu alright, will do before the end of the week. Thanks for the notice

@izqui izqui marked this pull request as ready for review October 12, 2020 20:52
@izqui izqui changed the title ERC-3000: Binding off-chain voting standard ERC-3000: Optimistic enactment standard Oct 12, 2020
@izqui izqui changed the title ERC-3000: Optimistic enactment standard ERC-3000: Optimistic enactment governance standard Oct 12, 2020
@izqui
Copy link
Contributor Author

izqui commented Oct 12, 2020

Ready for review and merging an initial draft @MicahZoltu

Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

The discussion-to change is required before merging as a draft. The rest are either suggestions or may be required changes prior to moving this to Review (the phase after Draft).

EIPS/eip-3000.md Outdated
eip: 3000
title: Optimistic enactment governance standard
author: Jorge Izquierdo (@izqui)
discussions-to: https://github.com/ethereum/EIPs/pulls/3000
Copy link
Contributor

Choose a reason for hiding this comment

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

Discussion cannot be the pull request itself. The recommended location for discussion-to links is https://ethereum-magicians.org/ but creating a separate GitHub issue is also an option if you don't want to create a thread there.

Comment on lines +14 to +18
ERC-3000 presents a basic on-chain spec for contracts to optimistically enact governance decisions made off-chain.

The standard is opinionated in defining the 6 entrypoint functions to contracts supporting the standard. But it allows for any sort of resolver mechanism for the challenge/response games characteristic of optimistic contracts.

While the authors currently believe resolving challenges [using a subjective oracle](https://aragon.org/blog/snapshot) is the right tradeoff, the standard has been designed such that changing to another mechanism is possible (a deterministic resolver like [Optimism's OVM](https://optimism.io) uses), even allowing to hot-swap it in the same live instance.
Copy link
Contributor

Choose a reason for hiding this comment

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

The Simple Summary should be quite short. Think email subject line, forum post sub-heading, or a blurb in the last line of a table. The second paragraph here is more appropriate for the Abstract, and the third paragraph is more appropriate for the Motivation.

Also consider not using the EIP number inline in the EIP (it is redundant) and keeping the tense as a technical document assertion.

Suggested change
ERC-3000 presents a basic on-chain spec for contracts to optimistically enact governance decisions made off-chain.
The standard is opinionated in defining the 6 entrypoint functions to contracts supporting the standard. But it allows for any sort of resolver mechanism for the challenge/response games characteristic of optimistic contracts.
While the authors currently believe resolving challenges [using a subjective oracle](https://aragon.org/blog/snapshot) is the right tradeoff, the standard has been designed such that changing to another mechanism is possible (a deterministic resolver like [Optimism's OVM](https://optimism.io) uses), even allowing to hot-swap it in the same live instance.
Interface for scheduling, executing and challenging contract executions.

EIPS/eip-3000.md Outdated
Comment on lines 20 to 60
## Data structures

Some data structures are definied which are later used in the standard interfaces:

```solidity
library ERC3000Data {
struct Container {
Payload payload;
Config config;
}

struct Payload {
uint256 nonce;
uint256 executionTime;
address submitter;
IERC3000Executor executor;
Action[] actions;
bytes proof;
}

struct Action {
address to;
uint256 value;
bytes data;
}

struct Config {
uint256 executionDelay;
Collateral scheduleDeposit;
Collateral challengeDeposit;
Collateral vetoDeposit;
address resolver;
bytes rules;
}

struct Collateral {
address token;
uint256 amount;
}
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't a standard section, and while non-standard sections aren't explicitly prohibited, they are frowned on. In this case, this is very much part of the specification and should be a sub-section of the specification.

EIPS/eip-3000.md Outdated
event Configured(bytes32 indexed containerHash, address indexed actor, ERC3000Data.Config config);
}
```
*See [up to date source](https://github.com/aragon/govern/blob/master/packages/erc3k/contracts/IERC3000.sol)*
Copy link
Contributor

Choose a reason for hiding this comment

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

Specifications should be standalone documents that do not depend on external documents. Consider either inlining linked code, or adding it as an asset in this repository at ../assets/eip-3000/IERC3000.sol.

EIPS/eip-3000.md Outdated

### 1. Aragon Govern

- [ERC-3000 spec (interfaces)](https://github.com/aragon/govern/blob/master/packages/erc3k)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the spec, anything else is not the spec but just a copy/mirror/implementation/derivative.

EIPS/eip-3000.md Outdated
Comment on lines 129 to 130
- [ERC-3000 spec (interfaces)](https://github.com/aragon/govern/blob/master/packages/erc3k)
- [Implementation](https://github.com/aragon/govern/blob/master/packages/govern-core)
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned above, wherever possible content should be inlined and not linked externally. This is especially true when the linked content is so directly correlated with this content and the license is different. It is better to not link to anything at all than to link to GPL licensed software because at least that way anyone who implements from this spec alone (without seeing the GPL code) can plausibly assert that they did not see the code, and thus did not copy that code. Linking to code that is copy left licensed risks users foot-gunning themselves by clicking through to the link.

EIPS/eip-3000.md Outdated

## Security Considerations

A ERC-3000 configuration specifies the resolver for challenges over scheduled items.
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
A ERC-3000 configuration specifies the resolver for challenges over scheduled items.
Configuration specifies the resolver for challenges over scheduled items.

@izqui
Copy link
Contributor Author

izqui commented Oct 13, 2020

Thanks for the thorough review @MicahZoltu. All concerns should have been addressed!

Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

Just needs a Rationale section to be merged as draft. I still recommend heading the other feedback and getting merged to Review is a bit harder (more strict requirements), but all of that can wait until after Draft is merged except this one.

EIPS/eip-3000.md Outdated
}
```

## Implementations
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
## Implementations
## Rationale
TBD
## Implementations

Rationale is a required section. It should focus on stating the reason why specific decisions were made over alternate choices. Some of the content currently is the Motivation would be more appropriate in the Rationale.

@izqui
Copy link
Contributor Author

izqui commented Oct 14, 2020

Alright @MicahZoltu, ready to go now!

Thanks for the review, and I will make further changes before asking to change to Review.

@MicahZoltu MicahZoltu merged commit 92ed7c2 into ethereum:master Oct 14, 2020
Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 6, 2021
Interface for scheduling, executing and challenging contract executions based on off-chain approval
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