LIP-25: Extensible Governance Contract

[kyriediculous]

Extensible Governance Contract

Abstract

This proposal describes an extensible governance system that can be used to update the parameters and code of the Livepeer protocol smart contracts and establish a basis for a clear upgrade path for the governance system to give stakeholders control over the system.

Motivation

At the moment, a core developer owned multisig has admin privileges over the Livepeer protocol, giving the core developers the ability to update contract code and parameters.

The goal for an extensible governance contract is to establish the technical foundation that allows for a variety of sophisticated binding voting systems to be implemented in the future.

After deployment, ownership of the Controller contract will be transferred to the extensible governance contract which will still have a core developer owned multisig as it's main and only actor until control can be phased to binding voting systems in later milestones of the governance roadmap.

From a high level this extensible governance system should fulfill following properties:

• Enable on-chain access rights allowing particular actors to execute (particular) code/parameter updates
• Allow future changes to the governance structure
• Enable on-chain rules for how code/parameters can be updated, e.g. delayed execution.
• Facilitate atomic execution of multiple upgrades

Specification

The extensible governance system will be a Smart Contract deployed on the Ethereum blockchain consisting of following components

• Access Control List
• Staged Execution
• Batched Execution

Access Control List

The Access Control List contains rules that grant or deny access to certain actors to execute code/parameter updates in the Livepeer protocol.

This mechanism grants access based on Ethereum addresses, whether that be an EOA, smart contract or multisig. The access rights can be for either the entire scope of the protocol or modular (cfr. root and users).

Example Implementation

An example implementation could bind actors other than owner to contract methods at specific target addresses. E.g. Binding Voting System 1 is allowed to call setRoundLength()on the RoundsManager and setUnbondingPeriod() on the BondingManager.

contract ACL is Ownable {

    /// @dev Actor => Contract => Function signature
    mapping(address => mapping(address => mapping(bytes4 => bool))) _accessRights;

    /**
    * @dev check whether msg.sender has access rights to execute '_data' at '_target'
    * @param _actor actor to check rights for
    * @param _target target contract address
    * @param _method 4-byte identifier of the method
    * @return true/false whether 'msg.sender' has sufficient rights
    */
    function accessRights(address _actor, address _target, bytes4 _method) public view returns (bool) {
        return _actor == owner || _accessRights[_actor][_target][_method];
    }

    /**
    * @dev set access rights for for an Ethereum address
    * @param _actor ethereum address to grant access rights
    * @param _target target contract address '_actor' is allowed to call
    * @param _methods list of methods on '_target' contract the '_actor' is allowed to call
    * @param _access list of booleans indicating access rights corresponding to the list of '_methods'
    */
    function setAccessRights(address _actor, address _target, bytes4[] memory _methods, bool[] memory _access) public {
        // Check that 'msg.sender' has sufficient rights to alter ACL rules
        require(accessRights(msg.sender, address(this), getMethodSignature(msg.data)), "access denied");
        for (uint256 i = 0; i < _methods.length; i++) {
            _accessRights[_actor][_target][_methods[i]] = _access[i];
        }
    }

    function getMethodSignature(bytes memory _data) public pure returns (bytes4 method) {
        assembly {
            method := mload(add(_data, 0x20))
        }
    }
}

Staged Updates & Delayed Execution

One potential rule for the governance contract to establish is a time delay for executing governance actions to give all stakeholders a clear view on pending updates and to allow those who disagree with the update to exit the protocol prior to execution of the update. Updates can be stored as state on-chain before execution providing transparency for stakeholders. Governance actions can be parameter updates, code changes, updating access rights or updating time delay.

Time delay can be defined by a parameter, DELAY: Number of blocks by which to delay execution of a staged update.

DELAY can be either a global or modular parameter, some benefits for a modular parameter, e.g. per actor basis/per function basis, include:

• facilitate longer delays for 'sensitive' updates
• allow quicker updates by the core team in the initial stage
• enable more dynamic protocol pausing/unpausing mechanics (e.g. core team can still initially pause on a very small delay but community can submit a proposal to unpause again on a longer delay)

The exact implementation is still open for discussion but there's several options:

• Role-based access control
• Simple nested map: map[function sig]map[address]delay

contract Staging is ACL {

    struct Update {
        address target;
        bytes data;
    }

    struct StagedUpdate {
        Update[] updates;
        uint256 timelock;
    }

    /// @dev UpdateStaged is emitted when an actor stages an update
    event UpdateStaged(address indexed actor, uint256 id, StagedUpdate stagedUpdate);

    /// @dev delays per actor
    mapping(address => uint256) delays;
    /// @dev staged updates
    mapping(uint256 => StagedUpdate) public stagedUpdates;
    /// @dev current update count
    uint256 public updateCount;

    /**
    * @dev stage an update for future execution.
    * @param _updates a list of updates to be executed.
    * @notice The entity staging the update must be allowed.
    */
    function stageUpdate(Update[] memory _updates) public {
        StagedUpdate storage stagedUpdate = stagedUpdates[updateCount];
        for (uint256 i = 0; i < _updates.length; i++) {
            require(accessRights(msg.sender, _updates[i].target, getMethodSignature(_updates[i].data)), "access denied");
            stagedUpdate.timelock = block.number + delays[msg.sender];
            stagedUpdate.updates.push(Update({target: _updates[i].target, data: _updates[i].data}));
        }
        emit UpdateStaged(msg.sender, updateCount, stagedUpdate);
        updateCount++;
    }

    /**
     * @dev set a update execution time delay for an actor
     * @param _actor ethereum address of the actor to set a delay for
     * @param _delay number of blocks to delay updates for
     * @notice will revert if 'msg.sender' is not authorized to call this method
     */
    function setDelay(address _actor, uint256 _delay) public {
        require(accessRights(msg.sender, address(this), getMethodSignature(msg.data)), "access denied");
        if (_delay == 0) {
            delete delays[_actor];
        } else {
            delays[_actor] = _delay;
        }
    }
}

Batched Execution

Batched execution allows multiple code/parameter updates to be bundled into a single on-chain action. This can reduce the complexity and number of steps required for a protocol upgrade that consists of multiple proposals/changes.

Since each parameter change is essentially a contract method call, the raw transaction data can be calculated before actually submitting the transaction to the target contract. Updates can then be executed as a batch as long as the combined set of method calls does not exceed the Ethereum block gas limit.

contract Execution is ACL, Staging {

    /// @dev UpdateExecuted is emitted when a staged update has been fully executed
    event UpdateExecuted(address indexed actor, StagedUpdate update);

    /**
    * @dev Execute a staged update.
    * @notice Updates are authorized during staging.
    * @notice Reverts if a transaction can not be executed.
    * @param  _id id of the staged update.
    */
    function executeUpdate(uint256 _id) public {
       StagedUpdate storage stagedUpdate = stagedUpdates[_id];
       require(block.number > stagedUpdate.timelock, "time delay for update not expired");
        for (uint256 i = 0; i < stagedUpdate.updates.length; i++) {
            (bool success,) = stagedUpdate.updates[i].target.call(stagedUpdate.updates[i].data);
            require(success, "could not execute update");
        }
        emit UpdateExecuted(msg.sender, stagedUpdate);
        delete stagedUpdates[_id];
    }
}

EthFiddle

The prototype contracts are available at https://ethfiddle.com/2iCPShA1pv

Upgrade Path

The governance contract's design rationale should establish a clear technical foundation to lay the groundwork for an upgrade path to give control over protocol code/parameters to the stakeholders. This section provides an example of what such an upgrade path might look like without setting explicit milestones.

1. Deploy governance contract with the core team multisig as owner.
2. Make core team multisig subject to a time delay for most actions (e.g. it might still be interesting to allow the core team to retain the right to pause/unpause the protocol in the early stages)
3. Grant access to a binding voting system to alter less sensitive protocol parameters
4. Hand over control of all protocol parameters to binding voting systems
5. Allow binding voting systems to execute code updates
6. Deploy a binding voting system to govern the governance contract's ACL
7. Remove core team multisig as owner and have ACL rules be established by a binding voting system

In scope For this proposal would be step 1 and step 2

Step 1 would be completed upon deployment of the Extensible Governance System. Step 2 will be decided by the outcome of the initial values discussion here.

Specification Rationale

A modular ACL list that is actor agnostic allows for maximum extensibility to the governance structure and abstracts any details of how a binding voting systems should look like. It merely sets the foundation through which different actors can execute updates to protocol code or parameters.

It further enables different types of actors or different types of voting systems to have control over different parts of the protocol. Different parameters might require different systems, e.g. higher QUORUM and THRESHOLD values or different voting schemes than tokenholder voting.

Facilitating executing batches of staged updates simplifies the upgrade path for both EOA and contract actors and provides full transparency to all stakeholders as to all staged updates, their actors and time delay.

A modular ACL and time delay allows for interesting mechanics and a clear governance upgrade path to hand control over to binding voting systems. The core team can initially retain owner access over the protocol contracts but updates subject to the LIP process can already be subject to a time delay as well. In future milestones control over certain parameters or code updates can be phased over to the community through a binding voting system.

Initial Parameter Values

There's mainly two initial values that need to be set

• The initial master actor
• The DELAY for the first actor, if applicable

It's established already that the first actor of this governance system will be the Livepeer Inc Multisig. Additional actors can be added over time through the Livepeer Governance process.

The DELAY parameter, when opting for a modular or role-based system can differ based on the action being taken. To separate design and initial value discussions a proposal for the initial values can be found here

Copyright

Copyright and related rights waived via CC0.

[dob]

Is it safe to say that this proposed implementation is entirely generic and there is nothing at all Livepeer protocol specific? (A good thing). Abstracted, it is essentially an owner-updated ACL with logic for proposing staged updates and them batching (or not batching) the execution of them.

One thing that would be important to think through is whether an upgrade mechanism for this contract itself is needed? One idea I can think of is that a new governance contract be deployed, and then this contract would have the permission to call setOwner on the Controller, to point to the new contract. If the owner of the governance contract is the voting protocol, then the community could invoke these upgrades through the LIP + Polling process.

[kyriediculous]

Is it safe to say that this proposed implementation is entirely generic and there is nothing at all Livepeer protocol specific? (A good thing). Abstracted, it is essentially an owner-updated ACL with logic for proposing staged updates and them batching (or not batching) the execution of them.

That is entirely correct.

One thing that would be important to think through is whether an upgrade mechanism for this contract itself is needed? One idea I can think of is that a new governance contract be deployed, and then this contract would have the permission to call setOwner on the Controller, to point to the new contract. If the owner of the governance contract is the voting protocol, then the community could invoke these upgrades through the LIP + Polling process.

I think you describe it well. The process for upgrading this contract would be entirely the same as for instantiating this one in the first place as that requires no additional engineering work. After the entire LIP process we would just have to change the Controller owner. For upgrades this can be a binding voting contract in the future. Because of the sensitive nature of this operation and the modular design of the ACL this can be a separate voting system from other LIPs even.

[yondonfu]

This is great! Thanks for writing this up. I've categorized my feedback by section below.

Access Control List

In this section's diagram, I think the following functions are missing:

• Controller
  • pause()
  • unpause()
  • transferOwnership()
• Minter
  • migrateToNewMinter()

I think adding these functions to the diagram would be helpful for readers to understand the full scope of contract functions that can be called by the governance contract.

Since access rights configuration is also a governance action I think its worth considering applying the same staged update rules to the setAccessRights() function i.e. it is subject to a delay and can be included in a batch update. One way to accomplish this may be to require the msg.sender in setAccessRights() to be the governance contract itself and an actor with access rights to call this function can stage an update for it.

Staged Updates & Delayed Execution

Since update delay configuration is also a governance action I think its worth considering applying the same staged update rules to the setDelay() function i.e. it is subject to a delay and can be included in a batch update. I think this could be accomplished using the same approach used for applying staged update rules to the setAccessRights() function.

Updates can be stored as state on-chain before execution providing transparency for stakeholders.

It may be worth considering also including the value (i.e. the amount of ETH) to attach to the update. None of the functions that can be called right now require ETH, but supporting attaching a value would make this update process more "complete" since any type of function call including those that require value to be included would be supported. We would also need to make the execution function for the staged update to be payable so the required amount of ETH can be provided at execution time.

DELAY can be either a global or modular parameter, some benefits for a modular parameter, e.g. per actor basis, include:

Great point. I was thinking about whether it would make sense for delays to be on a per function basis. For example, one actor would be able to call contract parameter update functions with a 3 day delay, but would be able to call the pause/unpause functions with a 0 day delay. This configuration wouldn't be achievable with a per actor delay though we could achieve something similar that is not quite the same by specifying a separate actor for contract parameter update functions and a separate actor for pause/unpause functions.

Two possible approaches for achieving the example configuration (which I think is also realistic) are:

• Setting delays per function. The downside with this approach is that if the delay for all contract parameter update functions for an actor should be X then that integer is stored once for each function.
• Use role based access control (RBAC). There could be a ParamSetter role and a Pauser role. The ParamSetter role would have a delay of X and would be able to call all contract parameter update functions. The Pauser role would have a delay of 0 and would only be able to call pause/unpause functions. Both of these roles could be granted to the same actor. See the OpenZeppelin docs for example usage.

General Comments

Is the proposed name for the governance contract ACL? In my mind, an ACL wouldn't perform actions and the governance contract itself is not an ACL, but it has an ACL. What do you think about calling it the Governor?

The current proposal doesn't include a recommendation for initial values for parameters such as the update delay which makes sense since the design is still being fleshed out. I think it would make sense to include the initial values along with the rationale for them in a separate proposal that can have its own discussion thread.

[kyriediculous]

Is the proposed name for the governance contract ACL? In my mind, an ACL wouldn't perform actions and the governance contract itself is not an ACL, but it has an ACL. What do you think about calling it the Governor?

Sure, haven't settled on naming.

From the prototype btw:

contract Governance is ACL, Staging, Execution {

    function setAccessRightsAndDelay(address _actor, address _target, bytes4[] memory _methods, bool[] memory _access, uint256 _delay) public {
        super.setAccessRights(_actor, _target, _methods, _access);
        super.setDelay(_actor, _delay);
    }
}

I think it would make sense to include the initial values along with the rationale for them in a separate proposal that can have its own discussion thread.

I will create a new issue for this separate discussion before submitting a PR for this draft LIP.

[yondonfu]

Note: The following is an editorial comment

A draft LIP has been created containing updates to the original post. The contents of the draft LIP should be used as the basis for further discussion.

[kyriediculous]

The current spec for LIP-25 includes an access control list. In subsequent LIP calls the usage of Role Based Access Control (RBAC) was discussed over the current proposal which describes an Address Based Access Control mechanism but isn't exclusive to one or the other.

Let's however reconsider the requirements for governance milestone 2

Deploy a technically extensible governance system that can be used to update the parameters and code of protocol smart contracts

There should be a clear path for upgrading the governance system to give stakeholders control over the system

The second requirement describes a clear upgrade path to give stakeholders control over the system or in other words remove the Livepeer inc Multisig as the sole unrestricted admin over the system.

While an ACL could definitely fulfill that requirement, they are not mutually exclusive. Having the ability to either change ownership of the governance contract described in this spec to an ACL contract, or redeploy the governance contract with an ACL included should already provide a clear upgrade path of phasing control away from the Livepeer Inc. Multisig.

Given that the governance contract precedes the actual implementation of a binding voting system and the development cycle that would need to go through, the Livepeer Inc multisig will probably remain the administrator of the protocol for the considerable future guided by the non-binding voting system from milestone 1.

It's also unclear at this stage if there will be multiple actors, while the current design is generic enough to accommodate for those needs, it is actually not certain if it will be necessary.

Rather than including a possible premature ACL design and making a decision regarding implementation details (such as RBAC or ABAC) it seems better to shelf including an ACL as part of this spec for now until it is clear whether we'll need such a feature. Although it could seem likely, this would give us more time to design such a system with a little more knowledge about the actual requirements for it. This seems reasonable given that there is still a clear upgrade path for giving control to the stakeholders.

Delayed execution can still be part of the system but a simpler that fits the current needs of the protocol:

• Pause/Unpause and contract upgrades on no delay
  • Param changes on a delay slightly longer than the unbonding period

This would allow for agile bug fixing and pausing the protocol in case user funds should be at risk due to a possible bug, but still provide some restrictions on parameter updates in terms of time delay.

---

This topic will be discussed during the next LIP call ( https://github.com/livepeer/community-governance/issues/3 )

[kyriediculous]

There were quite a few changes recently to the original spec.

• Removal of ACL structure
• Delay is now social convention
• Implementation changes
• API changes

Underneath is the latest version of the spec

---

Abstract

This proposal describes an extensible governance system that can be used to update the parameters and code of the Livepeer protocol smart contracts and establish a basis for a clear upgrade path for the governance system to give stakeholders control over the system.

Motivation

At the moment, a core developer owned multisig has admin privileges over the Livepeer protocol, giving the core developers the ability to update contract code and parameters.

The goal for an extensible governance contract is to establish the technical foundation that allows for a variety of sophisticated binding voting systems to be implemented in the future.

After deployment, ownership of the Controller contract will be transferred to the extensible governance contract which will still have a core developer owned multisig as it's main and only actor until control can be phased to binding voting systems in later milestones of the governance roadmap.

From a high level this extensible governance system should fulfill following properties:

• Enable on-chain access rights allowing particular actors to execute (particular) code/parameter updates
• Allow future changes to the governance structure
• Enable on-chain rules for how code/parameters can be updated, e.g. delayed execution.
• Facilitate atomic execution of multiple upgrades

Specification

The extensible governance system will be a Smart Contract deployed on the Ethereum blockchain consisting of following components

• Access Control
• Staged Execution
• Batched Execution

Types

struct update {
    address target;
    uint256 value;
    bytes data;
    uint256 nonce;
}

API

owner() returns (address)

Returns the address of the current owner of the Governance contract.

transferOwnership(address newOwner)

Transfers the ownership of the Governance contract to a different address, can only be called by the current owner.

nonce() returns (uint256)

Returns the nonce of the latest staged update.

stage(update[] _updates, bytes[] _sig, uint256 _delay)

Stage an update for execution, can consist of multiple separate updates to be executed atomically as a batch. The array of signatures should be respective to update which it matches. The updates have to be signed by the contract owner.

execute(update[] _updates)

Execute updates that were previously staged for execution, reverts if the delay has not expired.

Events

• UpdateStaged(update[] updates, uint256 delay)
• UpdateExecuted(update[] updates)
• OwnershipTransferred(address indexed previousOwner, address indexed newOwner)

Access Control

The new Governance contract, further referred to as Governor will take over ownership of the currently deployed Controller contract. The Controller in it's turn has ownership and a registry of the deployed components in the Livepeer Protocol (e.g. BondingManager, Minter) so it acts as a proxy through which the Governor can execute contract upgrades or parameter updates.

Initially ownership of the Governor will reside with the Livepeer Inc. Multisig e.g. using the basic 'Ownable' contract by OpenZeppelin.

Future iterations could include more complex access control mechanisms such as Role-Based Access Control if the requirements should dictate it. In the end it's a matter of who has ownership over the Governor. This can be an EOA, Multisig, Binding Voting Contract or some sort of Access Control List Contract.

The ability to either

• Changing ownership of the Governor

• Changing owership of the Controller to a new Governor

Should provide a clear upgrade path for eventually handing control over to the stakeholders of the Livepeer protocol without a necessity for a more complex ACL design to be in scope for this spec.

Staged Updates & Delayed Execution

One potential rule for the governance contract to establish is a time delay for executing governance actions to give all stakeholders a clear view on pending updates and to allow those who disagree with the update to exit the protocol prior to execution of the update. Updates can be stored as state on-chain before execution providing transparency for stakeholders. Governance actions can be parameter updates, code changes, updating access rights or updating time delay.

We can however distinguish separate reasons to upgrade and they would have different delay requirements:

• critical bug fixes
• pausing the protocol when an exploit is discovered that puts user funds at risks
• parameter updates
• code upgrades

The first two actions would be considered critical responses to incidents that could potentially occur. Even though the probability of serious incidents would be considered low following the Streamflow security audit, it would still be preferable for responses to incidents to take place on no delay in this iteration of the Governance system. Neither are these actions subject to the LIP process, which provides an additional reason.

The delay would be by social convention as long as there is no binding voting system and only Livepeer inc. would be able to stage updates. This should allow for sufficient flexibility to do bug fixes and pausing the protocol while providing ample possibility for transparancy and community scrutiny when the social convention is not respected by Livepeer inc.

Batched Execution

Batched execution allows multiple code/parameter updates to be bundled into a single on-chain action. This can reduce the complexity and number of steps required for a protocol upgrade that consists of multiple proposals/changes.

Since each parameter change is essentially a contract method call, the raw transaction data can be calculated before actually submitting the transaction to the target contract. Updates can then be executed as a batch as long as the combined set of method calls does not exceed the Ethereum block gas limit.

Upgrade Path

The governance contract's design rationale should establish a clear technical foundation to lay the groundwork for an upgrade path to give control over protocol code/parameters to the stakeholders. This section provides an example of what such an upgrade path might look like without setting explicit milestones.

1. Deploy governance contract with the core team multisig as owner

2. Make core team multisig subject to a time delay for parameter updates but have it retain the ability to pause/unpause and do contract upgrades

3. Grant access to a binding voting system to alter less sensitive protocol parameters

4. Hand over control of all protocol parameters to binding voting systems

5. Allow binding voting systems to execute code updates

6. Remove core team multisig as owner and establish a binding voting system or ACL contract as new owner.

In scope for this proposal would be step 1 and step 2

Step 1 would be completed upon deployment of the Extensible Governance System. Step 2 will be decided by the outcome of the initial values discussion here.

Initial Parameter Values

It's established already that intial owner this governance system will be the Livepeer Inc Multisig.

OWNER=0x04746b890d090ae3c4c5df0101cfd089a4faca6c

As aforementioned delay will initially be enforced through social convention but expectations for social rules should still be set.

The proposal is that the Livepeer Inc uses a delay of 1.5x the current unbonding period when staging contract upgrades or parameter updates. This value should be used as the _delay argument when calling stage().

DELAY = 5760 blocks x 7 x 150% = 60480 blocks

At recent 12 second blocktimes this results in about a 5,5 day unbonding period and 8,5 day parameter update delay

Implementation

The latest implementation can be found here.

Copyright

Copyright and related rights waived via CC0.

[yondonfu]

Regarding the transferOwnership() function:

I think this should be triggered via a stage + execute process similar to other updates for the same reasons why any other update would use that process i.e. for transparency on when an update can be executed. One way to do this is to require the caller of the function to be the Governor itself (which would be the case when the function is called via execute()). Note this approach would not work if the Governor inherits from the OZ Ownable contract and would require a custom transferOwnership() implementation.

Regarding the use of signature based authentication in the stage() function:

This would prevent the owner of the Governor from being a contract (i.e. a multisig) because contracts cannot generate signatures. There are workarounds such as registering an EOA for signatures to be validated against in the Governor or having the Governor calling a signature validation function on the contract if the bytecode at the address is non-zero. However, in this case I think msg.sender based authentication (i.e. checking if msg.sender is the owner) would be much simpler and I don't think signature based authentication is necessary here.

Regarding the execute() function signature:

Since execute() will always refer to an existing list of updates that was created via stage() can we pass in an identifier for the list of updates instead of passing in the entire list of updates? This would make constructing the execute() tx easier (pass in a single integer instead of passing in a list of structs) and would also slightly reduce gas costs (due to less calldata included in the tx).

Given that we're already associating each update with a nonce, we could just use the nonce as the identifier that can refer to a list of updates.

Regarding the update struct definition:

The nonce doesn't need to be stored per update - a list of updates passed into stage() will all be associated with the same nonce. Instead, each nonce can be stored once for a list of updates. Something like this could work:

struct Update {
    address[] targets;
    uint256 [] values;
    bytes[] data;
    uint256 executeBlock; // Block that the update can be executed. Set this to block.number + delay in stage()
}

mapping (uint256 => Update) public updates;

[kyriediculous]

1. Having the transferOwnership take place through the upgrade system itself could be a good idea.

2. Yeah you're right that with signature based auth in the current scheme that wouldn't be possible. Would be okay using sender based authentication here. At first it would seem cool that anyone could submit updates signed by the owner of the Governor, retrospectively that seems rather weird.

3. stage() does not store the actual calldata for an update. The only thing written to storage is the hash for the update. This is because storing that entire load of data on-chain would be very inefficient so it would actually not be more gas effective, memory and calldata are magnitudes cheaper to read from and manipulate.

The same update should always result in the same hash, wish is not only a very efficient check it also guarantees a few other things such as transaction ordering within a single update. It seems like the biggest point of contention here is the function signature and the UX. I don't think UX should ever take precedent over efficiency at the contract level (especially given the current context of gas pricing and the evolution towards stateless clients). Trying to implement good UX in at the contract level will always result in tradeoffs which are not good to make because very little people use the contract layer directly.

Furthermore with the evolution of ETH1.X it seems more and more reasonable to write contracts that are as stateless as possible. The current Governor implementation holds very minimal state since update hashes are removed after execution giving the caller the storage gas refund. This makes is that updates eventually are only the transactions and processing cost from the perspective of the Governor. While the same setup can be done with directly storing the calldata for an update, it would imply more storage reads and writes and thus lead to higher gas costs.

4. You're right about the nonce.

Given 3 & 4 I think we can find a compromise that results in good UX and good efficiency.

A user will always remember the update he/she wants to execute so target, data and value should never be a problem. The main issue for the execute() function is that one has to use the same nonces the update was staged with. This leads to the user having to remember (or look them up through the event log) the transaction details. If we use a single nonce per update that becomes much easier.

Since there's also events emitted that have this information it is actually quite trivial to create a dashboard/script that utilises event logs to reconstruct the transaction data. So I don't think there is any need to store the actual calldata and storing the hash is perfectly fine.

Something along these lines:

data types

    /// @dev mapping of updateHash (keccak256(update) => executeBlock (block.number + delay)
    mapping(bytes32 => uint256) updates; 

    struct update {
        address[] target;
        uint256[] value;
        bytes[] data;
        uint256 nonce;
    }

api

    /// @notice Stage a batch of updates to be executed
    /// @dev Anyone can execute this but only transactions signed by the owner are valid
    /// @param _update Update to be staged
    /// @param _delay (uint256) Delay (in number of blocks) for the update
    function stage(update memory _update, bytes[], uint256 _delay) public onlyOwner {
        require(nonce < _update.nonce, "nonce too low");

        bytes32 updateHash = keccak256(abi.encode(_update));
        updates[updateHash] = block.number.add(_delay);
        nonce = _update.nonce;

        emit UpdateStaged(_update, _delay);
    }

    ///@notice Execute a staged update.
    /// @dev Updates are authorized during staging.
    /// @dev Reverts if a transaction can not be executed.
    /// @param _updates (update[]) Array of updates to be staged
    function execute(update memory _update) public {
        bytes32 updateHash = keccak256(abi.encode(_update));
        uint256 executeBlock = updates[updateHash];

        require(executeBlock != 0, "update is not staged");
        require(block.number >= executeBlock, "delay for update not expired");

        // prevent re-entry and replay
        delete updates[updateHash];
        for (uint256 i = 0; i < _update.target.length; i++) {
            (bool success,) = _update.target[i].call.value(_update.value[i])(_update.data[i]);
            require(success, "could not execute update");
        }

        emit UpdateExecuted(_updates);
    }

[yondonfu]

Ah wasn't clear to me that the hash of an update struct would be used as the identifier in the contract based on the spec - thanks for the clarification. Adding a sentence noting this in the spec could be helpful for giving more clarity to readers.

The updated data types + API sections of your comment look good to me.

[yondonfu]

This LIP has just been assigned the Last Call status which kicks of a 10 day Last Call period for any remaining feedback prior to the LIP being assigned the Proposed status.

[yondonfu]

A delay that is 1.5x the unbonding period feels a bit long to me. If the purpose of the delay is to give stakers an opportunity to liquidate their stake if they disagree with an accepted proposal then I think an additional round or two after the unbonding period should be sufficient.

Separately, I want to acknowledge the minimum total time required for a LIP to go from Draft to Final that results from this LIP. At the moment, a 9 (unbonding period + 2) round delay would be roughly 7.8 days given 13 second block times which results in a minimum total time for a LIP to go from Draft to Final of 10 (Last Call) + 10 (poll period) + 7.8 = 27.8 days. I think it'd be worthwhile considering shortening the Last Call and/or the poll period - topic for another thread!

[dob]

Note that there was a 3rd party security review conducted by @nachomazzara. His notes are here: https://gist.github.com/nachomazzara/8ed11e628d77ebb59a155643c2cd7fd4

[yondonfu]

Closed by #70