Merge pull request #361 from rsksmart/GBI-2875/2-make-transferred-amount-check-strict

Gbi 2875/2 make transferred amount check strict

Author: Hakob23

contracts/libraries/Quotes.sol

@@ -47,6 +47,8 @@ library Quotes {
         bytes lpBtcAddress;
     }
 
+    uint256 public constant SAT_TO_WEI_CONVERSION = 10**10;
+
     error AmountTooLow(uint256 value, uint256 target);
 
     function checkAgreedAmount(
@@ -56,12 +58,15 @@ library Quotes {
         uint agreedAmount = 0;
         agreedAmount = quote.value + quote.callFee + quote.productFeeAmount + quote.gasFee;
 
+        // Adjust for rounding when converting from wei to sats and back
+        // This protects users from precision issues when client apps don't round properly
+        if (agreedAmount > SAT_TO_WEI_CONVERSION && (agreedAmount % SAT_TO_WEI_CONVERSION) != 0) {
+            agreedAmount -= (agreedAmount % SAT_TO_WEI_CONVERSION);
+        }
 
-        uint delta = agreedAmount / 10000;
-        // transferred amount should not be lower than (agreed amount - delta),
-        // where delta is intended to tackle rounding problems
-        if (agreedAmount - delta > transferredAmount) {
-            revert AmountTooLow(transferredAmount, agreedAmount - delta);
+        // transferred amount should not be lower than agreed amount
+        if (agreedAmount > transferredAmount) {
+            revert AmountTooLow(transferredAmount, agreedAmount);
         }
     }
 

test/pegin/register-pegin.test.ts

@@ -17,6 +17,7 @@ import {
   getRewardForQuote,
   getTestPeginQuote,
   totalValue,
+  expectedRoundedAmount,
 } from "../utils/quotes";
 import { expect } from "chai";
 import { ethers } from "hardhat";
@@ -234,9 +235,10 @@ describe("PegInContract registerPegIn function should", () => {
     );
   });
 
-  it("refund user when call was not done and user under paid the quote", async () => {
-    const { contract, fullLp, signers, bridgeMock, collateralManagement } =
-      await loadFixture(deployPegInContractFixture);
+  it("revert when user under paid the quote", async () => {
+    const { contract, fullLp, signers, bridgeMock } = await loadFixture(
+      deployPegInContractFixture
+    );
     const lbcAddress = await contract.getAddress();
     const user = signers[0];
 
@@ -259,6 +261,12 @@ describe("PegInContract registerPegIn function should", () => {
         nConfirmationSeconds: 600,
       });
 
+    const QuotesLib = new ethers.Contract(
+      ethers.ZeroAddress,
+      Quotes__factory.abi,
+      ethers.provider
+    );
+
     const height = PEGIN_CONSTANTS.HEIGHT_MOCK;
     const peginAmount = totalValue(quote) - ethers.parseEther("0.0001");
     await bridgeMock.setPegin(quoteHash, { value: peginAmount });
@@ -268,32 +276,22 @@ describe("PegInContract registerPegIn function should", () => {
       nConfirmationHeader
     );
 
-    const tx = contract
-      .connect(fullLp)
-      .registerPegIn(
-        quote,
-        signature,
-        PEGIN_CONSTANTS.RAW_TRANSACTION_MOCK,
-        PEGIN_CONSTANTS.PMT_MOCK,
-        height
-      );
-    await expect(tx)
-      .to.emit(contract, "PegInRegistered")
-      .withArgs(quoteHash, peginAmount);
-    await expect(tx).not.to.emit(collateralManagement, "Penalized");
-    await expect(tx)
-      .to.emit(contract, "Refund")
-      .withArgs(user.address, quoteHash, peginAmount, true);
-    await expect(tx).not.to.emit(contract, "CallForUser");
-    await expect(tx).not.to.emit(contract, "BridgeCapExceeded");
-    await expect(tx).not.to.emit(contract, "DaoContribution");
+    await expect(
+      contract
+        .connect(fullLp)
+        .registerPegIn(
+          quote,
+          signature,
+          PEGIN_CONSTANTS.RAW_TRANSACTION_MOCK,
+          PEGIN_CONSTANTS.PMT_MOCK,
+          height
+        )
+    )
+      .to.be.revertedWithCustomError(QuotesLib, "AmountTooLow")
+      .withArgs(peginAmount, expectedRoundedAmount(quote));
     await expect(contract.getCurrentContribution()).to.eventually.eq(0n);
     await expect(contract.getQuoteStatus(quoteHash)).to.eventually.eq(
-      PegInStates.PROCESSED_QUOTE
-    );
-    await expect(tx).to.changeEtherBalances(
-      [user, fullLp, contract],
-      [peginAmount, 0n, 0n]
+      PegInStates.UNPROCESSED_QUOTE
     );
     await expect(contract.getBalance(fullLp)).to.eventually.eq(0n);
   });
@@ -821,7 +819,6 @@ describe("PegInContract registerPegIn function should", () => {
       height + Number(quote.depositConfirmations) - 1,
       nConfirmationHeader
     );
-    const minValue = totalValue(quote) - totalValue(quote) / 10_000n;
     await expect(
       contract
         .connect(fullLp)
@@ -834,7 +831,7 @@ describe("PegInContract registerPegIn function should", () => {
         )
     )
       .to.be.revertedWithCustomError(QuotesLib, "AmountTooLow")
-      .withArgs(peginAmount, minValue);
+      .withArgs(peginAmount, expectedRoundedAmount(quote));
     await expect(contract.getCurrentContribution()).to.eventually.eq(0n);
     await expect(contract.getQuoteStatus(quoteHash)).to.eventually.eq(
       PegInStates.UNPROCESSED_QUOTE

test/utils/quotes.ts

@@ -108,6 +108,26 @@ export function totalValue(
   );
 }
 
+/**
+ * Calculate the expected amount after rounding for SAT to WEI conversion
+ * This matches the logic in Quotes.checkAgreedAmount
+ */
+export function expectedRoundedAmount(
+  quote: QuotesV2.PeginQuoteStruct | QuotesV2.PegOutQuoteStruct
+): bigint {
+  const SAT_TO_WEI_CONVERSION = 10n ** 10n;
+  const agreedAmount = totalValue(quote);
+
+  if (
+    agreedAmount > SAT_TO_WEI_CONVERSION &&
+    agreedAmount % SAT_TO_WEI_CONVERSION !== 0n
+  ) {
+    return agreedAmount - (agreedAmount % SAT_TO_WEI_CONVERSION);
+  }
+
+  return agreedAmount;
+}
+
 /**
  * Get mock bitcoin block headers for the given quote
  * @param args.quote The quote to get the block headers for