UpdateSilo Manual Review Findings

UpdateSilo Manual Review Findings

USO-01M: Potential Truncation of Unclaimed Root Accounting

Description:

The legacy farming of beans for the V2 implementation uses the concept of unclaimed roots, however, all the calculations utilise fractional operations that are prone to truncation and as such the unclaimed roots of a particular account can exceed the total unclaimed roots for the last person to claim them.

Example:

protocol/contracts/farm/facets/SiloFacet/UpdateSilo.sol
72function farmLegacyBeans(address account, uint32 update) private {
73 uint256 beans;
74 if (update < s.hotFix3Start) {
75 beans = balanceOfFarmableBeansV1(account);
76 if (beans > 0) s.v1SI.beans = s.v1SI.beans.sub(beans);
77 }
78
79 uint256 unclaimedRoots = balanceOfUnclaimedRoots(account);
80 uint256 beansV2 = balanceOfFarmableBeansV2(unclaimedRoots);
81 beans = beans.add(beansV2);
82 if (beansV2 > 0) s.v2SIBeans = s.v2SIBeans.sub(beansV2);
83 s.unclaimedRoots = s.unclaimedRoots.sub(unclaimedRoots);
84 s.a[account].lastSIs = s.season.sis;
85
86 uint256 seeds = beans.mul(C.getSeedsPerBean());
87 s.a[account].s.seeds = s.a[account].s.seeds.add(seeds);
88 s.a[account].s.stalk = s.a[account].s.stalk.add(beans.mul(C.getStalkPerBean()));
89 addBeanDeposit(account, season(), beans);
90}

Recommendation:

We advise a similar paradigm to balanceOfFarmableBeansV2 to be imposed whereby the total unclaimed roots are simply set to zero if the account's unclaimed roots exceed them.

Alleviation:

The same paradigm as balanceOfFarmableBeansV2 has been applied to the relevant statements as advised via a ternary operator.

View Fix on GitHub
Navigated to UpdateSilo Manual Review Findings