From 941831b57fce3d2fbbbacfd6d97aea4dca8f30a9 Mon Sep 17 00:00:00 2001 From: MrToph Date: Tue, 5 Mar 2024 14:21:22 +0000 Subject: [PATCH 1/3] improve updateStatus scale-down --- src/core/lib/OrderValidator.sol | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/core/lib/OrderValidator.sol b/src/core/lib/OrderValidator.sol index 4582318..c9fee7c 100644 --- a/src/core/lib/OrderValidator.sol +++ b/src/core/lib/OrderValidator.sol @@ -517,15 +517,13 @@ contract OrderValidator is Executor, ZoneInteraction { out := _a } - // Determine the amount to scale down the fill fractions. - let scaleDown := - gcd(numerator, gcd(filledNumerator, denominator)) + // Determine the amount to scale down the new filled fraction. + let scaleDown := gcd(filledNumerator, denominator) // Ensure that the divisor is at least one. let safeScaleDown := add(scaleDown, iszero(scaleDown)) - // Scale all fractional values down by gcd. - numerator := div(numerator, safeScaleDown) + // Scale new filled fractional values down by gcd. filledNumerator := div(filledNumerator, safeScaleDown) denominator := div(denominator, safeScaleDown) From ab953c5400f9441060a656ae8d32c0609a9f3a49 Mon Sep 17 00:00:00 2001 From: MrToph Date: Tue, 5 Mar 2024 15:01:19 +0000 Subject: [PATCH 2/3] fix reference implementation for updateStatus scale-down --- reference/lib/ReferenceOrderValidator.sol | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/reference/lib/ReferenceOrderValidator.sol b/reference/lib/ReferenceOrderValidator.sol index bdb67d7..34f16b2 100644 --- a/reference/lib/ReferenceOrderValidator.sol +++ b/reference/lib/ReferenceOrderValidator.sol @@ -323,13 +323,9 @@ contract ReferenceOrderValidator is || denominator > type(uint120).max ) { // Derive greatest common divisor using euclidean algorithm. - uint256 scaleDown = _greatestCommonDivisor( - numerator, - _greatestCommonDivisor(filledNumerator, denominator) - ); + uint256 scaleDown = _greatestCommonDivisor(filledNumerator, denominator); - // Scale all fractional values down by gcd. - numerator = numerator / scaleDown; + // Scale new filled fractional values down by gcd. filledNumerator = filledNumerator / scaleDown; denominator = denominator / scaleDown; From a5643822737b77a8d8d2511c6b3ff8fb51297bef Mon Sep 17 00:00:00 2001 From: MrToph Date: Tue, 5 Mar 2024 15:37:43 +0000 Subject: [PATCH 3/3] fix tests by fully reducing expected fill fractions --- test/foundry/FulfillAdvancedOrder.t.sol | 6 +++--- test/foundry/FulfillAvailableAdvancedOrder.t.sol | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/test/foundry/FulfillAdvancedOrder.t.sol b/test/foundry/FulfillAdvancedOrder.t.sol index 06a56f5..3fc0175 100644 --- a/test/foundry/FulfillAdvancedOrder.t.sol +++ b/test/foundry/FulfillAdvancedOrder.t.sol @@ -487,7 +487,7 @@ contract FulfillAdvancedOrder is BaseOrderTest { address(0) ); - // Assert six-tenths of the order has been fulfilled. + // Assert three-fifths of the order has been fulfilled. { ( bool isValidated, @@ -497,9 +497,9 @@ contract FulfillAdvancedOrder is BaseOrderTest { ) = context.consideration.getOrderStatus(orderHash); assertTrue(isValidated); assertFalse(isCancelled); - assertEq(totalFilled, 6); + assertEq(totalFilled, 3); - assertEq(totalSize, 10); + assertEq(totalSize, 5); assertEq(60, test1155_1.balanceOf(address(this), 1)); } } diff --git a/test/foundry/FulfillAvailableAdvancedOrder.t.sol b/test/foundry/FulfillAvailableAdvancedOrder.t.sol index 3d29e80..e581624 100644 --- a/test/foundry/FulfillAvailableAdvancedOrder.t.sol +++ b/test/foundry/FulfillAvailableAdvancedOrder.t.sol @@ -996,7 +996,7 @@ contract FulfillAvailableAdvancedOrder is BaseOrderTest { 100 ); - // Assert six-tenths of the offer has been fulfilled. + // Assert three-fifths of the offer has been fulfilled. { ( bool isValidated, @@ -1006,9 +1006,9 @@ contract FulfillAvailableAdvancedOrder is BaseOrderTest { ) = context.consideration.getOrderStatus(orderHash); assertTrue(isValidated); assertFalse(isCancelled); - assertEq(totalFilled, 6); + assertEq(totalFilled, 3); - assertEq(totalSize, 10); + assertEq(totalSize, 5); assertEq(60, test1155_1.balanceOf(address(this), 1)); } } @@ -1093,7 +1093,7 @@ contract FulfillAvailableAdvancedOrder is BaseOrderTest { 100 ); - // Assert six-tenths of the offer has been fulfilled. + // Assert three-fifths of the offer has been fulfilled. { ( bool isValidated, @@ -1103,9 +1103,9 @@ contract FulfillAvailableAdvancedOrder is BaseOrderTest { ) = context.consideration.getOrderStatus(orderHash); assertTrue(isValidated); assertFalse(isCancelled); - assertEq(totalFilled, 6); + assertEq(totalFilled, 3); - assertEq(totalSize, 10); + assertEq(totalSize, 5); assertEq(60, test1155_1.balanceOf(address(this), 1)); } }