From 287245f4a61be0fbddad9fb23f2f0b533599dfc1 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Wed, 19 Apr 2023 19:01:55 +0300 Subject: [PATCH 1/6] build: update v2-core feat: implement "onStreamCanceled" hook feat: add proxy plugin --- .gitmodules | 2 +- lib/v2-core | 2 +- src/SablierV2ProxyTarget.sol | 30 +++++++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/.gitmodules b/.gitmodules index 34e16199..0328cdbc 100644 --- a/.gitmodules +++ b/.gitmodules @@ -19,6 +19,6 @@ path = "lib/prb-test" url = "https://github.com/PaulRBerg/prb-test" [submodule "lib/v2-core"] - branch = "main" + branch = "fix-hooks" path = "lib/v2-core" url = "https://github.com/sablierhq/v2-core" diff --git a/lib/v2-core b/lib/v2-core index 6362f846..b343c608 160000 --- a/lib/v2-core +++ b/lib/v2-core @@ -1 +1 @@ -Subproject commit 6362f84642debc4d48109605db800e54163e726b +Subproject commit b343c60823e4f7a3e7b7be6af686fabb876f704f diff --git a/src/SablierV2ProxyTarget.sol b/src/SablierV2ProxyTarget.sol index 53a1f3ac..a1cd686d 100644 --- a/src/SablierV2ProxyTarget.sol +++ b/src/SablierV2ProxyTarget.sol @@ -3,9 +3,12 @@ pragma solidity >=0.8.19; import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/token/ERC20/utils/SafeERC20.sol"; +import { IPRBProxy } from "@prb/proxy/interfaces/IPRBProxy.sol"; +import { IPRBProxyPlugin } from "@prb/proxy/interfaces/IPRBProxyPlugin.sol"; import { ISablierV2Lockup } from "@sablier/v2-core/interfaces/ISablierV2Lockup.sol"; import { ISablierV2LockupLinear } from "@sablier/v2-core/interfaces/ISablierV2LockupLinear.sol"; import { ISablierV2LockupDynamic } from "@sablier/v2-core/interfaces/ISablierV2LockupDynamic.sol"; +import { ISablierV2LockupSender } from "@sablier/v2-core/interfaces/hooks/ISablierV2LockupSender.sol"; import { LockupDynamic, LockupLinear } from "@sablier/v2-core/types/DataTypes.sol"; import { IAllowanceTransfer } from "permit2/interfaces/IAllowanceTransfer.sol"; @@ -34,7 +37,7 @@ import { Batch, Permit2Params } from "./types/DataTypes.sol"; /// @title SablierV2ProxyTarget /// @notice Implements the {ISablierV2ProxyTarget} interface. -contract SablierV2ProxyTarget is ISablierV2ProxyTarget { +contract SablierV2ProxyTarget is ISablierV2ProxyTarget, IPRBProxyPlugin, ISablierV2LockupSender { using SafeERC20 for IERC20; /*////////////////////////////////////////////////////////////////////////// @@ -124,6 +127,21 @@ contract SablierV2ProxyTarget is ISablierV2ProxyTarget { lockup.withdrawMax(streamId, to); } + /// @inheritdoc ISablierV2LockupSender + function onStreamCanceled( + ISablierV2Lockup lockup, + uint256 streamId, + uint128 senderAmount, + uint128 recipientAmount + ) + external + { + recipientAmount; // silence the warning + IERC20 asset = lockup.getAsset(streamId); + address proxyOwner = IPRBProxy(msg.sender).owner(); + asset.safeTransfer({ to: proxyOwner, value: senderAmount }); + } + /*////////////////////////////////////////////////////////////////////////// SABLIER-V2-LOCKUP-LINEAR //////////////////////////////////////////////////////////////////////////*/ @@ -561,6 +579,16 @@ contract SablierV2ProxyTarget is ISablierV2ProxyTarget { streamId = dynamic.createWithMilestones(params); } + /*////////////////////////////////////////////////////////////////////////// + PROXY-PLUGIN + //////////////////////////////////////////////////////////////////////////*/ + + function methodList() external pure returns (bytes4[] memory methods) { + bytes4[] memory functionSig = new bytes4[](1); + functionSig[0] = this.onStreamCanceled.selector; + methods = functionSig; + } + /*////////////////////////////////////////////////////////////////////////// HELPER FUNCTIONS //////////////////////////////////////////////////////////////////////////*/ From d7a7a506432e3dea19946cb71c79fc032aea1963 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Wed, 19 Apr 2023 19:03:41 +0300 Subject: [PATCH 2/6] test: execute plugin for proxy --- test/Base.t.sol | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/Base.t.sol b/test/Base.t.sol index fe4e5390..5376896f 100644 --- a/test/Base.t.sol +++ b/test/Base.t.sol @@ -4,6 +4,7 @@ pragma solidity >=0.8.19 <0.9.0; import { ERC20 } from "@openzeppelin/token/ERC20/ERC20.sol"; import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; import { IPRBProxy } from "@prb/proxy/interfaces/IPRBProxy.sol"; +import { PRBProxyHelpers } from "@prb/proxy/PRBProxyHelpers.sol"; import { PRBProxyRegistry } from "@prb/proxy/PRBProxyRegistry.sol"; import { SablierV2Comptroller } from "@sablier/v2-core/SablierV2Comptroller.sol"; import { SablierV2LockupLinear } from "@sablier/v2-core/SablierV2LockupLinear.sol"; @@ -33,6 +34,7 @@ abstract contract Base_Test is Assertions, StdCheats { //////////////////////////////////////////////////////////////////////////*/ bytes32 internal immutable DOMAIN_SEPARATOR; + bytes onStreamCanceledData; /*////////////////////////////////////////////////////////////////////////// VARIABLES @@ -48,10 +50,11 @@ abstract contract Base_Test is Assertions, StdCheats { IERC20 internal dai = new ERC20("Dai Stablecoin", "DAI"); Defaults internal defaults; SablierV2LockupDynamic internal dynamic; + SablierV2LockupLinear internal linear; SablierV2NFTDescriptor internal nftDescriptor = new SablierV2NFTDescriptor(); AllowanceTransfer internal permit2 = new AllowanceTransfer(); IPRBProxy internal proxy; - SablierV2LockupLinear internal linear; + PRBProxyHelpers internal proxyHelpers = new PRBProxyHelpers(); SablierV2ProxyTarget internal target = new SablierV2ProxyTarget(permit2); WETH internal weth = new WETH(); @@ -61,6 +64,7 @@ abstract contract Base_Test is Assertions, StdCheats { constructor() { DOMAIN_SEPARATOR = permit2.DOMAIN_SEPARATOR(); + onStreamCanceledData = abi.encodeCall(proxyHelpers.installPlugin, (target)); } /*////////////////////////////////////////////////////////////////////////// @@ -75,7 +79,7 @@ abstract contract Base_Test is Assertions, StdCheats { users.sender = createUser("Sender"); // Deploy the sender's proxy contract. - proxy = new PRBProxyRegistry().deployFor(users.sender.addr); + (proxy,) = new PRBProxyRegistry().deployAndExecuteFor(users.sender.addr, address(proxyHelpers), onStreamCanceledData); // Deploy the defaults contract. defaults = new Defaults(users, proxy, dai); From 1e32a88fae1a470c20cb1326d12c306fae3cd7f2 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Thu, 20 Apr 2023 13:13:36 +0300 Subject: [PATCH 3/6] docs: add natspec for onStreamCanceled chore: order alphabetically functions test: deploy and install plugin in the setUp function test: onStreamCanceled function --- src/SablierV2ProxyTarget.sol | 39 ++++++++++++------- test/Base.t.sol | 6 ++- .../on-stream-canceled/onStreamCanceled.t.sol | 31 +++++++++++++++ 3 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 test/unit/on-stream-canceled/onStreamCanceled.t.sol diff --git a/src/SablierV2ProxyTarget.sol b/src/SablierV2ProxyTarget.sol index a1cd686d..a6bb38a1 100644 --- a/src/SablierV2ProxyTarget.sol +++ b/src/SablierV2ProxyTarget.sol @@ -112,6 +112,29 @@ contract SablierV2ProxyTarget is ISablierV2ProxyTarget, IPRBProxyPlugin, ISablie _postCancelMultiple(initialBalances, assets); } + /// @inheritdoc ISablierV2LockupSender + /// @dev This function is necessary to automatically redirect the funds to the sender, i.e. the proxy owner, when + /// recipients trigger cancellations. + function onStreamCanceled( + ISablierV2Lockup lockup, + uint256 streamId, + uint128 senderAmount, + uint128 recipientAmount + ) + external + { + recipientAmount; // silence the warning + + IERC20 asset = lockup.getAsset(streamId); + + // The `lockup` contract will have the proxy contract set as the sender. + address proxy = lockup.getSender(streamId); + address owner = IPRBProxy(proxy).owner(); + + // Transfer the funds from the proxy contract to the sender. + asset.safeTransfer({ to: owner, value: senderAmount }); + } + /// @inheritdoc ISablierV2ProxyTarget function renounce(ISablierV2Lockup lockup, uint256 streamId) external { lockup.renounce(streamId); @@ -127,21 +150,6 @@ contract SablierV2ProxyTarget is ISablierV2ProxyTarget, IPRBProxyPlugin, ISablie lockup.withdrawMax(streamId, to); } - /// @inheritdoc ISablierV2LockupSender - function onStreamCanceled( - ISablierV2Lockup lockup, - uint256 streamId, - uint128 senderAmount, - uint128 recipientAmount - ) - external - { - recipientAmount; // silence the warning - IERC20 asset = lockup.getAsset(streamId); - address proxyOwner = IPRBProxy(msg.sender).owner(); - asset.safeTransfer({ to: proxyOwner, value: senderAmount }); - } - /*////////////////////////////////////////////////////////////////////////// SABLIER-V2-LOCKUP-LINEAR //////////////////////////////////////////////////////////////////////////*/ @@ -583,6 +591,7 @@ contract SablierV2ProxyTarget is ISablierV2ProxyTarget, IPRBProxyPlugin, ISablie PROXY-PLUGIN //////////////////////////////////////////////////////////////////////////*/ + /// @inheritdoc IPRBProxyPlugin function methodList() external pure returns (bytes4[] memory methods) { bytes4[] memory functionSig = new bytes4[](1); functionSig[0] = this.onStreamCanceled.selector; diff --git a/test/Base.t.sol b/test/Base.t.sol index 5376896f..16ff8c67 100644 --- a/test/Base.t.sol +++ b/test/Base.t.sol @@ -79,7 +79,11 @@ abstract contract Base_Test is Assertions, StdCheats { users.sender = createUser("Sender"); // Deploy the sender's proxy contract. - (proxy,) = new PRBProxyRegistry().deployAndExecuteFor(users.sender.addr, address(proxyHelpers), onStreamCanceledData); + (proxy,) = new PRBProxyRegistry().deployAndExecuteFor({ + owner: users.sender.addr, + target: address(proxyHelpers), + data: onStreamCanceledData + }); // Deploy the defaults contract. defaults = new Defaults(users, proxy, dai); diff --git a/test/unit/on-stream-canceled/onStreamCanceled.t.sol b/test/unit/on-stream-canceled/onStreamCanceled.t.sol new file mode 100644 index 00000000..8485f0d7 --- /dev/null +++ b/test/unit/on-stream-canceled/onStreamCanceled.t.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.19 <0.9.0; + +import { Base_Test } from "../../Base.t.sol"; + +contract OnStreamCanceled_Unit_Test is Base_Test { + function test_OnStreamCanceled() external { + uint256 streamId = createWithRange(); + + // Warp into the future. + vm.warp(defaults.WARP_26_PERCENT()); + + // Make the `recipient` the caller. + changePrank(users.recipient.addr); + + uint256 balanceBefore = dai.balanceOf(users.sender.addr); + + // Asset flow: Sablier contract → proxy → proxy owner + // Expect transfers from the Sablier contract to the proxy, and then from the proxy to the + // proxy owner. + expectCallToTransfer({ to: address(proxy), amount: defaults.REFUND_AMOUNT() }); + expectCallToTransfer({ to: users.sender.addr, amount: defaults.REFUND_AMOUNT() }); + + // Cancel the stream. + linear.cancel(streamId); + + uint256 actualBalance = dai.balanceOf(users.sender.addr); + uint256 expectedBalance = balanceBefore + defaults.REFUND_AMOUNT(); + assertEq(actualBalance, expectedBalance, "balance does not match"); + } +} From c66703c7131a2bd956292b0eed2142f628d29c47 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Thu, 20 Apr 2023 13:36:25 +0300 Subject: [PATCH 4/6] test: methodList function test: add assertEq function for bytes4[] test: remove unnecessary assertEq functions --- test/helpers/Assertions.t.sol | 27 ++++++++++++++++++-------- test/unit/method-list/methodList.t.sol | 17 ++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) create mode 100644 test/unit/method-list/methodList.t.sol diff --git a/test/helpers/Assertions.t.sol b/test/helpers/Assertions.t.sol index 53abcd97..39b27ecd 100644 --- a/test/helpers/Assertions.t.sol +++ b/test/helpers/Assertions.t.sol @@ -5,24 +5,35 @@ import { PRBTest } from "@prb/test/PRBTest.sol"; import { Lockup } from "@sablier/v2-core/types/DataTypes.sol"; abstract contract Assertions is PRBTest { - event LogArray(Lockup.Status[] value); - event LogNamedArray(string key, Lockup.Status[] value); + event LogArray(bytes4[] value); + event LogNamedArray(string key, bytes4[] value); - /// @dev Compares two `Lockup.Status` enum values. - function assertEq(Lockup.Status a, Lockup.Status b) internal { - assertEq(uint8(a), uint8(b), "status"); + /// @dev Compares two `bytes4[]` arrays. + function assertEq(bytes4[] memory a, bytes4[] memory b) internal { + if (keccak256(abi.encode(a)) != keccak256(abi.encode(b))) { + emit Log("Error: a == b not satisfied [bytes4[]]"); + emit LogNamedArray(" Left", a); + emit LogNamedArray(" Right", b); + fail(); + } } - /// @dev Compares two `Lockup.Status[]` enum arrays. - function assertEq(Lockup.Status[] memory a, Lockup.Status[] memory b) internal { + /// @dev Compares two `bytes4[]` arrays. + function assertEq(bytes4[] memory a, bytes4[] memory b, string memory err) internal { if (keccak256(abi.encode(a)) != keccak256(abi.encode(b))) { - emit Log("Error: a == b not satisfied [Lockup.Status[]]"); + emit LogNamedString("Error", err); + emit Log("Error: a == b not satisfied [bytes4[]]"); emit LogNamedArray(" Left", a); emit LogNamedArray(" Right", b); fail(); } } + /// @dev Compares two `Lockup.Status` enum values. + function assertEq(Lockup.Status a, Lockup.Status b) internal { + assertEq(uint8(a), uint8(b), "status"); + } + /// @dev Compares two `Lockup.Status` enum values. function assertEq(Lockup.Status a, Lockup.Status b, string memory err) internal { assertEq(uint8(a), uint8(b), err); diff --git a/test/unit/method-list/methodList.t.sol b/test/unit/method-list/methodList.t.sol new file mode 100644 index 00000000..3d4cac9c --- /dev/null +++ b/test/unit/method-list/methodList.t.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.19 <0.9.0; + +import { ISablierV2LockupSender } from "@sablier/v2-core/interfaces/hooks/ISablierV2LockupSender.sol"; + +import { Base_Test } from "../../Base.t.sol"; + +contract MethodList_Unit_Test is Base_Test { + function test_MethodList() external { + bytes4[] memory functionSig = new bytes4[](1); + functionSig[0] = ISablierV2LockupSender.onStreamCanceled.selector; + + bytes4[] memory actualMethodList = target.methodList(); + bytes4[] memory expectedMethodList = functionSig; + assertEq(actualMethodList, expectedMethodList, "method list does not match"); + } +} From d6252c2de8be529e3f22fda4590a4a12f1e93fe7 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Thu, 20 Apr 2023 21:01:14 +0300 Subject: [PATCH 5/6] build: update v2-core --- .gitmodules | 2 +- lib/v2-core | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitmodules b/.gitmodules index 0328cdbc..34e16199 100644 --- a/.gitmodules +++ b/.gitmodules @@ -19,6 +19,6 @@ path = "lib/prb-test" url = "https://github.com/PaulRBerg/prb-test" [submodule "lib/v2-core"] - branch = "fix-hooks" + branch = "main" path = "lib/v2-core" url = "https://github.com/sablierhq/v2-core" diff --git a/lib/v2-core b/lib/v2-core index b343c608..2b71d076 160000 --- a/lib/v2-core +++ b/lib/v2-core @@ -1 +1 @@ -Subproject commit b343c60823e4f7a3e7b7be6af686fabb876f704f +Subproject commit 2b71d0762706ad04cdad58e37d2c6db0b6274496 From cec304484d292d7021013301a46ae4c80badc8a3 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Thu, 20 Apr 2023 21:13:13 +0300 Subject: [PATCH 6/6] refactor: add recipient parameter in hook test: write comment in the same line --- src/SablierV2ProxyTarget.sol | 2 ++ test/unit/on-stream-canceled/onStreamCanceled.t.sol | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/SablierV2ProxyTarget.sol b/src/SablierV2ProxyTarget.sol index a6bb38a1..16c9a0b9 100644 --- a/src/SablierV2ProxyTarget.sol +++ b/src/SablierV2ProxyTarget.sol @@ -118,11 +118,13 @@ contract SablierV2ProxyTarget is ISablierV2ProxyTarget, IPRBProxyPlugin, ISablie function onStreamCanceled( ISablierV2Lockup lockup, uint256 streamId, + address recipient, uint128 senderAmount, uint128 recipientAmount ) external { + recipient; // silence the warning recipientAmount; // silence the warning IERC20 asset = lockup.getAsset(streamId); diff --git a/test/unit/on-stream-canceled/onStreamCanceled.t.sol b/test/unit/on-stream-canceled/onStreamCanceled.t.sol index 8485f0d7..4ba85424 100644 --- a/test/unit/on-stream-canceled/onStreamCanceled.t.sol +++ b/test/unit/on-stream-canceled/onStreamCanceled.t.sol @@ -16,8 +16,7 @@ contract OnStreamCanceled_Unit_Test is Base_Test { uint256 balanceBefore = dai.balanceOf(users.sender.addr); // Asset flow: Sablier contract → proxy → proxy owner - // Expect transfers from the Sablier contract to the proxy, and then from the proxy to the - // proxy owner. + // Expect transfers from the Sablier contract to the proxy, and then from the proxy to the proxy owner. expectCallToTransfer({ to: address(proxy), amount: defaults.REFUND_AMOUNT() }); expectCallToTransfer({ to: users.sender.addr, amount: defaults.REFUND_AMOUNT() });