OrderBook
First Flight #43
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: high
Invalid

Redundant status variable assignments in getOrderDetailsString

ehl

Root + Impact

Description

  • The getOrderDetailsString function is designed to return a formatted string with order details including a status field that reflects the current state of the order.

  • The function performs redundant status variable assignments by first setting the status using a ternary operator, then immediately overwriting it with an if-else chain that performs identical checks, resulting in unnecessary gas consumption.

// Root cause in the codebase with @> marks to highlight the relevant section
@> string memory status = order.isActive
@> ? (block.timestamp < order.deadlineTimestamp ? "Active" : "Expired (Active but past deadline)")
@> : "Inactive (Filled/Cancelled)";
@> if (order.isActive && block.timestamp >= order.deadlineTimestamp) {
@> status = "Expired (Awaiting Cancellation)";
@> } else if (!order.isActive) {
@> status = "Inactive (Filled/Cancelled)";
@> } else {
@> status = "Active";
@> }

Risk

Likelihood:

  • This inefficiency occurs every time getOrderDetailsString is called, affecting all users querying order details

  • The redundant operations execute consistently across all function calls without any conditional bypass

Impact:

  • Unnecessary gas consumption for all users calling the function

  • Reduced code maintainability due to duplicate logic that can lead to inconsistencies

  • Performance degradation from redundant computations and memory assignments

Proof of Concept

This analysis shows how the initial ternary assignment is completely overwritten by the subsequent conditional logic:

// Initial assignment (completely redundant)
string memory status = order.isActive
? (block.timestamp < order.deadlineTimestamp ? "Active" : "Expired (Active but past deadline)")
: "Inactive (Filled/Cancelled)";
​
// Immediate overwrite with if-else chain
if (order.isActive && block.timestamp >= order.deadlineTimestamp) {
status = "Expired (Awaiting Cancellation)";
} else if (!order.isActive) {
status = "Inactive (Filled/Cancelled)";
} else {
status = "Active";
}

Recommended Mitigation

Remove the redundant initial assignment and use only the if-else logic to eliminate unnecessary gas consumption:

- string memory status = order.isActive
- ? (block.timestamp < order.deadlineTimestamp ? "Active" : "Expired (Active but past deadline)")
- : "Inactive (Filled/Cancelled)";
+ string memory status;
if (order.isActive && block.timestamp >= order.deadlineTimestamp) {
status = "Expired (Awaiting Cancellation)";
} else if (!order.isActive) {
status = "Inactive (Filled/Cancelled)";
} else {
status = "Active";
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.