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

Inefficient Use of public Instead of external

Author Revealed upon completion

Inefficient Use of public Instead of external

Description

  • The buyOrder(), createSellOrder(), amendSellOrder(), cancelSellOrder(), getOrder(), getOrderDetailsString() function is marked public but is never called internally within the OrderBook contract.
    When a public function is called externally, Solidity copies arguments from calldata to memory, which increases gas cost — especially with dynamic types.
    In contrast, external functions access arguments directly from calldata, making them more gas-efficient and reducing bytecode size.


Risk

Likelihood:

  • Developers often default to public, even for functions never called internally.

  • Solidity allows this without error or warning.

  • This inefficiency is common in many contracts, especially when not optimized for gas.

Impact:

  • Increased gas cost on every external call to buyOrder().

  • Slightly larger bytecode due to the internal callable "interface" that public functions generate.

  • Reduces overall protocol efficiency in high-frequency trading systems like an on-chain order book.


Proof of Concept

For prove this we mimic an function

mock.sol
​
contract GasTest {
uint256 public value;
​
function setValuePublic(uint256 _value) public {
value = _value;
}
​
function setValueExternal(uint256 _value) external {
value = _value;
}
}
​
​
Test case
​
function test_gasEfficiencyPublicVsExternal() public {
GasTest gasTest = new GasTest();
​
// Measure gas for public function
uint256 gasStartPublic = gasleft();
gasTest.setValuePublic(123);
uint256 gasUsedPublic = gasStartPublic - gasleft();
​
// Measure gas for external function
uint256 gasStartExternal = gasleft();
gasTest.setValueExternal(456);
uint256 gasUsedExternal = gasStartExternal - gasleft();
​
console2.log("gasUsedPublic", gasUsedPublic);
console2.log("gasUsedExternal", gasUsedExternal);
​
emit log_named_uint("Gas used (public)", gasUsedPublic);
emit log_named_uint("Gas used (external)", gasUsedExternal);
​
// gasUsedExternal=gasUsedPublic+1000000;
​
// For demonstration: usually, external is slightly cheaper when called externally
assertTrue(gasUsedExternal <= gasUsedPublic, "External should be as cheap or cheaper than public");
}
​
​
Result
​
Gas used (public): 23107
Gas used (external): 1151

Recommended Mitigation

Update the visibility of the buyOrder() function from public to external:

- function buyOrder(uint256 _orderId) public {
+ function buyOrder(uint256 _orderId) external {

Also apply the same change to other externally-only functions like:

createSellOrder()
​
amendSellOrder()
​
cancelSellOrder()
​
getOrder()
​
getOrderDetailsString()
Updates

Lead Judging Commences

yeahchibyke Lead Judge 1 day ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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