diff --git a/packages/nouns-contracts/contracts/StreamEscrow.sol b/packages/nouns-contracts/contracts/StreamEscrow.sol index d174841d2..e183e737d 100644 --- a/packages/nouns-contracts/contracts/StreamEscrow.sol +++ b/packages/nouns-contracts/contracts/StreamEscrow.sol @@ -20,8 +20,11 @@ pragma solidity ^0.8.19; import { IStreamEscrow } from './interfaces/IStreamEscrow.sol'; import { INounsToken } from './interfaces/INounsToken.sol'; import { IERC20 } from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; +import { SafeERC20 } from '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol'; contract StreamEscrow is IStreamEscrow { + using SafeERC20 for IERC20; + modifier onlyDAO() { require(msg.sender == daoExecutor, 'only dao'); _; @@ -73,6 +76,8 @@ contract StreamEscrow is IStreamEscrow { address streamCreator_, uint32 minimumTickDuration_ ) { + require(nounsRecipient_ != address(0), 'zero address'); + daoExecutor = daoExecutor_; ethRecipient = ethRecipient_; nounsRecipient = nounsRecipient_; @@ -119,14 +124,14 @@ contract StreamEscrow is IStreamEscrow { uint32 streamLastTick = currentTick + streamLengthInTicks; ethStreamEndingAtTick[streamLastTick] += ethPerTick; - // the remainder is immediately streamed to the DAO - uint256 remainder = msg.value % streamLengthInTicks; - sendETHToTreasury(remainder); - uint128 newEthStreamedPerTick = ethStreamedPerTick + ethPerTick; ethStreamedPerTick = newEthStreamedPerTick; streams[nounId] = Stream({ ethPerTick: ethPerTick, canceled: false, lastTick: streamLastTick }); emit StreamCreated(nounId, msg.value, streamLengthInTicks, ethPerTick, newEthStreamedPerTick, streamLastTick); + + // the remainder is immediately streamed to the DAO + uint256 remainder = msg.value % streamLengthInTicks; + sendETHToTreasury(remainder); } /** @@ -141,11 +146,13 @@ contract StreamEscrow is IStreamEscrow { lastForwardTimestamp = toUint48(block.timestamp); - sendETHToTreasury(ethStreamedPerTick); + uint128 ethStreamedPerTick_ = ethStreamedPerTick; (uint32 newTick, uint128 ethPerTickEnded) = increaseTicksAndFinishStreams(); emit StreamsForwarded(newTick, ethPerTickEnded, ethStreamedPerTick, lastForwardTimestamp); + + sendETHToTreasury(ethStreamedPerTick_); } /** @@ -293,8 +300,10 @@ contract StreamEscrow is IStreamEscrow { /** * @notice Allows the DAO to set the address that the Nouns tokens will be sent to when streams are canceled. + * The zero address is not allowed because it will cause the transfer to revert, which will cause all cancellations to revert. */ function setNounsRecipient(address newAddress) external onlyDAO { + require(newAddress != address(0), 'zero address'); nounsRecipient = newAddress; emit NounsRecipientSet(newAddress); } @@ -306,7 +315,7 @@ contract StreamEscrow is IStreamEscrow { * @param amount The amount of tokens to rescue. */ function rescueToken(address token, address to, uint256 amount) external onlyDAO { - IERC20(token).transfer(to, amount); + IERC20(token).safeTransfer(to, amount); } /** diff --git a/packages/nouns-contracts/test/foundry/StreamEscrow.t.sol b/packages/nouns-contracts/test/foundry/StreamEscrow.t.sol index 1bc3e8d00..99b755d65 100644 --- a/packages/nouns-contracts/test/foundry/StreamEscrow.t.sol +++ b/packages/nouns-contracts/test/foundry/StreamEscrow.t.sol @@ -693,6 +693,17 @@ contract DAOSettersTest is BaseStreamEscrowTest { // check that new recipient received the noun assertEq(nounsToken.ownerOf(1), makeAddr('nounsRecipient2')); } + + function test_setNounsRecipient_cantBeZero() public { + vm.prank(treasury); + vm.expectRevert('zero address'); + escrow.setNounsRecipient(address(0)); + } + + function test_nounsReceipient_cantBeZeroInConstructor() public { + vm.expectRevert('zero address'); + new StreamEscrow(treasury, ethRecipient, address(0), address(nounsToken), streamCreator, 24 hours); + } } contract RescueTokensTest is BaseStreamEscrowTest {