Skip to content
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

Optimize gas for _getFraction in AmountDeriver #384

Merged
merged 9 commits into from
Jun 8, 2022
Merged

Optimize gas for _getFraction in AmountDeriver #384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4d5c1f0
Optimize gas for _getFraction in AmountDeriver
Chomtana Jun 7, 2022
0410f66
Add more credit inplace of @hrkrshnn + @axic
Chomtana Jun 7, 2022
dea98a6
Change file name to follow the original pattern
Chomtana Jun 7, 2022
341d9e4
Revert to just optimized mulmod check
Chomtana Jun 7, 2022
6d99451
Remove gt as gt 0 = truth
Chomtana Jun 7, 2022
4cd8c9e
Add Disambiguation & Other Notes
Chomtana Jun 7, 2022
4ae3e3e
Reference AmountDerivationErrors.sol
Chomtana Jun 7, 2022
a2c1d58
Move constants to ConsiderationConstants
Chomtana Jun 7, 2022
5a8575d
Remove credit
Chomtana Jun 8, 2022
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
13 changes: 6 additions & 7 deletions contracts/lib/AmountDeriver.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
AmountDerivationErrors
} from "../interfaces/AmountDerivationErrors.sol";

import "./ConsiderationConstants.sol";

/**
* @title AmountDeriver
* @author 0age
Expand Down Expand Up @@ -98,16 +100,13 @@ contract AmountDeriver is AmountDerivationErrors {

// Ensure fraction can be applied to the value with no remainder. Note
// that the denominator cannot be zero.
bool exact;
assembly {
// Ensure new value contains no remainder via mulmod operator.
// Credit to @hrkrshnn + @axic for proposing this optimal solution.
exact := iszero(mulmod(value, numerator, denominator))
}

// Ensure that division gave a final result with no remainder.
if (!exact) {
revert InexactFraction();
if mulmod(value, numerator, denominator) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this does anything? The double iszero(iszero(...)) should be optimised out.

Copy link
Contributor Author

@Chomtana Chomtana Jun 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just mulmod(value, numerator, denominator) should be worked as @0age said since we need to test whether mulmod(value, numerator, denominator) is not zero and it is unsigned which means mulmod(value, numerator, denominator) is truthy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean if the change does anything gas wise. The double iszero should be removed by the optimiser.

Copy link
Contributor Author

@Chomtana Chomtana Jun 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does by removing unnecessary exact variable.

exact := iszero(mulmod(value, numerator, denominator))
if iszero(exact) {
...
}

Don't sure whether optimiser can remove double iszero that is stored into intermediate variable first.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many optimiser stages, including one which operates on opcode level, where known stack movement are supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just tested gas on remix using 2 separate contracts with same function name getFraction for both case and adding
a global variable assignment to the end

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.13;

error InexactFraction();

// error InexactFraction() @ AmountDerivationErrors.sol
uint256 constant InexactFraction_error_signature = (
    0xc63cf08900000000000000000000000000000000000000000000000000000000
);
uint256 constant InexactFraction_error_len = 0x20;

contract TestMulMod {
    uint256 public x;

    /**
     * @dev Internal pure function to return a fraction of a given value and to
     *      ensure the resultant value does not have any fractional component.
     *      Note that this function assumes that zero will never be supplied as
     *      the denominator parameter; invalid / undefined behavior will result
     *      should a denominator of zero be provided.
     *
     * @param numerator   A value indicating the portion of the order that
     *                    should be filled.
     * @param denominator A value indicating the total size of the order. Note
     *                    that this value cannot be equal to zero.
     * @param value       The value for which to compute the fraction.
     *
     * @return newValue The value after applying the fraction.
     */
    function getFraction(
        uint256 numerator,
        uint256 denominator,
        uint256 value
    ) public returns (uint256 newValue) {
        // Return value early in cases where the fraction resolves to 1.
        if (numerator == denominator) {
            return value;
        }

        // Ensure fraction can be applied to the value with no remainder. Note
        // that the denominator cannot be zero.
        assembly {
            // Ensure new value contains no remainder via mulmod operator.
            // Credit to @hrkrshnn + @axic for proposing this optimal solution.
            // Further optimization by @Chomtana and @0age
            if mulmod(value, numerator, denominator) {
                mstore(0, InexactFraction_error_signature)
                revert(0, InexactFraction_error_len)
            }
        }

        // Multiply the numerator by the value and ensure no overflow occurs.
        uint256 valueTimesNumerator = value * numerator;

        // Divide and check for remainder. Note that denominator cannot be zero.
        assembly {
            // Perform division without zero check.
            newValue := div(valueTimesNumerator, denominator)
        }

        x = newValue;
    }

    /**
     * @dev Internal pure function to return a fraction of a given value and to
     *      ensure the resultant value does not have any fractional component.
     *      Note that this function assumes that zero will never be supplied as
     *      the denominator parameter; invalid / undefined behavior will result
     *      should a denominator of zero be provided.
     *
     * @param numerator   A value indicating the portion of the order that
     *                    should be filled.
     * @param denominator A value indicating the total size of the order. Note
     *                    that this value cannot be equal to zero.
     * @param value       The value for which to compute the fraction.
     *
     * @return newValue The value after applying the fraction.
     */
    // function getFraction(
    //     uint256 numerator,
    //     uint256 denominator,
    //     uint256 value
    // ) public returns (uint256 newValue) {
    //     // Return value early in cases where the fraction resolves to 1.
    //     if (numerator == denominator) {
    //         return value;
    //     }

    //     // Ensure fraction can be applied to the value with no remainder. Note
    //     // that the denominator cannot be zero.
    //     bool exact;
    //     assembly {
    //         // Ensure new value contains no remainder via mulmod operator.
    //         // Credit to @hrkrshnn + @axic for proposing this optimal solution.
    //         exact := iszero(mulmod(value, numerator, denominator))
    //     }

    //     // Ensure that division gave a final result with no remainder.
    //     if (!exact) {
    //         revert InexactFraction();
    //     }

    //     // Multiply the numerator by the value and ensure no overflow occurs.
    //     uint256 valueTimesNumerator = value * numerator;

    //     // Divide and check for remainder. Note that denominator cannot be zero.
    //     assembly {
    //         // Perform division without zero check.
    //         newValue := div(valueTimesNumerator, denominator)
    //     }

    //     x = newValue;
    // }
}

Normal case (2, 5, 10)
Before: 24189 gas
After: 24176 gas

Revert case (2, 5, 8)
Before: 21845 gas
After: 21804 gas

mstore(0, InexactFraction_error_signature)
revert(0, InexactFraction_error_len)
}
}

// Multiply the numerator by the value and ensure no overflow occurs.
Expand Down
7 changes: 7 additions & 0 deletions contracts/lib/ConsiderationConstants.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,3 +344,10 @@ uint256 constant Conduit_transferItem_from_ptr = 0x40;
uint256 constant Conduit_transferItem_to_ptr = 0x60;
uint256 constant Conduit_transferItem_identifier_ptr = 0x80;
uint256 constant Conduit_transferItem_amount_ptr = 0xa0;

// Declare constant for errors related to amount derivation.
// error InexactFraction() @ AmountDerivationErrors.sol
uint256 constant InexactFraction_error_signature = (
0xc63cf08900000000000000000000000000000000000000000000000000000000
);
uint256 constant InexactFraction_error_len = 0x20;