Allow non-beneficiaries to trigger release of their funds #1275
Conversation
frangio
added
kind:improvement
contracts
| address payee = msg.sender; | ||
|
|
||
| require(shares[payee] > 0); | ||
| function release(address _payee) public { |
There was a problem hiding this comment.
I forgot to ask for comments on this. What do you think? I felt that the previous name was not representative now.
There was a problem hiding this comment.
I think a plain withdraw is better than both of those, actually. I'm also not sure why we have escrow.withdraw but pullPayment.withdrawPayments.
There was a problem hiding this comment.
I think we've had this discussion before. The thing is TokenTimelock is using release. It would be nice to make it consistent and use the same term everywhere, if it makes sense.
There was a problem hiding this comment.
We probably have 😛 I'd rather rename TokenTimelock's to withdraw, but this is not the place for that discussion.
| const gasCost = (new web3.BigNumber(gas)).times(gasPrice); | ||
|
|
||
| await this.bounty.withdrawPayments({ from: researcher, gasPrice: gasPrice }); | ||
| await this.bounty.withdrawPayments(researcher, { gasPrice: 0 }); |
There was a problem hiding this comment.
This gasPrice: 0 thing is something we should include in the testing guide (#1077).
| @@ -51,7 +51,7 @@ contract('PostDeliveryCrowdsale', function ([_, investor, wallet, purchaser]) { | |||
| }); | |||
|
|
|||
| it('does not allow beneficiaries to withdraw tokens before crowdsale ends', async function () { | |||
| await expectThrow(this.crowdsale.withdrawTokens({ from: investor }), EVMRevert); | |||
| await expectThrow(this.crowdsale.withdrawTokens(investor), EVMRevert); | |||
There was a problem hiding this comment.
Do we want to do { from: anyone } on these, for further clarification? I'm not a fan of the extra verboseness, but dislike using the default account.
There was a problem hiding this comment.
We've never discussed this but I've actually always used the _ account as the anyone account.
There was a problem hiding this comment.
I think we should use anyone, if only just for the sake of explicitness (in a separate PR)
|
@nventuro These are all the instances I found, by searching for the use of |
Fixes #1132.