tezos-commons / baseDAO

BaseDAO - a generic smart contract framework for DAOs on Tezos
57 stars 15 forks source link

Modify entrypoints on which to apply the no-xtz check #256

Closed pasqu4le closed 3 years ago

pasqu4le commented 3 years ago

Clarification and motivation

We currently have a safety check for most entrypoints to disallow xtz tokens to be mistakenly sent to the contract and allow these funds to be stuck there.

We need to reconsider on which entrypoints we should keep this check and on which we don't need to (see #91).

As a rule of thumb, the distinction should be to keep disallowing these transfers for entrypoints commonly called by users, which may misunderstand the docs and attempt to send tez to.

Namely, they need to keep refusing xtz:

Other entrypoints allow xtz for a specific reason and have to keep doing so:

The remaining ones can be reconsidered.

Acceptance criteria

We should decide what to keep in forbid_xtz_params and allow_xtz_params parts of the overall parameter and then make the changes.

pasqu4le commented 3 years ago

@rinn7e IMO we should keep all use-callable entrypoints as xtz-forbidding.

In particular, aside from the ones above, I think we should keep into forbid_xtz_params:

the reason being that these too could be misinterpreted.

OTOH I think we can move to the allow_xtz_params:

and keep Transfer_contract_tokens there as well, as these are admin-only entrypoints and it might be useful to make an xtz transfer while calling them for the admin.

In summary I think the only change that we should do is to move Transfer_ownership and Accept_ownership to allow_xtz_params and keep everything else the same.

What do you think? Does that sound good to you?

rinn7e commented 3 years ago

Seems fine for me. The goal is to allow admin to use this entrypoint to send tez to the contract, right?

On another note, in case of contract migration to a new address, is there currently a way to send tez to the new contract?

pasqu4le commented 3 years ago

The goal is to allow admin to use this entrypoint to send tez to the contract, right?

Well sort of, yes. I guess the goal would be better defined as: lifting the limitation as much as possible without making it dangerous for users. Which turns out to be equal to lifting it from admin-only entrypoints after all.

On another note, in case of contract migration to a new address, is there currently a way to send tez to the new contract?

Only through proposals, which is intended as after the admin transfers his role to the DAO itself everything should be decided by the participants using the proposals (including migrating all of the funds).