fix: Handle ArithmeticException in safeFractionMultiply() (#10200)

Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
hashgraph · Dec 1, 2023 · ba16e00 · ba16e00
1 parent a7dea9f
commit ba16e00
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 34 deletions.
diff --git a/.../com/hedera/node/app/service/token/impl/handlers/transfer/customfees/AdjustmentUtils.java b/.../com/hedera/node/app/service/token/impl/handlers/transfer/customfees/AdjustmentUtils.java
@@ -42,8 +42,10 @@ private AdjustmentUtils() {
      * @param denominator The denominator of the fraction
      * @param amount The given units transferred
      * @return The amount to be charged
+     * @throws ArithmeticException If denominator is 0
      */
-    public static long safeFractionMultiply(final long numerator, final long denominator, final long amount) {
+    public static long safeFractionMultiply(final long numerator, final long denominator, final long amount)
+            throws ArithmeticException {
         if (amount != 0 && numerator > Long.MAX_VALUE / amount) {
             return BigInteger.valueOf(amount)
                     .multiply(BigInteger.valueOf(numerator))
@@ -157,12 +159,10 @@ public static Map<AccountID, Long> getFungibleTokenCredits(final Map<AccountID,
      * Given a list of changes for a specific token, filters all fungible credits including hbar or
      * fungible token balances for a given beneficiary and returns them
      * @param result The {@link AssessmentResult} object
-     * @param tokenId The token id
      * @param sender The sender of the nft
      * @return The list of credits
      */
-    public static List<ExchangedValue> getFungibleCredits(
-            final AssessmentResult result, final TokenID tokenId, final AccountID sender) {
+    public static List<ExchangedValue> getFungibleCredits(final AssessmentResult result, final AccountID sender) {
         final var tokenChanges = result.getImmutableInputTokenAdjustments();
         // get all the fungible changes that are credited to the sender of nft in the same transaction.
         // this includes hbar and fungible token balances

diff --git a/...node/app/service/token/impl/handlers/transfer/customfees/CustomFractionalFeeAssessor.java b/...node/app/service/token/impl/handlers/transfer/customfees/CustomFractionalFeeAssessor.java
@@ -231,9 +231,13 @@ private long reclaim(final long amount, @NonNull final Map<AccountID, Long> cred
         for (final var entry : credits.entrySet()) {
             final var account = entry.getKey();
             final var creditAmount = entry.getValue();
-            final var toReclaimHere = safeFractionMultiply(creditAmount, availableToReclaim, amount);
-            credits.put(account, creditAmount - toReclaimHere);
-            amountReclaimed += toReclaimHere;
+            try {
+                final var toReclaimHere = safeFractionMultiply(creditAmount, availableToReclaim, amount);
+                credits.put(account, creditAmount - toReclaimHere);
+                amountReclaimed += toReclaimHere;
+            } catch (final ArithmeticException e) {
+                throw new HandleException(CUSTOM_FEE_OUTSIDE_NUMERIC_RANGE);
+            }
         }
 
         if (amountReclaimed < amount) {

diff --git a/...ra/node/app/service/token/impl/handlers/transfer/customfees/CustomRoyaltyFeeAssessor.java b/...ra/node/app/service/token/impl/handlers/transfer/customfees/CustomRoyaltyFeeAssessor.java
@@ -28,6 +28,7 @@
 import com.hedera.hapi.node.base.AccountID;
 import com.hedera.hapi.node.transaction.AssessedCustomFee;
 import com.hedera.hapi.node.transaction.CustomFee;
+import com.hedera.node.app.spi.workflows.HandleException;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.util.List;
 import javax.inject.Inject;
@@ -73,7 +74,7 @@ public void assessRoyaltyFees(
         }
 
         // get all hbar and fungible token changes from given input to the current level
-        final var exchangedValue = getFungibleCredits(result, tokenId, sender);
+        final var exchangedValue = getFungibleCredits(result, sender);
         for (final var fee : feeMeta.customFees()) {
             final var collector = fee.feeCollectorAccountId();
             if (!fee.fee().kind().equals(CustomFee.FeeOneOfType.ROYALTY_FEE)) {
@@ -133,34 +134,38 @@ private void chargeRoyalty(
             final var royaltySpec = fee.royaltyFeeOrThrow();
             final var feeCollector = fee.feeCollectorAccountIdOrThrow();
 
-            final var royalty = safeFractionMultiply(
-                    royaltySpec.exchangeValueFraction().numerator(),
-                    royaltySpec.exchangeValueFraction().denominator(),
-                    amount);
-            validateTrue(royalty <= amount, INSUFFICIENT_SENDER_ACCOUNT_BALANCE_FOR_CUSTOM_FEE);
+            try {
+                final var royalty = safeFractionMultiply(
+                        royaltySpec.exchangeValueFraction().numerator(),
+                        royaltySpec.exchangeValueFraction().denominator(),
+                        amount);
+                validateTrue(royalty <= amount, INSUFFICIENT_SENDER_ACCOUNT_BALANCE_FOR_CUSTOM_FEE);
 
-            /* The id of the charging token is only used here to avoid recursively charging
-            on fees charged in the units of their denominating token; but this is a credit,
-            hence the id is irrelevant, and we can use null. */
-            if (denom == null) {
-                // exchange is for hbar
-                adjustHbarFees(result, account, fee);
-            } else {
-                // exchange is for token
-                adjustHtsFees(result, account, feeCollector, feeMeta, royalty, denom);
-            }
+                /* The id of the charging token is only used here to avoid recursively charging
+                on fees charged in the units of their denominating token; but this is a credit,
+                hence the id is irrelevant, and we can use null. */
+                if (denom == null) {
+                    // exchange is for hbar
+                    adjustHbarFees(result, account, fee);
+                } else {
+                    // exchange is for token
+                    adjustHtsFees(result, account, feeCollector, feeMeta, royalty, denom);
+                }
 
-            final var assessedCustomFeeBuilder = AssessedCustomFee.newBuilder()
-                    .amount(royalty)
-                    .feeCollectorAccountId(feeCollector)
-                    .effectivePayerAccountId(account);
-            if (denom == null) {
-                // exchange is for hbar
-                result.addAssessedCustomFee(assessedCustomFeeBuilder.build());
-            } else {
-                // exchange is for token
-                result.addAssessedCustomFee(
-                        assessedCustomFeeBuilder.tokenId(denom).build());
+                final var assessedCustomFeeBuilder = AssessedCustomFee.newBuilder()
+                        .amount(royalty)
+                        .feeCollectorAccountId(feeCollector)
+                        .effectivePayerAccountId(account);
+                if (denom == null) {
+                    // exchange is for hbar
+                    result.addAssessedCustomFee(assessedCustomFeeBuilder.build());
+                } else {
+                    // exchange is for token
+                    result.addAssessedCustomFee(
+                            assessedCustomFeeBuilder.tokenId(denom).build());
+                }
+            } catch (final ArithmeticException e) {
+                throw new HandleException(INSUFFICIENT_SENDER_ACCOUNT_BALANCE_FOR_CUSTOM_FEE);
             }
         }
     }