Description
Solidity rounds down the result of an integer division, and because of that, it is always recommended to multiply before dividing to avoid that precision loss.
In the case of a prior division over multiplication, the final result may face serious precision loss as the first answer would face truncated precision and
then multiplied to another integer.
The problem arises in the FjordAuction's auctionEnd()
function which is responsible for calculating the multiplier
variable:
function auctionEnd() external {
...
multiplier = totalTokens.mul(PRECISION_18).div(totalBids);
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
}
As one can see here, the multiplier
is calculated using the multiplication of totalTokens
and 1e18
and then divided by the totalBids
.
This by default is not a problem, however, considering the fact that when claiming the token, this variable is multiplied by the userBids
,
we may face significant precision loss (especially when the auction token's decimal is pretty low e.g. WBTC with 8 decimals):
function claimTokens() external {
...
uint256 claimable = userBids.mul(multiplier).div(PRECISION_18);
bids[msg.sender] = 0;
auctionToken.transfer(msg.sender, claimable);
emit TokensClaimed(msg.sender, claimable);
}
Thus, in the case of high bids, and low token decimals, bidders couldn't be able to call claimTokens()
and lose both their accumulated auction token rewards as well as their initial Fjord points investment.
Impact
Loss of auction tokens and Fjord Points due to the precision loss in case of low auction token decimal
The sum of individual claimables may not be equal the initial considered rewards
Proof of Concept
To illustrate the issue, we assume the auction token to be WBTC with a decimal of 8, and the total rewards of 100 WBTC.
In this specific test, we take the preceding steps:
An auction is started with 100 WBTC by the owner
Users bid ~ 4.999e28 points on the auction
Alice bids 8.8246e24 (to make the total bids become equal to 5e28)
After the auctionEnd()
is called, Alice will lose her rewards as well as point investments
pragma solidity =0.8.21;
import "../src/FjordStaking.sol";
import { FjordPoints } from "../src/FjordPoints.sol";
import { Test } from "forge-std/Test.sol";
import { MockERC20 } from "solmate/test/utils/mocks/MockERC20.sol";
import {FjordAuction} from "../src/FjordAuction.sol";
contract FjordTest is Test {
FjordAuction auction;
FjordStaking fjordStaking;
MockERC20 token;
MockERC20 wbtc;
FjordPoints points;
address internal constant SABLIER_ADDRESS = address(0xB10daee1FCF62243aE27776D7a92D39dC8740f95);
address minter = makeAddr("minter");
address newMinter = makeAddr("new_minter");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address authorizedSender = address(this);
function setUp() public {
points = new FjordPoints();
token = new MockERC20("Fjord", "FJO", 18);
fjordStaking =
new FjordStaking(address(token), address(this), SABLIER_ADDRESS, authorizedSender, address(points));
points.setStakingContract(address(fjordStaking));
wbtc = new MockERC20("Wrapped Bitcoin", "WBTC", 8);
auction = new FjordAuction(address(points), address(wbtc), 7 days, 10e9);
}
function testPrecisionLoss() public {
deal(address(points), address(this), 1e30);
assertEq(points.balanceOf(address(this)), 1e30);
points.approve(address(auction), type(uint256).max);
auction.bid(49991.1754e24);
vm.warp(block.timestamp + 2 days);
deal(address(points), alice, 1e25);
assertEq(points.balanceOf(address(alice)), 1e25);
vm.startPrank(alice);
points.approve(address(auction), type(uint256).max);
auction.bid(8.8246e24);
vm.stopPrank();
vm.warp(block.timestamp + 7 days);
auction.auctionEnd();
vm.prank(alice);
auction.claimTokens();
console.log("Alice's auction token balance after claiming is: ", token.balanceOf(alice));
uint actualReward = (8.8246e24 * 10e9 * 1e18) / (1e18 * 5e28);
console.log("Alice's actual auction token balance should be: ", actualReward);
}
}
The result is:
Ran 1 test for test/FjordTest.t.sol:FjordTest
[PASS] testPrecisionLoss() (gas: 537906)
Logs:
Alice's auction token balance after claiming is: 0
Alice's actual auction token balance should be: 1764920
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.29ms (2.13ms CPU time)
This test shows that for the case of totalBids = 5e28
, and Alice's userBids = 8.8246e24
, WBTC's totalTokens = 10e9
:
Expected rewards for Alice: 1764920 (~ $1058 with WBTC price equal to $60k)
Actual rewards for Alice: 0
Tools Used
Manual Review
Recommendations
Consider selecting a proper precision multiplier considering the auction tokens decimal and also prioritizing the multiplication over division.