Use Hash as ID and add Nonce #7
Conversation
|
@fubuloubu let me know your thoughts if you have time. Not everything we discussed but steps toward it. |
| struct Proposal: | ||
| hash: bytes32 | ||
| approvers: DynArray[address, MAX_OWNERS] |
There was a problem hiding this comment.
Curious if we even need this structure... Just skip to signature verification and execution only. Do all the approval signature gathering off-chain, in fact you could just not collect rejections because there isn't really a fundamental difference between a rejection and never executing in the first place.
There was a problem hiding this comment.
Correct, you should be able to collect everything off chain and only do execution on chain. In fact that was something I never understood about the gnosis implementation - this necessity to rely on proposal 1 to be canceled before you make proposal 2 never made sense to me
less gas consumed too
There was a problem hiding this comment.
I think one of the requirements for Gnosis is that all the transactions be executed in order, when in reality ordering is a more subjective thing (what should happen next?). Also if you don't trust your consigned you probably should remove them.
Common thing is that a step might be forgotten in the process, and you'll have to rearrange nonces. If it's totally just based on hash id, that's not really a problem anymore. Further, I think it might even make sense to allow batch execution of multiple proposals at once, as that could save some gas and eliminate the need for special multicall handling.
|
|
||
| self.proposals[id].hash = hash | ||
| log Proposed(msg.sender, id) | ||
| def approvals(hash: bytes32) -> uint256: |
There was a problem hiding this comment.
If we move approvals all off-chain, we don't need this anymore
| assert len(self.proposals[id].approvers) >= self.minimum, "Proposal has not been approved by the minimum number of owners" | ||
| assert self.proposals[id].hash == keccak256(_abi_encode(target, calldata, amount)), "Proposal hash does not match provided data" | ||
|
|
||
| ## Neutralize proposal before executing | ||
| self.proposals[id] = empty(Proposal) |
There was a problem hiding this comment.
If we remove proposals, we don't need these anymore. Can just focus on loading the Executed event stream to display what has been executed.
|
|
||
|
|
||
| @external | ||
| def execute(id: uint256, target: address, calldata: Bytes[2000], amount: uint256): | ||
| def execute(target: address, calldata: Bytes[2000], amount: uint256, nonce: uint256): | ||
| id: bytes32 = keccak256(_abi_encode(target, calldata, amount, nonce)) |
There was a problem hiding this comment.
Can instead remap this to an EIP712 signed typed data structure, to obtain the hash for signature verification.
Makes some changes
keccak256(target . calldata . value . nonce)Note this means that there is no longer the concept of "proposing". You simply approve a hash.
Tests updated as well.
TODO: