From b9273fe54915d86449281967a31a54aa1bac6af0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Tue, 29 Aug 2023 18:06:41 -0300 Subject: [PATCH 01/11] feat: add support for recurring payments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- contracts/contracts/Subscriptions.sol | 39 +++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/contracts/contracts/Subscriptions.sol b/contracts/contracts/Subscriptions.sol index acd95e7..a6a6941 100644 --- a/contracts/contracts/Subscriptions.sol +++ b/contracts/contracts/Subscriptions.sol @@ -56,6 +56,7 @@ contract Subscriptions is Ownable { uint128 rate ); event Unsubscribe(address indexed user, uint256 indexed epoch); + event Extend(address indexed user, uint64 oldEnd, uint64 newEnd, uint256 amount); event PendingSubscriptionCreated( address indexed user, uint256 indexed epoch, @@ -218,6 +219,44 @@ contract Subscriptions is Ownable { emit AuthorizedSignerRemoved(user, _signer); } + /// @notice Create a subscription for the tx origin address. + /// Will override an active subscription if one exists. + /// @dev The function's name and signature, `create`, are used to comply with the `IPayment` + /// interface for recurring payments. + /// @param user Subscription owner. Must match tx.origin. + /// @param data Encoded start, end and rate for the new subscription. + function create(address user, bytes calldata data) public { + require( + user == tx.origin, + 'Can only create subscription for calling EOA' + ); + (uint64 start, uint64 end, uint128 rate) = abi.decode( + data, + (uint64, uint64, uint128) + ); + _subscribe(user, start, end, rate); + } + + /// @notice Extends an active subscription end time. + /// The time the subscription will be extended by is calculated as `amount / rate`, where + /// `rate` is the existing subscription rate and `amount` is the new amount of tokens provided. + /// @dev The function's name, `addTo`, is used to comply with the `IPayment` interface for recurring payments. + /// @param user Subscription owner. + /// @param amount Total amount to be added to the subscription. + function addTo(address user, uint256 amount) public { + require(amount > 0, 'amount must be positive'); + + Subscription memory sub = subscriptions[user]; + require(sub.start != 0, 'no active subscription'); + require(sub.rate != 0, 'cannot extend a zero rate subscription'); + + uint64 oldEnd = sub.end; + uint64 newEnd = oldEnd + uint64(amount / sub.rate); + _subscribe(user, sub.start, newEnd, sub.rate); + + emit Extend(user, oldEnd, newEnd, amount); + } + /// @param _user Subscription owner. /// @param _signer Address authorized to sign messages on the owners behalf. /// @return isAuthorized True if the given signer is set as an authorized signer for the given From b9555338736118318b3be2ead52f724fcab9d0ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Tue, 29 Aug 2023 21:10:36 -0300 Subject: [PATCH 02/11] fix: add onlyRecurringPayments modifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- contracts/contracts/Subscriptions.sol | 56 +++++++++++++++++++++------ 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/contracts/contracts/Subscriptions.sol b/contracts/contracts/Subscriptions.sol index a6a6941..b12a01f 100644 --- a/contracts/contracts/Subscriptions.sol +++ b/contracts/contracts/Subscriptions.sol @@ -45,9 +45,11 @@ contract Subscriptions is Ownable { mapping(address => mapping(address => bool)) public authorizedSigners; /// @notice Mapping of user to pending subscription. mapping(address => Subscription) public pendingSubscriptions; + /// @notice Address of the recurring payments contract. + address public recurringPayments; // -- Events -- - event Init(address token, uint64 epochSeconds); + event Init(address token, uint64 epochSeconds, address recurringPayments); event Subscribe( address indexed user, uint256 indexed epoch, @@ -56,7 +58,12 @@ contract Subscriptions is Ownable { uint128 rate ); event Unsubscribe(address indexed user, uint256 indexed epoch); - event Extend(address indexed user, uint64 oldEnd, uint64 newEnd, uint256 amount); + event Extend( + address indexed user, + uint64 oldEnd, + uint64 newEnd, + uint256 amount + ); event PendingSubscriptionCreated( address indexed user, uint256 indexed epoch, @@ -78,17 +85,31 @@ contract Subscriptions is Ownable { uint256 indexed startEpoch, uint256 indexed endEpoch ); + event RecurringPaymentsUpdated(address indexed recurringPayments); + + modifier onlyRecurringPayments() { + require( + msg.sender == recurringPayments, + 'caller is not the recurring payments contract' + ); + _; + } // -- Functions -- /// @param _token The ERC-20 token held by this contract /// @param _epochSeconds The Duration of each epoch in seconds. /// @dev Contract ownership must be transfered to the gateway after deployment. - constructor(address _token, uint64 _epochSeconds) { + constructor( + address _token, + uint64 _epochSeconds, + address _recurringPayments + ) { token = IERC20(_token); epochSeconds = _epochSeconds; uncollectedEpoch = block.timestamp / _epochSeconds; + _setRecurringPayments(_recurringPayments); - emit Init(_token, _epochSeconds); + emit Init(_token, _epochSeconds, _recurringPayments); } /// @notice Create a subscription for the sender. @@ -221,15 +242,14 @@ contract Subscriptions is Ownable { /// @notice Create a subscription for the tx origin address. /// Will override an active subscription if one exists. - /// @dev The function's name and signature, `create`, are used to comply with the `IPayment` + /// @dev The function's name and signature, `create`, are used to comply with the `IPayment` /// interface for recurring payments. /// @param user Subscription owner. Must match tx.origin. /// @param data Encoded start, end and rate for the new subscription. - function create(address user, bytes calldata data) public { - require( - user == tx.origin, - 'Can only create subscription for calling EOA' - ); + function create( + address user, + bytes calldata data + ) public onlyRecurringPayments { (uint64 start, uint64 end, uint128 rate) = abi.decode( data, (uint64, uint64, uint128) @@ -238,7 +258,7 @@ contract Subscriptions is Ownable { } /// @notice Extends an active subscription end time. - /// The time the subscription will be extended by is calculated as `amount / rate`, where + /// The time the subscription will be extended by is calculated as `amount / rate`, where /// `rate` is the existing subscription rate and `amount` is the new amount of tokens provided. /// @dev The function's name, `addTo`, is used to comply with the `IPayment` interface for recurring payments. /// @param user Subscription owner. @@ -251,12 +271,16 @@ contract Subscriptions is Ownable { require(sub.rate != 0, 'cannot extend a zero rate subscription'); uint64 oldEnd = sub.end; - uint64 newEnd = oldEnd + uint64(amount / sub.rate); + uint64 newEnd = oldEnd + uint64(amount / sub.rate); _subscribe(user, sub.start, newEnd, sub.rate); emit Extend(user, oldEnd, newEnd, amount); } + function setRecurringPayments(address _recurringPayments) public onlyOwner { + _setRecurringPayments(_recurringPayments); + } + /// @param _user Subscription owner. /// @param _signer Address authorized to sign messages on the owners behalf. /// @return isAuthorized True if the given signer is set as an authorized signer for the given @@ -343,6 +367,14 @@ contract Subscriptions is Ownable { return unlocked(sub.start, sub.end, sub.rate); } + /// @notice Sets the recurring payments contract address. + /// @param _recurringPayments Address of the recurring payments contract. + function _setRecurringPayments(address _recurringPayments) private { + require(_recurringPayments != address(0), 'recurringPayments cannot be zero address'); + recurringPayments = _recurringPayments; + emit RecurringPaymentsUpdated(_recurringPayments); + } + /// @notice Create a subscription for a user /// Will override an active subscription if one exists. /// @param user Owner for the new subscription. From 362b29da5983852db66b1f830af4bcd024d57ce6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Thu, 31 Aug 2023 17:38:02 -0300 Subject: [PATCH 03/11] fix: dont clamp start date in internal method, be explicit about it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- contracts/contracts/Subscriptions.sol | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/contracts/contracts/Subscriptions.sol b/contracts/contracts/Subscriptions.sol index b12a01f..a60682f 100644 --- a/contracts/contracts/Subscriptions.sol +++ b/contracts/contracts/Subscriptions.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.17; - +import 'hardhat/console.sol'; import '@openzeppelin/contracts/access/Ownable.sol'; import '@openzeppelin/contracts/token/ERC20/IERC20.sol'; import '@openzeppelin/contracts/utils/math/Math.sol'; @@ -114,10 +114,14 @@ contract Subscriptions is Ownable { /// @notice Create a subscription for the sender. /// Will override an active subscription if one exists. + /// @dev Setting a start time in the past will clamp it to the current block timestamp. + /// This protects users from paying for a subscription during a period of time they were + /// not able to use it. /// @param start Start timestamp for the new subscription. /// @param end End timestamp for the new subscription. /// @param rate Rate for the new subscription. function subscribe(uint64 start, uint64 end, uint128 rate) public { + start = uint64(Math.max(start, block.timestamp)); _subscribe(msg.sender, start, end, rate); } @@ -165,6 +169,9 @@ contract Subscriptions is Ownable { /// @notice Creates a subscription template without requiring funds. Expected to be used with /// `fulfil`. + /// @dev Setting a start time in the past will clamp it to the current block timestamp when fulfilled. + /// This protects users from paying for a subscription during a period of time they were + /// not able to use it. /// @param start Start timestamp for the pending subscription. /// @param end End timestamp for the pending subscription. /// @param rate Rate for the pending subscription. @@ -244,6 +251,7 @@ contract Subscriptions is Ownable { /// Will override an active subscription if one exists. /// @dev The function's name and signature, `create`, are used to comply with the `IPayment` /// interface for recurring payments. + /// @dev Note that this function does not protect user against a start time in the past. /// @param user Subscription owner. Must match tx.origin. /// @param data Encoded start, end and rate for the new subscription. function create( @@ -265,6 +273,7 @@ contract Subscriptions is Ownable { /// @param amount Total amount to be added to the subscription. function addTo(address user, uint256 amount) public { require(amount > 0, 'amount must be positive'); + require(user != address(0), 'user is null'); Subscription memory sub = subscriptions[user]; require(sub.start != 0, 'no active subscription'); @@ -272,7 +281,14 @@ contract Subscriptions is Ownable { uint64 oldEnd = sub.end; uint64 newEnd = oldEnd + uint64(amount / sub.rate); - _subscribe(user, sub.start, newEnd, sub.rate); + + _setEpochs(sub.start, sub.end, -int128(sub.rate)); + _setEpochs(sub.start, newEnd, int128(sub.rate)); + + subscriptions[user].end = newEnd; + + bool success = token.transferFrom(msg.sender, address(this), amount); + require(success, 'IERC20 token transfer failed'); emit Extend(user, oldEnd, newEnd, amount); } @@ -377,6 +393,8 @@ contract Subscriptions is Ownable { /// @notice Create a subscription for a user /// Will override an active subscription if one exists. + /// @dev Note that setting a start time in the past is allowed. If this behavior is not desired, + /// the caller can clamp the start time to the current block timestamp. /// @param user Owner for the new subscription. /// @param start Start timestamp for the new subscription. /// @param end End timestamp for the new subscription. @@ -389,7 +407,6 @@ contract Subscriptions is Ownable { ) private { require(user != address(0), 'user is null'); require(user != address(this), 'invalid user'); - start = uint64(Math.max(start, block.timestamp)); require(start < end, 'start must be less than end'); // This avoids unexpected behavior from truncation, especially in `locked` and `unlocked`. From ab4a467188a79b9d355e7f64acd663405bdaa93f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Thu, 31 Aug 2023 17:43:34 -0300 Subject: [PATCH 04/11] fix: tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- contracts/build/Subscriptions.abi | 117 ++++++++++++++++++++++++++ contracts/contracts/Subscriptions.sol | 4 +- contracts/test/contract.test.ts | 5 +- 3 files changed, 122 insertions(+), 4 deletions(-) diff --git a/contracts/build/Subscriptions.abi b/contracts/build/Subscriptions.abi index 639fc5d..1f520fe 100644 --- a/contracts/build/Subscriptions.abi +++ b/contracts/build/Subscriptions.abi @@ -10,6 +10,11 @@ "internalType": "uint64", "name": "_epochSeconds", "type": "uint64" + }, + { + "internalType": "address", + "name": "_recurringPayments", + "type": "address" } ], "stateMutability": "nonpayable", @@ -53,6 +58,37 @@ "name": "AuthorizedSignerRemoved", "type": "event" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "user", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint64", + "name": "oldEnd", + "type": "uint64" + }, + { + "indexed": false, + "internalType": "uint64", + "name": "newEnd", + "type": "uint64" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "Extend", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -67,6 +103,12 @@ "internalType": "uint64", "name": "epochSeconds", "type": "uint64" + }, + { + "indexed": false, + "internalType": "address", + "name": "recurringPayments", + "type": "address" } ], "name": "Init", @@ -128,6 +170,19 @@ "name": "PendingSubscriptionCreated", "type": "event" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "recurringPayments", + "type": "address" + } + ], + "name": "RecurringPaymentsUpdated", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -228,6 +283,24 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "user", + "type": "address" + }, + { + "internalType": "uint256", + "name": "amount", + "type": "uint256" + } + ], + "name": "addTo", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { @@ -309,6 +382,24 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "user", + "type": "address" + }, + { + "internalType": "bytes", + "name": "data", + "type": "bytes" + } + ], + "name": "create", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [], "name": "currentEpoch", @@ -467,6 +558,19 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "recurringPayments", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -510,6 +614,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "_recurringPayments", + "type": "address" + } + ], + "name": "setRecurringPayments", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { diff --git a/contracts/contracts/Subscriptions.sol b/contracts/contracts/Subscriptions.sol index a60682f..4da942d 100644 --- a/contracts/contracts/Subscriptions.sol +++ b/contracts/contracts/Subscriptions.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.17; -import 'hardhat/console.sol'; + import '@openzeppelin/contracts/access/Ownable.sol'; import '@openzeppelin/contracts/token/ERC20/IERC20.sol'; import '@openzeppelin/contracts/utils/math/Math.sol'; @@ -210,7 +210,7 @@ contract Subscriptions is Ownable { ); // Create the subscription using the pending subscription details - _subscribe(_to, pendingSub.start, pendingSub.end, pendingSub.rate); + _subscribe(_to, subStart, pendingSub.end, pendingSub.rate); delete pendingSubscriptions[_to]; // Send any extra tokens back to the user diff --git a/contracts/test/contract.test.ts b/contracts/test/contract.test.ts index 55f0094..1c04b90 100644 --- a/contracts/test/contract.test.ts +++ b/contracts/test/contract.test.ts @@ -26,6 +26,7 @@ describe('Subscriptions contract', () => { let subscriber1: Account; let subscriber2: Account; let subscriberNoFunds: Account; + let recurringPayments: Account; // Contracts let subscriptions: Subscriptions; @@ -36,7 +37,7 @@ describe('Subscriptions contract', () => { before(async function () { // eslint-disable-next-line @typescript-eslint/no-extra-semi - [deployer, subscriber1, subscriber2, subscriberNoFunds] = + [deployer, subscriber1, subscriber2, subscriberNoFunds, recurringPayments] = await getAccounts(); setAutoMine(true); @@ -49,7 +50,7 @@ describe('Subscriptions contract', () => { false ); subscriptions = await deployment.deploySubscriptions( - [stableToken.address, subscriptionsEpochSeconds], + [stableToken.address, subscriptionsEpochSeconds, recurringPayments.address], deployer.signer, false ); From 28eec516e407bb477b79bd886ceb96173ea51dad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Mon, 4 Sep 2023 16:35:16 -0300 Subject: [PATCH 05/11] feat(rp): add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- contracts/contracts/Subscriptions.sol | 14 +- contracts/test/contract.test.ts | 357 +++++++++++++++++++++++++- 2 files changed, 363 insertions(+), 8 deletions(-) diff --git a/contracts/contracts/Subscriptions.sol b/contracts/contracts/Subscriptions.sol index 4da942d..6ce1b4f 100644 --- a/contracts/contracts/Subscriptions.sol +++ b/contracts/contracts/Subscriptions.sol @@ -247,12 +247,12 @@ contract Subscriptions is Ownable { emit AuthorizedSignerRemoved(user, _signer); } - /// @notice Create a subscription for the tx origin address. + /// @notice Create a subscription for a user. /// Will override an active subscription if one exists. /// @dev The function's name and signature, `create`, are used to comply with the `IPayment` /// interface for recurring payments. /// @dev Note that this function does not protect user against a start time in the past. - /// @param user Subscription owner. Must match tx.origin. + /// @param user Subscription owner. /// @param data Encoded start, end and rate for the new subscription. function create( address user, @@ -265,7 +265,7 @@ contract Subscriptions is Ownable { _subscribe(user, start, end, rate); } - /// @notice Extends an active subscription end time. + /// @notice Extends a subscription's end time. /// The time the subscription will be extended by is calculated as `amount / rate`, where /// `rate` is the existing subscription rate and `amount` is the new amount of tokens provided. /// @dev The function's name, `addTo`, is used to comply with the `IPayment` interface for recurring payments. @@ -276,11 +276,12 @@ contract Subscriptions is Ownable { require(user != address(0), 'user is null'); Subscription memory sub = subscriptions[user]; - require(sub.start != 0, 'no active subscription'); + require(sub.start != 0, 'no subscription found'); require(sub.rate != 0, 'cannot extend a zero rate subscription'); uint64 oldEnd = sub.end; uint64 newEnd = oldEnd + uint64(amount / sub.rate); + require(block.timestamp < newEnd, 'new end cannot be in the past'); _setEpochs(sub.start, sub.end, -int128(sub.rate)); _setEpochs(sub.start, newEnd, int128(sub.rate)); @@ -386,7 +387,10 @@ contract Subscriptions is Ownable { /// @notice Sets the recurring payments contract address. /// @param _recurringPayments Address of the recurring payments contract. function _setRecurringPayments(address _recurringPayments) private { - require(_recurringPayments != address(0), 'recurringPayments cannot be zero address'); + require( + _recurringPayments != address(0), + 'recurringPayments cannot be zero address' + ); recurringPayments = _recurringPayments; emit RecurringPaymentsUpdated(_recurringPayments); } diff --git a/contracts/test/contract.test.ts b/contracts/test/contract.test.ts index 1c04b90..66446ef 100644 --- a/contracts/test/contract.test.ts +++ b/contracts/test/contract.test.ts @@ -27,6 +27,7 @@ describe('Subscriptions contract', () => { let subscriber2: Account; let subscriberNoFunds: Account; let recurringPayments: Account; + let newRecurringPayments: Account; // Contracts let subscriptions: Subscriptions; @@ -37,8 +38,14 @@ describe('Subscriptions contract', () => { before(async function () { // eslint-disable-next-line @typescript-eslint/no-extra-semi - [deployer, subscriber1, subscriber2, subscriberNoFunds, recurringPayments] = - await getAccounts(); + [ + deployer, + subscriber1, + subscriber2, + subscriberNoFunds, + recurringPayments, + newRecurringPayments, + ] = await getAccounts(); setAutoMine(true); }); @@ -50,7 +57,11 @@ describe('Subscriptions contract', () => { false ); subscriptions = await deployment.deploySubscriptions( - [stableToken.address, subscriptionsEpochSeconds, recurringPayments.address], + [ + stableToken.address, + subscriptionsEpochSeconds, + recurringPayments.address, + ], deployer.signer, false ); @@ -62,6 +73,9 @@ describe('Subscriptions contract', () => { await stableToken .connect(deployer.signer) .transfer(subscriber2.address, oneMillion); + await stableToken + .connect(deployer.signer) + .transfer(recurringPayments.address, oneMillion); // Approve the subscription contract to transfer tokens from the user await stableToken @@ -70,6 +84,9 @@ describe('Subscriptions contract', () => { await stableToken .connect(subscriber2.signer) .approve(subscriptions.address, oneMillion); + await stableToken + .connect(recurringPayments.signer) + .approve(subscriptions.address, oneMillion); }); describe('constructor', function () { @@ -86,6 +103,41 @@ describe('Subscriptions contract', () => { subscriptionsEpochSeconds ); }); + + it('should set the recurring payments address', async function () { + expect(await subscriptions.recurringPayments()).to.eq( + recurringPayments.address + ); + }); + }); + + describe('setters', function () { + it('should set the recurring payments address', async function () { + const tx = subscriptions.setRecurringPayments( + newRecurringPayments.address + ); + + await expect(tx) + .to.emit(subscriptions, 'RecurringPaymentsUpdated') + .withArgs(newRecurringPayments.address); + expect(await subscriptions.recurringPayments()).to.eq( + newRecurringPayments.address + ); + }); + + it('should prevent unauthorized users to change the recurring payments addressd', async function () { + const tx = subscriptions + .connect(subscriber1.signer) + .setRecurringPayments(newRecurringPayments.address); + await expect(tx).revertedWith('Ownable: caller is not the owner'); + }); + + it('should prevent setting the recurring payments address to zero address', async function () { + const tx = subscriptions.setRecurringPayments( + ethers.constants.AddressZero + ); + await expect(tx).revertedWith('recurringPayments cannot be zero address'); + }); }); describe('transferOwnership', function () { @@ -412,6 +464,7 @@ describe('Subscriptions contract', () => { ).revertedWith('end too large'); }); }); + describe('unsubscribe', function () { it('should allow user to cancel an active subscription', async function () { const now = await latestBlockTimestamp(); @@ -510,6 +563,183 @@ describe('Subscriptions contract', () => { }); }); + describe('create', function () { + it('should create a subscription for a user', async function () { + const now = await latestBlockTimestamp(); + const start = now.sub(10); + const end = now.add(510); + const rate = BigNumber.from(5); + const data = ethers.utils.defaultAbiCoder.encode( + ['uint64', 'uint64', 'uint128'], + [start, end, rate] + ); + + await create( + stableToken, + subscriptions, + recurringPayments, + subscriber1.address, + data + ); + }); + + it('should prevent unauthorized users to call create', async function () { + const now = await latestBlockTimestamp(); + const start = now.sub(10); + const end = now.add(510); + const rate = BigNumber.from(5); + const data = ethers.utils.defaultAbiCoder.encode( + ['uint64', 'uint64', 'uint128'], + [start, end, rate] + ); + + const tx = subscriptions + .connect(subscriber1.signer) + .create(subscriber1.address, data); + await expect(tx).revertedWith( + 'caller is not the recurring payments contract' + ); + }); + }); + + describe('addTo', function () {}); + + describe('extend', function () { + it('should revert if the amount to extend is zero', async function () { + const tx = subscriptions.addTo( + ethers.constants.AddressZero, + BigNumber.from(0) + ); + await expect(tx).revertedWith('amount must be positive'); + }); + + it('should revert if user is the zero address', async function () { + const tx = subscriptions.addTo( + ethers.constants.AddressZero, + BigNumber.from(1000) + ); + await expect(tx).revertedWith('user is null'); + }); + + it('should revert when extending a subscription that does not exist', async function () { + const tx = subscriptions.addTo(subscriber2.address, BigNumber.from(10)); + await expect(tx).revertedWith('no subscription found'); + }); + + it('should revert when the new end time is in the past', async function () { + const now = await latestBlockTimestamp(); + const start = now.add(500); + const end = now.add(1000); + const rate = BigNumber.from(5); + const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/5 = 1000 + 400 = 1400 + + const subscribeBlockNumber = await subscribe( + stableToken, + subscriptions, + subscriber1, + start, + end, + rate + ); + + // mine past the newEnd + await mineNBlocks(1500); + + const tx = addToSubscription( + stableToken, + subscriptions, + recurringPayments, + subscriber1.address, + amountToExtend, + subscribeBlockNumber + ); + await expect(tx).revertedWith('new end cannot be in the past'); + }); + + it('should allow extending an active subscription', async function () { + const now = await latestBlockTimestamp(); + const start = now; + const end = now.add(1000); + const rate = BigNumber.from(5); + const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/5 = 1000 + 400 = 1400 const user = subscriber2.address; + + const subscribeBlockNumber = await subscribe( + stableToken, + subscriptions, + subscriber1, + start, + end, + rate + ); + + // mine past the start of the subscription + await mineNBlocks(150); + + await addToSubscription( + stableToken, + subscriptions, + recurringPayments, + subscriber1.address, + amountToExtend, + subscribeBlockNumber + ); + }); + + it('should allow extending an expired subscription', async function () { + const now = await latestBlockTimestamp(); + const start = now; + const end = now.add(1000); + const rate = BigNumber.from(5); + const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/5 = 1000 + 400 = 1400 const user = subscriber2.address; + + const subscribeBlockNumber = await subscribe( + stableToken, + subscriptions, + subscriber1, + start, + end, + rate + ); + + // mine past the end of the subscription, but not past the new end + await mineNBlocks(1100); + + await addToSubscription( + stableToken, + subscriptions, + recurringPayments, + subscriber1.address, + amountToExtend, + subscribeBlockNumber + ); + }); + + it('should allow extending a one epoch subscription', async function () { + const now = await latestBlockTimestamp(); + const start = now; + const end = now.add(5); + const rate = BigNumber.from(5); + const amountToExtend = BigNumber.from(2000); + + const subscribeBlockNumber = await subscribe( + stableToken, + subscriptions, + subscriber1, + start, + end, + rate + ); + await addToSubscription( + stableToken, + subscriptions, + recurringPayments, + subscriber1.address, + amountToExtend, + subscribeBlockNumber + ); + }); + }); + describe.skip('collect', function () {}); describe('setPendingSubscription', function () { @@ -888,6 +1118,75 @@ async function unsubscribe( } } +async function create( + stableToken: StableToken, + subscriptions: Subscriptions, + signer: Account, + userAddress: string, + data: string +) { + const decoded = ethers.utils.defaultAbiCoder.decode( + ['uint64', 'uint64', 'uint128'], + data + ); + const start = decoded[0]; + const end = decoded[1]; + const rate = decoded[2]; + + const amount = rate.mul(end.sub(start)); + const epochSeconds = await subscriptions.epochSeconds(); + + // Before state + const beforeBlock = await latestBlockNumber(); + const beforeBalance = await stableToken.balanceOf(signer.address); + const beforeContractBalance = await stableToken.balanceOf( + subscriptions.address + ); + + // * Tx + const tx = subscriptions.connect(signer.signer).create(userAddress, data); + await tx; + const txTimestamp = await time.latest(); + const txEpoch = BigNumber.from(txTimestamp).div(epochSeconds).add(1); + + // * Check events + await expect(tx) + .to.emit(subscriptions, 'Subscribe') + .withArgs(userAddress, txEpoch, start, end, rate); + + // * Check balances + const afterBalance = await stableToken.balanceOf(signer.address); + const afterContractBalance = await stableToken.balanceOf( + subscriptions.address + ); + + // Actual amount deposited might be less than intended if subStart < block.number + const amountDeposited = beforeBalance.sub(afterBalance); + expect(amountDeposited).to.lte(amount); + expect(afterContractBalance).to.eq( + beforeContractBalance.add(amountDeposited) + ); + + // * Check state + const sub = await subscriptions.subscriptions(userAddress); + expect(sub.start).to.eq(start); + expect(sub.end).to.eq(end); + expect(sub.rate).to.eq(rate); + + const afterBlock = await latestBlockNumber(); + + await testEpochDetails( + subscriptions, + start, + end, + rate, + beforeBlock, + afterBlock + ); + + return (await tx).blockNumber!; +} + async function testEpochDetails( subscriptions: Subscriptions, start: BigNumber, @@ -1094,3 +1393,55 @@ async function fulfil( return (await tx).blockNumber!; } + +async function addToSubscription( + stableToken: StableToken, + subscriptions: Subscriptions, + signer: Account, + user: string, + amount: BigNumber, + subscribeBlockNumber: number | undefined +) { + // Before state + const beforeSub = await subscriptions.subscriptions(user); + const beforeBalance = await stableToken.balanceOf(signer.address); + const beforeContractBalance = await stableToken.balanceOf( + subscriptions.address + ); + const newEnd = beforeSub.end.add(amount.div(beforeSub.rate)); + // const additionalTokens = beforeSub.rate.mul(newEnd.sub(beforeSub.end)); + + // * Tx + const tx = subscriptions.connect(signer.signer).addTo(user, amount); + + // * Check events + await expect(tx) + .to.emit(subscriptions, 'Extend') + .withArgs(user, beforeSub.end, newEnd, amount); + + // * Check balances + const afterBalance = await stableToken.balanceOf(signer.address); + const afterContractBalance = await stableToken.balanceOf( + subscriptions.address + ); + expect(afterBalance).to.eq(beforeBalance.sub(amount)); + expect(afterContractBalance).to.eq( + beforeContractBalance.add(amount) + ); + + // * Check state + const afterSub = await subscriptions.subscriptions(user); + expect(afterSub.start).to.eq(beforeSub.start); + expect(afterSub.end).to.eq(newEnd); + expect(afterSub.rate).to.eq(beforeSub.rate); + + // Sub + extend -> Epoch changes should match those of a sub [start, newEnd) + await testEpochDetails( + subscriptions, + beforeSub.start, + newEnd, + beforeSub.rate, + BigNumber.from(subscribeBlockNumber! - 1), + BigNumber.from((await tx).blockNumber!) + ); +} From cbdf479cc8aa72865639dd9faa38708d25824aa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Mon, 4 Sep 2023 16:36:56 -0300 Subject: [PATCH 06/11] chore: clarify addTo behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- contracts/contracts/Subscriptions.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/contracts/Subscriptions.sol b/contracts/contracts/Subscriptions.sol index 6ce1b4f..7bbdf15 100644 --- a/contracts/contracts/Subscriptions.sol +++ b/contracts/contracts/Subscriptions.sol @@ -268,6 +268,7 @@ contract Subscriptions is Ownable { /// @notice Extends a subscription's end time. /// The time the subscription will be extended by is calculated as `amount / rate`, where /// `rate` is the existing subscription rate and `amount` is the new amount of tokens provided. + /// The new end time must be in the future. /// @dev The function's name, `addTo`, is used to comply with the `IPayment` interface for recurring payments. /// @param user Subscription owner. /// @param amount Total amount to be added to the subscription. From 89f83bac461d3665dff98086abf3c8177dc3effb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 8 Sep 2023 11:25:13 -0300 Subject: [PATCH 07/11] Update contracts/test/contract.test.ts Co-authored-by: Theo Butler --- contracts/test/contract.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/test/contract.test.ts b/contracts/test/contract.test.ts index 66446ef..766149b 100644 --- a/contracts/test/contract.test.ts +++ b/contracts/test/contract.test.ts @@ -125,7 +125,7 @@ describe('Subscriptions contract', () => { ); }); - it('should prevent unauthorized users to change the recurring payments addressd', async function () { + it('should prevent unauthorized users from changing the recurring payments address', async function () { const tx = subscriptions .connect(subscriber1.signer) .setRecurringPayments(newRecurringPayments.address); From d65598bbc715912c6a961f681a342aaf31163ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 8 Sep 2023 11:25:23 -0300 Subject: [PATCH 08/11] Update contracts/test/contract.test.ts Co-authored-by: Theo Butler --- contracts/test/contract.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/test/contract.test.ts b/contracts/test/contract.test.ts index 766149b..1181281 100644 --- a/contracts/test/contract.test.ts +++ b/contracts/test/contract.test.ts @@ -583,7 +583,7 @@ describe('Subscriptions contract', () => { ); }); - it('should prevent unauthorized users to call create', async function () { + it('should prevent unauthorized users from calling create', async function () { const now = await latestBlockTimestamp(); const start = now.sub(10); const end = now.add(510); From 27776ab09c86de0561ef41c4e6c0ba9d7c20ab37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 8 Sep 2023 11:31:15 -0300 Subject: [PATCH 09/11] chore: fix typos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- contracts/test/contract.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/contracts/test/contract.test.ts b/contracts/test/contract.test.ts index 1181281..3813b03 100644 --- a/contracts/test/contract.test.ts +++ b/contracts/test/contract.test.ts @@ -602,8 +602,6 @@ describe('Subscriptions contract', () => { }); }); - describe('addTo', function () {}); - describe('extend', function () { it('should revert if the amount to extend is zero', async function () { const tx = subscriptions.addTo( @@ -661,7 +659,7 @@ describe('Subscriptions contract', () => { const start = now; const end = now.add(1000); const rate = BigNumber.from(5); - const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/5 = 1000 + 400 = 1400 const user = subscriber2.address; + const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/5 = 1000 + 400 = 1400 const subscribeBlockNumber = await subscribe( stableToken, @@ -690,7 +688,7 @@ describe('Subscriptions contract', () => { const start = now; const end = now.add(1000); const rate = BigNumber.from(5); - const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/5 = 1000 + 400 = 1400 const user = subscriber2.address; + const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/5 = 1000 + 400 = 1400 const subscribeBlockNumber = await subscribe( stableToken, From 435c70034f125b7a9f27a723a6fa0984956e8274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Tue, 19 Sep 2023 12:11:30 -0300 Subject: [PATCH 10/11] fix: when extending make sure user does not pay for time in the past (0xM H-01) (#86) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: when extending make sure user does not pay for time in the past (0xM H-01) Signed-off-by: Tomás Migone * fix: revert when extending if amount is not multiple of rate (0xM L-02) (#87) Signed-off-by: Tomás Migone --- contracts/contracts/Subscriptions.sol | 10 +++---- contracts/test/contract.test.ts | 40 +++++++++++++++++++-------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/contracts/contracts/Subscriptions.sol b/contracts/contracts/Subscriptions.sol index 7bbdf15..70ffc82 100644 --- a/contracts/contracts/Subscriptions.sol +++ b/contracts/contracts/Subscriptions.sol @@ -268,7 +268,7 @@ contract Subscriptions is Ownable { /// @notice Extends a subscription's end time. /// The time the subscription will be extended by is calculated as `amount / rate`, where /// `rate` is the existing subscription rate and `amount` is the new amount of tokens provided. - /// The new end time must be in the future. + /// If the subscription was expired the extension will start from the current block timestamp. /// @dev The function's name, `addTo`, is used to comply with the `IPayment` interface for recurring payments. /// @param user Subscription owner. /// @param amount Total amount to be added to the subscription. @@ -279,10 +279,10 @@ contract Subscriptions is Ownable { Subscription memory sub = subscriptions[user]; require(sub.start != 0, 'no subscription found'); require(sub.rate != 0, 'cannot extend a zero rate subscription'); + require(amount % sub.rate == 0, "amount not multiple of rate"); - uint64 oldEnd = sub.end; - uint64 newEnd = oldEnd + uint64(amount / sub.rate); - require(block.timestamp < newEnd, 'new end cannot be in the past'); + uint64 newEnd = uint64(Math.max(sub.end, block.timestamp)) + + uint64(amount / sub.rate); _setEpochs(sub.start, sub.end, -int128(sub.rate)); _setEpochs(sub.start, newEnd, int128(sub.rate)); @@ -292,7 +292,7 @@ contract Subscriptions is Ownable { bool success = token.transferFrom(msg.sender, address(this), amount); require(success, 'IERC20 token transfer failed'); - emit Extend(user, oldEnd, newEnd, amount); + emit Extend(user, sub.end, newEnd, amount); } function setRecurringPayments(address _recurringPayments) public onlyOwner { diff --git a/contracts/test/contract.test.ts b/contracts/test/contract.test.ts index 3813b03..73f3faa 100644 --- a/contracts/test/contract.test.ts +++ b/contracts/test/contract.test.ts @@ -3,7 +3,7 @@ import {time} from '@nomicfoundation/hardhat-network-helpers'; import {expect} from 'chai'; import * as deployment from '../utils/deploy'; -import {getAccounts, Account, toGRT} from '../utils/helpers'; +import {getAccounts, Account, toGRT, provider} from '../utils/helpers'; import {Subscriptions} from '../types/contracts/Subscriptions'; import {StableToken} from '../types/contracts/test/StableMock.sol/StableToken'; @@ -624,7 +624,7 @@ describe('Subscriptions contract', () => { await expect(tx).revertedWith('no subscription found'); }); - it('should revert when the new end time is in the past', async function () { + it('should allow extending an expired subscription', async function () { const now = await latestBlockTimestamp(); const start = now.add(500); const end = now.add(1000); @@ -643,7 +643,7 @@ describe('Subscriptions contract', () => { // mine past the newEnd await mineNBlocks(1500); - const tx = addToSubscription( + await addToSubscription( stableToken, subscriptions, recurringPayments, @@ -651,16 +651,29 @@ describe('Subscriptions contract', () => { amountToExtend, subscribeBlockNumber ); - await expect(tx).revertedWith('new end cannot be in the past'); }); - + + it('should not allow extending an active subscription if amount is not multiple of rate', async function () { + const now = await latestBlockTimestamp(); + const start = now; + const end = now.add(1000); + const rate = BigNumber.from(7); + const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/7 = 1000 + 286 = 1286 + + // mine past the start of the subscription + await mineNBlocks(150); + + const tx = subscriptions.addTo(subscriber1.address, amountToExtend); + await expect(tx).revertedWith('amount not multiple of rate'); + }); + it('should allow extending an active subscription', async function () { const now = await latestBlockTimestamp(); const start = now; const end = now.add(1000); const rate = BigNumber.from(5); const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/5 = 1000 + 400 = 1400 - + const subscribeBlockNumber = await subscribe( stableToken, subscriptions, @@ -689,7 +702,7 @@ describe('Subscriptions contract', () => { const end = now.add(1000); const rate = BigNumber.from(5); const amountToExtend = BigNumber.from(2000); // newEnd: end + 2000/5 = 1000 + 400 = 1400 - + const subscribeBlockNumber = await subscribe( stableToken, subscriptions, @@ -1406,13 +1419,18 @@ async function addToSubscription( const beforeContractBalance = await stableToken.balanceOf( subscriptions.address ); - const newEnd = beforeSub.end.add(amount.div(beforeSub.rate)); - // const additionalTokens = beforeSub.rate.mul(newEnd.sub(beforeSub.end)); // * Tx const tx = subscriptions.connect(signer.signer).addTo(user, amount); + const receipt = await (await tx).wait(); + const txTimestamp = ( + await subscriptions.provider.getBlock(receipt.blockNumber!) + ).timestamp; // * Check events + const newEnd = BigNumber.from( + Math.max(beforeSub.end.toNumber(), txTimestamp) + ).add(Math.ceil(amount.toNumber() / beforeSub.rate.toNumber())); await expect(tx) .to.emit(subscriptions, 'Extend') .withArgs(user, beforeSub.end, newEnd, amount); @@ -1423,9 +1441,7 @@ async function addToSubscription( subscriptions.address ); expect(afterBalance).to.eq(beforeBalance.sub(amount)); - expect(afterContractBalance).to.eq( - beforeContractBalance.add(amount) - ); + expect(afterContractBalance).to.eq(beforeContractBalance.add(amount)); // * Check state const afterSub = await subscriptions.subscriptions(user); From 00870ddacaf56d8263432ee3a55b4a1eb12f841e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Migone?= Date: Fri, 20 Oct 2023 10:32:15 -0500 Subject: [PATCH 11/11] chore: deployment files for new contract (#90) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomás Migone --- contracts/addresses.json | 2 +- contracts/tasks/deploy.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/contracts/addresses.json b/contracts/addresses.json index 183a093..2e5ff57 100644 --- a/contracts/addresses.json +++ b/contracts/addresses.json @@ -3,6 +3,6 @@ "Subscriptions": "0x482f58d3513E386036670404b35cB3F2DF67a750" }, "421613": { - "Subscriptions": "0x29f49a438c747e7Dd1bfe7926b03783E47f9447B" + "Subscriptions": "0x1c4053A0CEBfA529134CB9ddaE3C3D0B144384aA" } } diff --git a/contracts/tasks/deploy.ts b/contracts/tasks/deploy.ts index 23bd32e..887cbf5 100644 --- a/contracts/tasks/deploy.ts +++ b/contracts/tasks/deploy.ts @@ -6,6 +6,7 @@ import { deploySubscriptions } from '../utils/deploy' task('deploy', 'Deploy the subscription contract (use L2 network!)') .addParam('token', 'Address of the ERC20 token') + .addParam('recurringPayments', 'Address of the recurring payments contract') .addOptionalParam('epochSeconds', 'Epoch length in seconds.', 3, types.int) .setAction(async (taskArgs, hre: HardhatRuntimeEnvironment) => { const accounts = await hre.ethers.getSigners() @@ -16,7 +17,7 @@ task('deploy', 'Deploy the subscription contract (use L2 network!)') console.log('Deploying subscriptions contract with the account:', accounts[0].address); await deploySubscriptions( - [taskArgs.token, taskArgs.epochSeconds], + [taskArgs.token, taskArgs.epochSeconds, taskArgs.recurringPayments], accounts[0] as unknown as Wallet, ) }) \ No newline at end of file