search

OpenZeppelin / openzeppelin-contracts

OpenZeppelin Contracts is a library for secure smart contract development.
https://openzeppelin.com/contracts
MIT License
24.94k stars 11.8k forks source link

RefundableCrowdsale is not trustless #918

Closed tomoyuki28jp closed 6 years ago

tomoyuki28jp commented 6 years ago

I think RefundableCrowdsale does not have to inherit FinalizableCrowdsale. Changing RefundableCrowdsale inherits TimedCrowdsale, Ownable instead of FinalizableCrowdsale makes it trustless.

ref: https://github.com/OpenZeppelin/zeppelin-solidity/issues/301

come-maiz commented 6 years ago

The RefundableCrowdsale uses the variable isFinalized from FinalizableCrowdsale: https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/crowdsale/distribution/RefundableCrowdsale.sol#L38 Also the finalization function is called by the function finalize from FinalizableCrowdsale.

I don't see how this would work without inheriting from FinalizableCrowdsale. And I think that we can continue the discussion about trustlessness in #301. So I'm going to close this one, but please let me know if I'm missing a detail or something.

pura vida

tomoyuki28jp commented 6 years ago

@elopio Thanks for your reply. Why RefundableCrowdsale is FinalizableCrowdsale? I think RefundableCrowdsale should refund when TimedCrowdsale.hasClosed() && !goalReached() . RefundableCrowdsale is not trustless because RefundableCrowdsale inherit FinalizableCrowdsale and an owner must finalize it.

come-maiz commented 6 years ago

I understand now what you mean @tomoyuki28jp. The conclusion on the other issue was to propose a trustless finalizable, in addition to the current implementation. If that one lands, we could make a trustless refundable inherit from it. Or you could also propose an independent trustless RefundableCrowdsale, and we can discuss about it on the PR.

tomoyuki28jp commented 6 years ago

I will consider to create a PR, thanks for your reply, @elopio :)