Fix rounding management of partial fills (#1352)

* Fix rounding management of partial fills * Renamed tests
airswap · Feb 21, 2025 · 4bde2d5 · 4bde2d5
1 parent d55731d
commit 4bde2d5
Show file tree

Hide file tree

Showing 2 changed files with 115 additions and 6 deletions.
diff --git 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
@@ -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)