Add treasury wallet in global storage #555
Conversation
satyamakgec
changed the title
|
@satyamakgec As per discussion this morning, please can you add treasury wallet to module config files (where it existed previously) but with the logic that address(0) is valid (and corresponds to using the DS version). |
| @@ -257,7 +257,7 @@ contract ERC20DividendCheckpoint is ERC20DividendCheckpointStorage, DividendChec | |||
| dividends[_dividendIndex].reclaimed = true; | |||
| Dividend storage dividend = dividends[_dividendIndex]; | |||
| uint256 remainingAmount = dividend.amount.sub(dividend.claimedAmount); | |||
| require(IERC20(dividendTokens[_dividendIndex]).transfer(wallet, remainingAmount), "transfer failed"); | |||
| require(IERC20(dividendTokens[_dividendIndex]).transfer(getTreasuryWallet(), remainingAmount), "transfer failed"); | |||
There was a problem hiding this comment.
For good measure do you think we should be checking that getTreasuryWallet() doesn't return address(0)? I agree that with the current code it can't, but in some future model if there was a bug, perhaps the datastore treasury wallet could be over-written with 0, and we wouldn't want to send funds there under any circumstances.
There was a problem hiding this comment.
on this measure, global treasury wallet is only be changed using the changeTreasuryWallet() function that is present in the SecurityToken and we have the 0 address check init. So it doesn't require any further check. And yes there is no harm to add one more check.
| @@ -45,6 +46,7 @@ contract STFactory is ISTFactory { | |||
| ); | |||
| //NB When dataStore is generated, the security token address is automatically set via the constructor in DataStoreProxy. | |||
| newSecurityToken.changeDataStore(dataStoreFactory.generateDataStore(address(newSecurityToken))); | |||
| newSecurityToken.changeTreasuryWallet(_treasuryWallet); | |||
There was a problem hiding this comment.
What's the reason to not add this to the token constructor directly?
There was a problem hiding this comment.
because datastore doesn't aware of the security token address at that time.
No description provided.