From 4bde2d5ada1789bea0765ff0f649bb838baa045e Mon Sep 17 00:00:00 2001 From: Smartcontrart <100962328+smartcontrart@users.noreply.github.com> Date: Fri, 21 Feb 2025 04:07:17 -0500 Subject: [PATCH] Fix rounding management of partial fills (#1352) * Fix rounding management of partial fills * Renamed tests --- source/delegate/contracts/Delegate.sol | 2 +- source/delegate/test/Delegate.js | 119 +++++++++++++++++++++++-- 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/source/delegate/contracts/Delegate.sol b/source/delegate/contracts/Delegate.sol index 870cc6146..a1395c676 100644 --- a/source/delegate/contracts/Delegate.sol +++ b/source/delegate/contracts/Delegate.sol @@ -143,7 +143,7 @@ contract Delegate is IDelegate, Ownable { // Ensure the signer amount is valid if ( - rule.signerAmount * _senderAmount != rule.senderAmount * _signerAmount + _signerAmount != (rule.signerAmount * _senderAmount) / rule.senderAmount ) { revert SignerAmountInvalid(); } diff --git a/source/delegate/test/Delegate.js b/source/delegate/test/Delegate.js index 2fa47e215..bd37f9545 100644 --- a/source/delegate/test/Delegate.js +++ b/source/delegate/test/Delegate.js @@ -81,17 +81,17 @@ describe('Delegate Unit', () => { .returns(DEFAULT_SIGNER_AMOUNT) } - async function setUpApprovals() { + async function setUpApprovals(senderAmount, signerAmount) { await senderToken.mock.approve - .withArgs(delegate.address, DEFAULT_SENDER_AMOUNT) + .withArgs(delegate.address, senderAmount) .returns(true) await senderToken.mock.approve - .withArgs(swapERC20.address, DEFAULT_SENDER_AMOUNT) + .withArgs(swapERC20.address, senderAmount) .returns(true) await signerToken.mock.approve - .withArgs(swapERC20.address, DEFAULT_SIGNER_AMOUNT) + .withArgs(swapERC20.address, signerAmount) .returns(true) } @@ -130,7 +130,7 @@ describe('Delegate Unit', () => { await senderToken.mock.transfer.returns(true) await signerToken.mock.transfer.returns(true) - setUpApprovals() + await setUpApprovals(DEFAULT_SENDER_AMOUNT, DEFAULT_SIGNER_AMOUNT) }) describe('Constructor and admin functions', async () => { @@ -412,6 +412,115 @@ describe('Delegate Unit', () => { ).to.emit(delegate, 'DelegatedSwapFor') }) + it('successfully swaps with rounded-down values - Upper bound', async () => { + const senderAmount = '1100' + const signerAmount = '1600' + + await delegate + .connect(sender) + .setRule( + sender.address, + senderToken.address, + senderAmount, + signerToken.address, + signerAmount, + RULE_EXPIRY + ) + + //1100 * 10 / 220 = 5000 + const senderPartialFill = ( + (BigInt(senderAmount) * BigInt(10)) / + BigInt(220) + ).toString() + + //1600 * 10 / 22 = 72.7272727... + // rounds down to 72 + const signerPartialFill = ( + (BigInt(signerAmount) * BigInt(10)) / + BigInt(220) + ).toString() + + expect(signerPartialFill).to.equal('72') + + const order = await createSignedOrderERC20( + { + senderAmount: senderPartialFill, + signerAmount: signerPartialFill, + }, + signer + ) + + await setUpAllowances( + sender.address, + senderPartialFill, + signer.address, + (BigInt(signerPartialFill) + BigInt(PROTOCOL_FEE)).toString() + ) + await setUpBalances(signer.address, sender.address) + + await setUpApprovals( + senderPartialFill, + (BigInt(signerPartialFill) + BigInt(PROTOCOL_FEE)).toString() + ) + + await expect( + delegate.connect(signer).swap(sender.address, ...order) + ).to.emit(delegate, 'DelegatedSwapFor') + }) + + it('successfully swaps with rounded-down values - Lower bound', async () => { + const senderAmount = '1100' + const signerAmount = '1600' + + await delegate + .connect(sender) + .setRule( + sender.address, + senderToken.address, + senderAmount, + signerToken.address, + signerAmount, + RULE_EXPIRY + ) + + //1100 * 10 / 22 = 500 + const senderPartialFill = ( + (BigInt(senderAmount) * BigInt(10)) / + BigInt(22) + ).toString() + + //1600 * 10 / 22 = 727.272727... + // rounds down to 727 + const signerPartialFill = ( + (BigInt(signerAmount) * BigInt(10)) / + BigInt(22) + ).toString() + + expect(signerPartialFill).to.equal('727') + + const order = await createSignedOrderERC20( + { + senderAmount: senderPartialFill, + signerAmount: signerPartialFill, + }, + signer + ) + await setUpAllowances( + sender.address, + senderPartialFill, + signer.address, + (BigInt(signerPartialFill) + BigInt(PROTOCOL_FEE)).toString() + ) + await setUpBalances(signer.address, sender.address) + await setUpApprovals( + senderPartialFill, + (BigInt(signerPartialFill) + BigInt(PROTOCOL_FEE)).toString() + ) + await expect( + delegate.connect(signer).swap(sender.address, ...order) + ).to.emit(delegate, 'DelegatedSwapFor') + }) + it('successfully swaps with a manager', async () => { await delegate.connect(sender).authorize(manager.address)