Deleted/active recipients flag

Giveth / vaultcontroller

Vault Architecture

GNU General Public License v3.0

0 stars 3 forks source link

Deleted/active recipients flag #35

Open adria0 opened 7 years ago

adria0 commented 7 years ago

Consider to add a flag active in struct Recipient(like struct Spender) instead using activationTime == 0 as deletion flag

jbaylina commented 7 years ago

It but it would take more gas...

GriffGreen commented 7 years ago

gas optimization is something I want to do some time on this contract... i don't know if its best done now or after the first run of tests.... but low gas is a huge priority... mostly cause i think I will end up using these contracts a lot and am very cheap ;-)

dipenjoshi commented 7 years ago

i'm mostly worried about out of gas exceptions. being cheap on gas is 2nd priority.

GriffGreen commented 7 years ago

I agree about the out of gas annoyances! This is something that mostly needs to be fixed in estimateGas() on the UI side, but it has been a big problem for us

jbaylina commented 7 years ago

Here, the discusion is about "gas optimization" <-> "code readability" .

adria0 commented 7 years ago

Proposal:

Finish tests (& fix code bugs)
Make special tests only to check gas usage for specific scenarios (which ones?)
Make proposals for "gas optimization" <-> "code readability" equilibrium. Check changes against gas tests and then decide.

GriffGreen commented 7 years ago

sounds good.