Submission Details
Severity:
Public vs External Function Visibility
Summary
Functions declared as public but only called externally result in unnecessary gas costs due to Solidity's parameter handling. public functions copy calldata to memory, while external functions maintain parameters in calldata.
Vulnerability Details
Five functions use public visibility but lack internal calls:
function startGame() public payable returns (uint256) {
address player = msg.sender;
require(msg.value >= 1 ether, "not enough ether sent");
initializeDeck(player);
uint256 card1 = drawCard(player);
uint256 card2 = drawCard(player);
addCardForPlayer(player, card1);
addCardForPlayer(player, card2);
return playersHand(player);
}
function hit() public {
require(
playersDeck[msg.sender].playersCards.length > 0,
"Game not started"
);
uint256 handBefore = playersHand(msg.sender);
require(handBefore <= 21, "User is bust");
uint256 newCard = drawCard(msg.sender);
addCardForPlayer(msg.sender, newCard);
uint256 handAfter = playersHand(msg.sender);
if (handAfter > 21) {
emit PlayerLostTheGame("Player is bust", handAfter);
endGame(msg.sender, false);
}
}
function call() public {
require(
playersDeck[msg.sender].playersCards.length > 0,
"Game not started"
);
uint256 playerHand = playersHand(msg.sender);
// Calculate the dealer's threshold for stopping (between 17 and 21)
uint256 standThreshold = (uint256(
keccak256(
abi.encodePacked(block.timestamp, msg.sender, block.prevrandao)
)
) % 5) + 17;
// Dealer draws cards until their hand reaches or exceeds the threshold
while (dealersHand(msg.sender) < standThreshold) {
uint256 newCard = drawCard(msg.sender);
addCardForDealer(msg.sender, newCard);
}
uint256 dealerHand = dealersHand(msg.sender);
// Determine the winner
if (dealerHand > 21) {
emit PlayerWonTheGame(
"Dealer went bust, players winning hand: ",
playerHand
);
endGame(msg.sender, true);
} else if (playerHand > dealerHand) {
emit PlayerWonTheGame(
"Dealer's hand is lower, players winning hand: ",
playerHand
);
endGame(msg.sender, true);
} else {
emit PlayerLostTheGame(
"Dealer's hand is higher, dealers winning hand: ",
dealerHand
);
endGame(msg.sender, false);
}
}
function getPlayerCards(
address player
) public view returns (uint256[] memory) {
return playersDeck[player].playersCards;
}
function getDealerCards(
address player
) public view returns (uint256[] memory) {
return dealersDeck[player].dealersCards;
}
}
Proof of Concept
Test demonstrates gas overhead of public vs external:
// SPDX-License-Identifier: UNLICENSED
import "forge-std/Test.sol";
import "forge-std/console2.sol";
import "../src/TwentyOne.sol";
contract InternalContract {
TwentyOne public game;
constructor(address gameAddress) {
game = TwentyOne(gameAddress);
}
function tryInternalCall() public view returns (uint256[] memory) {
return game.getPlayerCards(address(this));
}
}
contract VisibilityTest is Test {
TwentyOne public game;
InternalContract public internalCaller;
function setUp() public {
game = new TwentyOne();
internalCaller = new InternalContract(address(game));
vm.deal(address(this), 10 ether);
}
function testPublicVsExternal() public {
// Get gas snapshot before public calls
uint256 gasStart = gasleft();
game.startGame{value: 1 ether}();
uint256[] memory playerCards = game.getPlayerCards(address(this));
uint256 gasUsedPublic = gasStart - gasleft();
console2.log("Gas used for public calls:", gasUsedPublic);
// Test internal contract calls to public functions
uint256 gasStartInternal = gasleft();
internalCaller.tryInternalCall();
uint256 gasUsedInternal = gasStartInternal - gasleft();
console2.log("Gas used for internal call:", gasUsedInternal);
assertTrue(gasUsedPublic > gasUsedInternal, "Public calls should use more gas");
}
}
Result:
forge test --match-contract VisibilityTest -vvv
[⠒] Compiling...
No files changed, compilation skipped
Ran 1 test for test/VisibilityTest.sol:VisibilityTest
[PASS] testPublicVsExternal() (gas: 1263042)
Logs:
Gas used for public calls: 1284297
Gas used for internal call: 11251
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 376.02µs (157.54µs CPU time)
Ran 1 test suite in 2.91ms (376.02µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Increased gas costs: 1,273,046 gas per transaction (~0.025 ETH at 20 gwei)
All users affected through higher transaction costs
Tools Used
slither .
forge test --match-contract VisibilityTest -vvv
Manuel code review
Recommendations
Change function visibility to external:
function startGame() external payable returns (uint256);
function hit() external;
function call() external;
function getPlayerCards(address) external view returns (uint256[] memory);
function getDealerCards(address) external view returns (uint256[] memory);
Updates
Rewards breakdown
nSLOC
147This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.