daostack / arc

Arc is an operating system for DAOs.
GNU General Public License v3.0
47 stars 25 forks source link

feat(contracts): Add WalletScheme contract and tests #758

Open AugustoL opened 4 years ago

AugustoL commented 4 years ago

Adds the wallet scheme contract, it is based in the genericScheme but it doesnt make the genericCall to the controller, instead it can execute multiple staticCalls to any contract. (It does not allow execute staticCalls on the scheme itself and controller).

I know that this "breaks" the current design of the DAOstack insfrastructure, although we (DXdao devs) would like to go in this direction by replacing the avatar role into one of this schemes, have different layers of security (wallets with ++ funds, higher requirements to execute proposals).

Interested in your review @orenyodfat and @leviadam .

orenyodfat commented 4 years ago

@AugustoL could you please explain more about the motivation behind this scheme . In relation to security concerns :

AugustoL commented 4 years ago

@AugustoL could you please explain more about the motivation behind this scheme . In relation to security concerns :

  • This scheme has no constraint re the target contract it sends the funds to(which are part of the proposal) so in a sense you are getting more general porpose scheme though you are paying with more vulnerability.

Hola @orenyodfat, effectively. The idea of the WalletScheme is to have the flexibility to make static calls directly to any contract. The idea is to have multiple wallets with different level of security (weekly withdrawal limit, minimum execution time, required reputation, etc).

By not calling the controller we wont be able to use the global constraints, right? maybe we would have to add this configuration in the wallet scheme itself, right?

orenyodfat commented 4 years ago

@AugustoL could you please explain more about the motivation behind this scheme . In relation to security concerns :

  • This scheme has no constraint re the target contract it sends the funds to(which are part of the proposal) so in a sense you are getting more general porpose scheme though you are paying with more vulnerability.

Hola @orenyodfat, effectively. The idea of the WalletScheme is to have the flexibility to make static calls directly to any contract. The idea is to have multiple wallets with different level of security (weekly withdrawal limit, minimum execution time, required reputation, etc).

By not calling the controller we wont be able to use the global constraints, right? maybe we would have to add this configuration in the wallet scheme itself, right?

Yes. no global constraint on this scheme.

Few options for that :

  1. multiple standard genericSchemes. each for each wallet.
  2. use ContributionReward proposals to send funds from the avatar to each wallet.
  3. have a generic scheme with white listed target contracts.

Please note that option 3 is new (same as the one you implemented) and will require support on the higher layers (migration/subgraph/arc.js/alchemy). option 1&2 are already supported by alchemy .