Skip to content

_🛠️ Refactor suggestion_ #1737

Open
@thedavidmeister

Description

@thedavidmeister
          _🛠️ Refactor suggestion_

⚠️ Potential issue

Validate orderIORatio and orderOutputMax are non-negative before use

The values popped from the interpreter stack can be negative (nothing in eval4 guarantees positivity).
If either comes back < 0 we will:

  • accept a negative ratio that later triggers an under-/overflow in multiply / divide;
  • record a negative outputMax that propagates through vault-IO calculations until increaseVaultBalance reverts with NegativeVaultBalanceChange, wasting gas for every affected taker/clear call.

Add an early guard after reading the stack:

             assembly ("memory-safe") {
                 orderIORatio := mload(add(calculateOrderStack, 0x20))
                 orderOutputMax := mload(add(calculateOrderStack, 0x40))
             }
-            {
+            // Prevent negative or zero-ratio surprises as early as possible.
+            if (orderIORatio.lte(Float.wrap(0))) {
+                revert ZeroOrNegativeIORatio(orderIORatio);
+            }
+            if (orderOutputMax.lt(Float.wrap(0))) {
+                revert NegativeOutputMax(orderOutputMax);
+            }
+            {
                 // The order owner can't send more than the smaller of their vault
                 // balance or their per-order limit.

(Using your existing Float helpers keeps the check cheap.)
Adding these invariants avoids downstream reverts and protects against malicious or buggy order bytecode.

Committable suggestion skipped: line range outside the PR's diff.

Originally posted by @coderabbitai[bot] in #860 (comment)

Metadata

Metadata

Labels

contractRelated to solidity contracts

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions