Skip to content

Commit

Permalink
馃敀 escrow: validate streams on cancel and withdraw
Browse files Browse the repository at this point in the history 
Co-authored-by: danilo neves cruz <cruzdanilo@gmail.com>
  • Loading branch information
@itofarina @cruzdanilo
itofarina and cruzdanilo committed Oct 11, 2023
1 parent 1dfca20 commit 2cd9c82
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 30 deletions.
5 changes: 5 additions & 0 deletions .changeset/heavy-gorillas-watch.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@exactly/protocol": patch
---

馃敀 escrow: validate streams on cancel and withdraw
53 changes: 29 additions & 24 deletions .gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,35 +93,40 @@ DebtPreviewerTest:testPreviewLeverageSameWETHAssetMaxRatioMultipleCollateralAndD
DebtPreviewerTest:testPreviewLeverageSameWETHAssetMultipleCollateralAndDebtWithMinHealthFactor() (gas: 1708472)
DebtPreviewerTest:testPreviewMaxRatioWithdrawWithSameAssetLeverage() (gas: 1020516)
DebtPreviewerTest:testPreviewSameAssetInvalidLeverageShouldCapRatio() (gas: 910754)
EscrowedEXATest:testCancelFromStreamAndGetReserveBack() (gas: 457085)
EscrowedEXATest:testCancelFromStreamJustCreated() (gas: 412309)
EscrowedEXATest:testCancelShouldDeleteReserves() (gas: 470857)
EscrowedEXATest:testCancelShouldGiveReservesBack() (gas: 676123)
EscrowedEXATest:testCancelTwiceShouldRevert() (gas: 470361)
EscrowedEXATest:testCancelWithInvalidAccount() (gas: 348019)
EscrowedEXATest:testGrantTransferrerRoleAsAdmin() (gas: 45901)
EscrowedEXATest:testMint() (gas: 165141)
EscrowedEXATest:testCancelExternalStreams() (gas: 452302)
EscrowedEXATest:testCancelExternalStreamsWithesEXACancel() (gas: 903873)
EscrowedEXATest:testCancelFromStreamAndGetReserveBack() (gas: 457589)
EscrowedEXATest:testCancelFromStreamJustCreated() (gas: 412791)
EscrowedEXATest:testCancelShouldDeleteReserves() (gas: 471143)
EscrowedEXATest:testCancelShouldGiveReservesBack() (gas: 676562)
EscrowedEXATest:testCancelTwiceShouldRevert() (gas: 456028)
EscrowedEXATest:testCancelWithInvalidAccount() (gas: 348283)
EscrowedEXATest:testFakeTokenWithesEXARecipient() (gas: 932349)
EscrowedEXATest:testGrantTransferrerRoleAsAdmin() (gas: 45812)
EscrowedEXATest:testMint() (gas: 165163)
EscrowedEXATest:testMintMoreThanBalance() (gas: 27564)
EscrowedEXATest:testMintToAnother() (gas: 167337)
EscrowedEXATest:testMintZero() (gas: 14163)
EscrowedEXATest:testRedeemAsNotRedeemer() (gas: 214641)
EscrowedEXATest:testMintToAnother() (gas: 167359)
EscrowedEXATest:testMintZero() (gas: 14185)
EscrowedEXATest:testRedeemAsNotRedeemer() (gas: 214783)
EscrowedEXATest:testRedeemAsRedeemer() (gas: 165497)
EscrowedEXATest:testRedeemAsRedeemerToAnother() (gas: 170381)
EscrowedEXATest:testSetReserveRatioAsAdmin() (gas: 24450)
EscrowedEXATest:testRedeemAsRedeemerToAnother() (gas: 170399)
EscrowedEXATest:testSetReserveRatioAsAdmin() (gas: 24495)
EscrowedEXATest:testSetReserveRatioAsNotAdmin() (gas: 47318)
EscrowedEXATest:testSetVestingPeriodAsAdmin() (gas: 24598)
EscrowedEXATest:testSetReserveRatioAsZero() (gas: 16293)
EscrowedEXATest:testSetVestingPeriodAsAdmin() (gas: 24642)
EscrowedEXATest:testSetVestingPeriodAsNotAdmin() (gas: 47411)
EscrowedEXATest:testTransferToNotTransferrer() (gas: 174775)
EscrowedEXATest:testTransferToTransferrer() (gas: 210353)
EscrowedEXATest:testVest() (gas: 346856)
EscrowedEXATest:testVestToAnother() (gas: 403671)
EscrowedEXATest:testVestToAnotherAndCancel() (gas: 489338)
EscrowedEXATest:testVestWithPermitReserve() (gas: 458552)
EscrowedEXATest:testTransferToNotTransferrer() (gas: 174841)
EscrowedEXATest:testTransferToTransferrer() (gas: 210463)
EscrowedEXATest:testVest() (gas: 346917)
EscrowedEXATest:testVestToAnother() (gas: 403826)
EscrowedEXATest:testVestToAnotherAndCancel() (gas: 489580)
EscrowedEXATest:testVestWithPermitReserve() (gas: 458876)
EscrowedEXATest:testVestZero() (gas: 14151)
EscrowedEXATest:testWithdrawFromStreamAndGetReserveBack() (gas: 329552)
EscrowedEXATest:testWithdrawMaxFromMultipleStreams() (gas: 1006649)
EscrowedEXATest:testWithdrawMaxShouldGiveReserveBackWhenDepleted() (gas: 327542)
EscrowedEXATest:testWithdrawMaxWithInvalidSender() (gas: 339414)
EscrowedEXATest:testWithdrawFromStreamAndGetReserveBack() (gas: 329816)
EscrowedEXATest:testWithdrawFromUnknownStream() (gas: 898481)
EscrowedEXATest:testWithdrawMaxFromMultipleStreams() (gas: 1007617)
EscrowedEXATest:testWithdrawMaxShouldGiveReserveBackWhenDepleted() (gas: 327828)
EscrowedEXATest:testWithdrawMaxWithInvalidSender() (gas: 339700)
InterestRateModelTest:testFixedBorrowRate() (gas: 8089)
InterestRateModelTest:testFloatingBorrowRate() (gas: 6236)
InterestRateModelTest:testMinFixedRate() (gas: 7610)
Expand Down
18 changes: 15 additions & 3 deletions contracts/periphery/EscrowedEXA.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ contract EscrowedEXA is ERC20VotesUpgradeable, AccessControlUpgradeable {
function vest(uint128 amount, address to) public returns (uint256 streamId) {
assert(amount != 0);
_burn(msg.sender, amount);
uint256 reserve = amount.mulWadDown(reserveRatio);
uint256 reserve = amount.mulWadUp(reserveRatio);
exa.safeTransferFrom(msg.sender, address(this), reserve);
streamId = sablier.createWithDurations(
CreateWithDurations({
Expand Down Expand Up @@ -131,6 +131,7 @@ contract EscrowedEXA is ERC20VotesUpgradeable, AccessControlUpgradeable {
function _cancel(uint256[] memory streamIds) internal returns (uint256 streamsReserves, uint128 refundableAmount) {
for (uint256 i = 0; i < streamIds.length; ++i) {
uint256 streamId = streamIds[i];
checkStream(streamId);
assert(msg.sender == sablier.getRecipient(streamId));
streamsReserves += reserves[streamId];
delete reserves[streamId];
Expand All @@ -146,6 +147,7 @@ contract EscrowedEXA is ERC20VotesUpgradeable, AccessControlUpgradeable {
function withdrawMax(uint256[] memory streamIds) public {
for (uint256 i = 0; i < streamIds.length; ++i) {
uint256 streamId = streamIds[i];
checkStream(streamId);
assert(msg.sender == sablier.getRecipient(streamId));
withdrawMax(streamId);
}
Expand All @@ -162,12 +164,20 @@ contract EscrowedEXA is ERC20VotesUpgradeable, AccessControlUpgradeable {
}
}

/// @notice Checks if a stream is valid through its reserve. Reverts with `InvalidStream` if it is not.
/// @param streamId streamId to check.
function checkStream(uint256 streamId) internal view {
if (reserves[streamId] == 0) revert InvalidStream();
}

/// @notice Hook called when a recipient cancels a stream.
/// @notice Mints esEXA to the recipient with the remaining EXA received from the canceled stream.
/// @param streamId streamId of the cancelled stream.
/// @param recipient recipient of the cancelled stream.
/// @param senderAmount amount of EXA sent to the recipient.
function onStreamCanceled(uint256, address recipient, uint128 senderAmount, uint128) external {
/// @param senderAmount amount of EXA received back from the stream cancelling.
function onStreamCanceled(uint256 streamId, address recipient, uint128 senderAmount, uint128) external {
assert(msg.sender == address(sablier));
checkStream(streamId);
_mint(recipient, senderAmount);
}

Expand All @@ -183,6 +193,7 @@ contract EscrowedEXA is ERC20VotesUpgradeable, AccessControlUpgradeable {
/// @param reserveRatio_ New reserve ratio.
/// @dev Caller must have DEFAULT_ADMIN_ROLE.
function setReserveRatio(uint256 reserveRatio_) public onlyRole(DEFAULT_ADMIN_ROLE) {
assert(reserveRatio_ != 0);
reserveRatio = reserveRatio_;
emit ReserveRatioSet(reserveRatio_);
}
Expand All @@ -207,6 +218,7 @@ contract EscrowedEXA is ERC20VotesUpgradeable, AccessControlUpgradeable {
}

error Untransferable();
error InvalidStream();

interface ISablierV2LockupLinear {
function cancel(uint256 streamId) external;
Expand Down
122 changes: 119 additions & 3 deletions test/EscrowedEXA.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@ import { ForkTest, stdError } from "./Fork.t.sol";
import {
EXA,
Permit,
Broker,
Durations,
EscrowedEXA,
InvalidStream,
Untransferable,
CreateWithDurations,
ISablierV2LockupLinear
} from "../contracts/periphery/EscrowedEXA.sol";
import { MockERC20 } from "solmate/src/test/utils/mocks/MockERC20.sol";

contract EscrowedEXATest is ForkTest {
using FixedPointMathLib for uint256;
Expand Down Expand Up @@ -186,7 +191,7 @@ contract EscrowedEXATest is ForkTest {
assertEq(exa.balanceOf(address(this)), initialEXA + withdrawableAmount, "should give reserves back");
assertEq(esEXA.reserves(streamId1), 0, "reserves[streamId1] == 0");
assertEq(esEXA.reserves(streamId2), 0, "reserves[streamId2] == 0");
assertEq(esEXA.balanceOf(address(this)), esEXABefore + initialAmount, "should give back half of the esexa");
assertEq(esEXA.balanceOf(address(this)), esEXABefore + initialAmount, "should give back half of the esEXA");
}

function testVestToAnotherAndCancel() external {
Expand Down Expand Up @@ -221,7 +226,7 @@ contract EscrowedEXATest is ForkTest {
esEXA.cancel(streamIds);

assertEq(esEXA.reserves(streamId), 0, "reserves[streamId] == 0");
assertEq(esEXA.balanceOf(address(this)), esEXABefore + amount / 2, "should give back half of the esexa");
assertEq(esEXA.balanceOf(address(this)), esEXABefore + amount / 2, "should give back half of the esEXA");
assertEq(exa.balanceOf(address(this)), exaBefore + reserve + withdrawableAmount, "should give back reserve");
}

Expand All @@ -235,7 +240,7 @@ contract EscrowedEXATest is ForkTest {
uint256[] memory streamIds = new uint256[](1);
streamIds[0] = streamId;
esEXA.cancel(streamIds);
vm.expectRevert(abi.encodeWithSelector(SablierV2Lockup_StreamDepleted.selector, streamId));
vm.expectRevert(abi.encodeWithSelector(InvalidStream.selector));
esEXA.cancel(streamIds);
}

Expand Down Expand Up @@ -442,6 +447,117 @@ contract EscrowedEXATest is ForkTest {
assertEq(exa.balanceOf(address(this)), exaBefore + reserve, "reserve should be back");
}

function testFakeTokenWithesEXARecipient() external {
MockERC20 token = new MockERC20("Malicious token", "MT", 18);
uint128 amount = 1_000 ether;
token.mint(address(this), amount);
token.approve(address(sablier), type(uint256).max);

uint256 esEXABefore = esEXA.balanceOf(address(this));
uint256 streamId = sablier.createWithDurations(
CreateWithDurations({
asset: EXA(address(token)),
sender: address(this),
recipient: address(esEXA),
totalAmount: amount,
cancelable: true,
durations: Durations({ cliff: 0, total: 1 }),
broker: Broker({ account: address(0), fee: 0 })
})
);
sablier.cancel(streamId);

assertEq(esEXA.balanceOf(address(this)), esEXABefore, "esEXA balance shouldn't change");
}

function testCancelExternalStreams() external {
uint128 amount = 1_000 ether;
exa.approve(address(sablier), type(uint256).max);
uint256 esEXABefore = esEXA.balanceOf(address(this));
uint256 streamId = sablier.createWithDurations(
CreateWithDurations({
asset: exa,
sender: address(this),
recipient: address(esEXA),
totalAmount: amount,
cancelable: true,
durations: Durations({ cliff: 0, total: 1 }),
broker: Broker({ account: address(0), fee: 0 })
})
);
sablier.cancel(streamId);
assertEq(esEXA.balanceOf(address(this)), esEXABefore, "esEXA balance shouldn't change");

streamId = sablier.createWithDurations(
CreateWithDurations({
asset: exa,
sender: address(esEXA),
recipient: address(this),
totalAmount: amount,
cancelable: true,
durations: Durations({ cliff: 0, total: 1 }),
broker: Broker({ account: address(0), fee: 0 })
})
);
uint256 exaBefore = exa.balanceOf(address(this));
sablier.cancel(streamId);
assertEq(exa.balanceOf(address(this)), exaBefore, "EXA balance shouldn't change");
assertEq(esEXA.balanceOf(address(this)), esEXABefore, "should not mint esEXA");
}

function testCancelExternalStreamsWithesEXACancel() external {
MockERC20 token = new MockERC20("Malicious token", "MT", 18);
uint128 amount = 1_000 ether;
token.mint(address(this), amount);
token.approve(address(sablier), type(uint256).max);

uint256 esEXABefore = esEXA.balanceOf(address(this));
uint256[] memory streams = new uint256[](1);
streams[0] = sablier.createWithDurations(
CreateWithDurations({
asset: EXA(address(token)),
sender: address(esEXA),
recipient: address(this),
totalAmount: amount,
cancelable: true,
durations: Durations({ cliff: 0, total: 1 }),
broker: Broker({ account: address(0), fee: 0 })
})
);

vm.expectRevert(InvalidStream.selector);
esEXA.cancel(streams);
assertEq(esEXA.balanceOf(address(this)), esEXABefore, "esEXA balance shouldn't change");
}

function testSetReserveRatioAsZero() external {
vm.expectRevert(stdError.assertionError);
esEXA.setReserveRatio(0);
}

function testWithdrawFromUnknownStream() external {
MockERC20 token = new MockERC20("Random token", "RNT", 18);
uint128 amount = 1_000 ether;
token.mint(address(this), amount);
token.approve(address(sablier), type(uint256).max);

uint256[] memory streams = new uint256[](1);
streams[0] = sablier.createWithDurations(
CreateWithDurations({
asset: EXA(address(token)),
sender: address(this),
recipient: address(this),
totalAmount: amount,
cancelable: true,
durations: Durations({ cliff: 0, total: 1 }),
broker: Broker({ account: address(0), fee: 0 })
})
);

vm.expectRevert(abi.encodeWithSelector(InvalidStream.selector));
esEXA.withdrawMax(streams);
}

event ReserveRatioSet(uint256 reserveRatio);
event VestingPeriodSet(uint256 vestingPeriod);
event Vest(address indexed caller, address indexed account, uint256 indexed streamId, uint256 amount);
Expand Down

0 comments on commit 2cd9c82

Please sign in to comment.