Skip to content

fix(recurring): bind batch schedule key to signed terms #1754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LeoSlrRf wants to merge 1 commit into from
Closed

fix(recurring): bind batch schedule key to signed terms #1754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 79 additions & 12 deletions packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran
error ERC20RecurringPaymentProxy__ZeroAddress();
error ERC20RecurringPaymentProxy__TransferFailed();
error ERC20RecurringPaymentProxy__ShortPull();
error ERC20RecurringPaymentProxy__ZeroScheduleId();

bytes32 public constant RELAYER_ROLE = keccak256('RELAYER_ROLE');

Expand Down Expand Up @@ -172,6 +173,78 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran
}
}

function _scheduleKeyFromPermit(SchedulePermit calldata p) private pure returns (bytes32) {
return
keccak256(
abi.encode(
p.subscriber,
p.token,
p.recipient,
p.feeAddress,
p.amount,
p.feeAmount,
p.relayerFee,
p.periodSeconds,
p.firstPayment,
p.totalPayments,
p.strictOrder
)
);
}

function scheduleKeyFromPermit(SchedulePermit calldata p) public pure returns (bytes32) {
return _scheduleKeyFromPermit(p);
}

function _scheduleKeyFromBatch(SchedulePermitBatch calldata p) private pure returns (bytes32) {
if (p.scheduleId == bytes32(0)) revert ERC20RecurringPaymentProxy__ZeroScheduleId();
return
keccak256(
abi.encode(
p.subscriber,
p.scheduleId,
p.token,
p.relayerFee,
p.totalPayments,
p.strictOrder,
_hashUint32Array(p.dueTimes),
_hashLegs(p.initialLegs),
_hashLegs(p.recurringLegs)
)
);
}

function scheduleKeyFromBatch(SchedulePermitBatch calldata p) public pure returns (bytes32) {
return _scheduleKeyFromBatch(p);
}

function _assertUnpaid(bytes32 scheduleKey, uint8 index) private view {
if (triggeredPaymentsBitmap[scheduleKey] & (1 << index) != 0) {
revert ERC20RecurringPaymentProxy__AlreadyPaid();
}
}

function _assertOrder(
bytes32 scheduleKey,
uint8 index,
bool strictOrder
) private view {
if (strictOrder && index != lastPaymentIndex[scheduleKey] + 1) {
revert ERC20RecurringPaymentProxy__PaymentOutOfOrder();
}
}

function _markPaid(
bytes32 scheduleKey,
uint8 index,
bool strictOrder
) private {
triggeredPaymentsBitmap[scheduleKey] |= (1 << index);
if (strictOrder) {
lastPaymentIndex[scheduleKey] = index;
}
}

function _pullExact(
IERC20 token,
address from,
Expand Down Expand Up @@ -226,31 +299,25 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran
_assertSigner(p.subscriber, digest, signature);
if (block.timestamp > p.deadline) revert ERC20RecurringPaymentProxy__SignatureExpired();

if (index == 0) revert ERC20RecurringPaymentProxy__IndexOutOfBounds();
if (index >= 256) revert ERC20RecurringPaymentProxy__IndexTooLarge();

if (p.strictOrder) {
if (index != lastPaymentIndex[digest] + 1)
revert ERC20RecurringPaymentProxy__PaymentOutOfOrder();
lastPaymentIndex[digest] = index;
}

if (index > p.totalPayments) revert ERC20RecurringPaymentProxy__IndexOutOfBounds();

bytes32 scheduleKey = _scheduleKeyFromPermit(p);
_assertOrder(scheduleKey, index, p.strictOrder);
_assertUnpaid(scheduleKey, index);

uint256 execTime = uint256(p.firstPayment) + uint256(index - 1) * p.periodSeconds;
if (block.timestamp < execTime) revert ERC20RecurringPaymentProxy__NotDueYet();

uint256 mask = 1 << index;
uint256 word = triggeredPaymentsBitmap[digest];
if (word & mask != 0) revert ERC20RecurringPaymentProxy__AlreadyPaid();
triggeredPaymentsBitmap[digest] = word | mask;

uint256 total = p.amount + p.feeAmount + p.relayerFee;

IERC20 token = IERC20(p.token);
_pullExact(token, p.subscriber, total);
_approveFeeProxy(token, p.amount + p.feeAmount);
_proxyTransfer(p, paymentReference);
_payRelayer(token, p.relayerFee);
_markPaid(scheduleKey, index, p.strictOrder);
}

function setRelayer(address oldRelayer, address newRelayer) external onlyOwner {
Expand Down
141 changes: 130 additions & 11 deletions packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,20 +684,20 @@ describe('ERC20RecurringPaymentProxy', () => {

const permit = createSchedulePermit();
const signature = await createSignature(permit, subscriber);
const digest = await erc20RecurringPaymentProxy.hashSchedule(permit);
const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit);

await expect(
erc20RecurringPaymentProxy
.connect(relayer)
.triggerRecurringPayment(permit, signature, 1, paymentReference),
).to.be.reverted;
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0);
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0);

await testERC20.transfer(subscriberAddress, 500);
await erc20RecurringPaymentProxy
.connect(relayer)
.triggerRecurringPayment(permit, signature, 1, paymentReference);
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.not.equal(0);
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.not.equal(0);
});

it('cannot settle an unfunded subscriber from a residual proxy balance', async () => {
Expand All @@ -709,14 +709,14 @@ describe('ERC20RecurringPaymentProxy', () => {

const permit = createSchedulePermit({ token: silentFail.address });
const signature = await createSignature(permit, subscriber);
const digest = await erc20RecurringPaymentProxy.hashSchedule(permit);
const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit);

await expect(
erc20RecurringPaymentProxy
.connect(relayer)
.triggerRecurringPayment(permit, signature, 1, paymentReference),
).to.be.revertedWith('ERC20RecurringPaymentProxy__TransferFailed');
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0);
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0);
expect(await silentFail.balanceOf(erc20RecurringPaymentProxy.address)).to.equal(500);
expect(await silentFail.balanceOf(recipientAddress)).to.equal(0);
});
Expand All @@ -731,14 +731,14 @@ describe('ERC20RecurringPaymentProxy', () => {

const permit = createSchedulePermit({ token: feeOnTransfer.address });
const signature = await createSignature(permit, subscriber);
const digest = await erc20RecurringPaymentProxy.hashSchedule(permit);
const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit);

await expect(
erc20RecurringPaymentProxy
.connect(relayer)
.triggerRecurringPayment(permit, signature, 1, paymentReference),
).to.be.revertedWith('ERC20RecurringPaymentProxy__ShortPull');
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0);
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0);
});

it('reverts when the token returns false without reverting', async () => {
Expand All @@ -751,14 +751,14 @@ describe('ERC20RecurringPaymentProxy', () => {

const permit = createSchedulePermit({ token: silentFail.address });
const signature = await createSignature(permit, subscriber);
const digest = await erc20RecurringPaymentProxy.hashSchedule(permit);
const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit);

await expect(
erc20RecurringPaymentProxy
.connect(relayer)
.triggerRecurringPayment(permit, signature, 1, paymentReference),
).to.be.revertedWith('ERC20RecurringPaymentProxy__TransferFailed');
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0);
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0);
});

it('does not mark the cycle paid when the relayer-fee transfer fails', async () => {
Expand All @@ -771,18 +771,137 @@ describe('ERC20RecurringPaymentProxy', () => {

const permit = createSchedulePermit({ token: failTransfer.address });
const signature = await createSignature(permit, subscriber);
const digest = await erc20RecurringPaymentProxy.hashSchedule(permit);
const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit);

await expect(
erc20RecurringPaymentProxy
.connect(relayer)
.triggerRecurringPayment(permit, signature, 1, paymentReference),
).to.be.revertedWith('ERC20RecurringPaymentProxy__TransferFailed');
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0);
expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0);
expect(await failTransfer.balanceOf(recipientAddress)).to.equal(0);
});
});

describe('Schedule key replay', () => {
const paymentReference = '0x1234567890abcdef';

it('re-signing with a new nonce or deadline does not reset paid indices', async () => {
await testERC20.transfer(subscriberAddress, 500);
await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500);

const permit = createSchedulePermit();
const signature = await createSignature(permit, subscriber);
await erc20RecurringPaymentProxy
.connect(relayer)
.triggerRecurringPayment(permit, signature, 1, paymentReference);

const resigned = { ...permit, nonce: 1, deadline: permit.deadline + 86400 };
const resignedSignature = await createSignature(resigned, subscriber);
const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit);

expect(await erc20RecurringPaymentProxy.scheduleKeyFromPermit(resigned)).to.equal(
scheduleKey,
);
await expect(
erc20RecurringPaymentProxy
.connect(relayer)
.triggerRecurringPayment(resigned, resignedSignature, 1, paymentReference),
).to.be.revertedWith('ERC20RecurringPaymentProxy__AlreadyPaid');
});

it('rejects index 0', async () => {
await testERC20.transfer(subscriberAddress, 500);
await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500);

const permit = createSchedulePermit();
const signature = await createSignature(permit, subscriber);

await expect(
erc20RecurringPaymentProxy
.connect(relayer)
.triggerRecurringPayment(permit, signature, 0, paymentReference),
).to.be.revertedWith('ERC20RecurringPaymentProxy__IndexOutOfBounds');
});

it('keeps the batch schedule key stable across nonce and deadline re-sign', async () => {
const permit = {
subscriber: subscriberAddress,
token: testERC20.address,
relayerFee: 1,
totalPayments: 1,
nonce: 0,
deadline: Math.floor(Date.now() / 1000) + 86400,
strictOrder: false,
scheduleId: '0x0101010101010101010101010101010101010101010101010101010101010101',
dueTimes: [Math.floor(Date.now() / 1000)],
initialLegs: [],
recurringLegs: [],
};
const resigned = { ...permit, nonce: 9, deadline: permit.deadline + 1 };
expect(await erc20RecurringPaymentProxy.scheduleKeyFromBatch(permit)).to.equal(
await erc20RecurringPaymentProxy.scheduleKeyFromBatch(resigned),
);
});

it('changes the batch schedule key when signed terms change', async () => {
const permit = {
subscriber: subscriberAddress,
token: testERC20.address,
relayerFee: 1,
totalPayments: 1,
nonce: 0,
deadline: Math.floor(Date.now() / 1000) + 86400,
strictOrder: false,
scheduleId: '0x0101010101010101010101010101010101010101010101010101010101010101',
dueTimes: [Math.floor(Date.now() / 1000)],
initialLegs: [],
recurringLegs: [],
};
const key = await erc20RecurringPaymentProxy.scheduleKeyFromBatch(permit);
expect(
await erc20RecurringPaymentProxy.scheduleKeyFromBatch({
...permit,
token: ethers.constants.AddressZero,
}),
).to.not.equal(key);
expect(
await erc20RecurringPaymentProxy.scheduleKeyFromBatch({ ...permit, strictOrder: true }),
).to.not.equal(key);
expect(
await erc20RecurringPaymentProxy.scheduleKeyFromBatch({
...permit,
recurringLegs: [
{
recipient: recipientAddress,
amount: 1,
paymentReference: ethers.utils.hexZeroPad('0x01', 32),
},
],
}),
).to.not.equal(key);
});

it('rejects a zero batch scheduleId', async () => {
const permit = {
subscriber: subscriberAddress,
token: testERC20.address,
relayerFee: 0,
totalPayments: 1,
nonce: 0,
deadline: Math.floor(Date.now() / 1000) + 86400,
strictOrder: false,
scheduleId: ethers.constants.HashZero,
dueTimes: [Math.floor(Date.now() / 1000)],
initialLegs: [],
recurringLegs: [],
};
await expect(erc20RecurringPaymentProxy.scheduleKeyFromBatch(permit)).to.be.revertedWith(
'ERC20RecurringPaymentProxy__ZeroScheduleId',
);
});
});

describe('EIP-1271 signatures', () => {
const paymentReference = '0x1234567890abcdef';

Expand Down