code-423n4 / contracts

Code4rena contracts
32 stars 6 forks source link

Review: decide about timelock controller #26

Closed gpersoon closed 2 years ago

gpersoon commented 2 years ago

One of the attention points is where to store the funds of the sponsors, see: https://discord.com/channels/810916927919620096/905119522296590418/912742834900205628

A suggestion by @HickupHH3 is to use a separate timelock contract: https://discord.com/channels/810916927919620096/905119522296590418/912812168485933137

If that is used then the governor doesn't need an integrated timelock contract (i think) https://github.com/code-423n4/contracts/blob/2624db1089fb46067682a44614bb7e894c9bad8b/contracts/ArenaGovernor.sol#L8

Also the timelock contract needs to have some functions added to transfer funds.

HickupHH3 commented 2 years ago

My understanding is that TimelockController supports native / ERC20 token transfers already (in general, arbitrary code execution). Proposals have to be made and executed on the ArenaGovernor, which then calls TimelockController.

It is important to note that any asset sent to the Governor contract will be inaccessible. All funds should be held by the TimelockController. https://docs.openzeppelin.com/contracts/4.x/api/governance#GovernorTimelockControl

TODO: write tests to ensure this is the case.

MrToph commented 2 years ago

Yes, everything needs to go through the Timelock, the timelock is also the owner of all contracts. The Timelock just executes governor proposals, so you can transfer funds if there's a proposal that reached quorum

MrToph commented 2 years ago

I always assumed the distribution of awards should come with a governance vote but would be good to double-check @adamavenir

sockdrawermoney commented 2 years ago

Yeah, my assumption was a governance vote to either release funds to an EOA for distribution or to directly distribute them from the governance action.