Audit of the Alkimiya.io V2.1 Silica smart contracts
Date | August 2023 |
Auditor | Tamara Ringas (@tamaraRingas) |
Alkimiya Technologies, Inc.
Alkimiya is a permissionless, open-source protocol for consensus capital markets. Proof-of-work (PoW) miners and proof-of-stake (PoS) validators can create swap contracts to sell their hashpower or staking power to buyers. These orders are facilitated through ERC-20 escrow contracts called Silica.
- Introduction
- Disclaimer
- Severity classification
- Scope
- Summary
- Security Findings
- Gas Findings
- Appendix I - Process Outline
- Appendix II - Static Analysis Reports
- Appendix III - Generated Documentation
Alkimiya requested a security and gas audit as well as any required remediation of the Silica smart contracts as of git commit hash f92215d.
There were 15 contracts in scope, as well as their parent contracts and corresponding interfaces.
After the audit and additional testing was completed, updates to the codebase were applied and further tests were added which can be found at git commit hash 4ce02ea.
More information about ALkimiya can be found on their documentation page
A smart contract security review can never verify the complete absence of vulnerabilities. Each audit is a time, resource and expertise bound effort that cannot guarantee 100% security. Subsequent security reviews and on-chain monitoring are strongly recommended. An Immunify bug bounty program has also been set up for the contracts.
- Source lines of code (SLOC) in source files: 3020
- Source lines of code (SLOC) in tests : 5654
The following smart contracts were in the scope of the security review:
- Abstract Silica
- Silica Factory
- Silica
- Silica ETH Staking
- Swap Proxy
- Rewards Proxy
- Oracle Registry
- Oracle
- Oracle PoS
- ETH Staking Oracle
The Alkimiya V2 Silica contracts were well written and thoroughly tested. As such, 0 high risk
findings were made, 5 medium findings
were identified and 4 minor findings
found. 10 gas optimisations
were found, 9 of which were implemented which saved a total of 386498 gas
across both deployment costs and recurring function calls.
None found.
1. A vulnerability with the OpenZeppelin 4.6
ECDSA library which is vulnerable to the signature malleability exploit was used in SwapProxy
.
Here is a PoC demonstrating how signature malleability using compact signatures can be exectued.
Resolution
The vulnerability was patched in version 4.7.3
. The patched library is now implemented instead of the vulnerable one.
2. OpenZeppelin Ownable library implemented instead of Ownable2Step.
The Ownable
implementation of access control can lead to loss of ownership in the event of a transferOwnership()
call. If the owner mistypes the newOwner
address, there is no way to recover that ownership.
Recommendation
It is recommend to use a two-step ownership transfer procedure, including the confirmation (accept) of the ownership transfer.
Resolution
Two-step ownership transfer was implemented by using Ownable2Step.
In the event of the presence of a bug or vulnerability in the codebase, there was no way to prevemt new Silica from being created or pausing the contracts until they were remedied.
Pause functionality was therefore implemented on the SilicaFactory
, SwapProxy
and RewardsProxy
contracts via inheritance of the the Pausable contract.
// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;
import "@openzeppelin/contracts/access/Ownable2Step.sol";
import "./interfaces/pause/IPausable.sol";
contract Pausable is Ownable2Step, IPausable {
bool public paused;
modifier whileNotPaused() {
require(!paused, "Contract is currently paused");
_;
}
function pause() external onlyOwner {
paused = true;
emit contractPaused(address(this));
}
function activate() external onlyOwner {
paused = false;
emit contractActivated(address(this));
}
}
For all Ownable
smart contracts except the Silica
& SilicsETHStaking
ones, the owner
account is the Alkimiya team. This account has many privledges that allow it to control the smart contract system.
The previous owner
address was that of an externally owned account
, which poses a risk to the system because it can be comprimised and the new controller of the owner
address would have immense control over the system.
Resolution
The owner
account was transferred to a multisig
smart contract, where multiple actors (EOA wallets) need to sign a transaction for it to be executed.
In OracleRegistry.sol
, the function setOracleAddress()
creates an oracle by setting the approved oracle address to correspond to an input token.
function setOracleAddress(
address _token,
uint256 _oracleType,
address _oracleAddr
) public onlyOwner {
oracleRegistry[_token][_oracleType] = _oracleAddr;
emit OracleRegistered(_token, _oracleType, _oracleAddr);
}
Recommendation
Add checks to ensure that the token
and oracle
addresses are valid.
Resolution
function setOracleAddress(
address _token,
uint256 _oracleType,
address _oracleAddr
) public onlyOwner {
require(_token != address(0), "Invalid Token Address");
require(_token.code.length > 0, "Invalid Token Contract");
require(_oracleAddr != address(0, "Invalid Oracle Address"))
oracleRegistry[_token][_oracleType] = _oracleAddr;
emit OracleRegistered(_token, _oracleType, _oracleAddr);
}
-
Unit tests for
RewardMath
&PayoutMath
were added. -
Fuzz tests were added for all contracts.
All smart contracts had a floating pragma set.
pragma solidity ^0.8.4;
This was replaced by solc version 0.8.19
which is a stable version of the Solidity compiler.
pragma solidity 0.8.19;
The use of version 0.8.19
also allowed for naming of mapping elements.
In Solidity, it is best practice for internal functions to named as follows
function _internalFunction() internal {}
This naming convention was implemented across all smart contracts.
Comments were added to all smart contracts and Foundry documentation was created.
- Once-off deployment calls =
282506 gas
- Recurring calls =
103992 gas
- Total =
386498 gas
i++
⇒++i
- Unchecked increment of
i
i
is default tozero
, needn't set it
Before
function getInRange(
uint256 _firstDay,
uint256 _lastDay
) external view override returns (uint256[] memory hashrateArray, uint256[] memory rewardArray) {
uint256 numElements = _lastDay + 1 - _firstDay;
rewardArray = new uint256[](numElements);
hashrateArray = new uint256[](numElements);
for (uint256 i = 0; i < numElements; i++) {
AlkimiyaIndex memory indexCopy = index[_firstDay + i];
rewardArray[i] = indexCopy.reward;
hashrateArray[i] = indexCopy.hashrate;
}
}
After
function getInRange(
uint256 _firstDay,
uint256 _lastDay
) external view override returns (uint256[] memory hashrateArray, uint256[] memory rewardArray) {
uint256 numElements = _lastDay + 1 - _firstDay;
rewardArray = new uint256[](numElements);
hashrateArray = new uint256[](numElements);
for (uint256 i ; i < numElements; ) {
AlkimiyaIndex memory indexCopy = index[_firstDay + i];
rewardArray[i] = indexCopy.reward;
hashrateArray[i] = indexCopy.hashrate;
unchecked { ++i; }
}
}
This three-step optimisation was applied to all arrays.
Before
⇒ RewardsProxy
deployment cost decreased by 9806 gas
(Once off expense)
⇒ streamRewards()
call decreased by 155 gas (per call)
Total RewardProxy
gas saved = 9961 gas
- Once-off calls =
9806 gas
- Recurring calls =
155 gas
⇒ SilicaV_1
deployment cost reduced by 11207 gas
(Once off expense)
⇒ buyerCollectPayout()
calls reduced by 4745 gas per call
⇒ buyerCollectPayoutOnDefault()
calls reduced by 578 gas per call
⇒getDayofDefault()
calls reduced by 357 gas per call
⇒ getRewardDeliveredSoFar()
calls reduced by 378 gas per call
⇒ getRewardDueNextOracleupdate()
calls reduced by 7404 gas per call
⇒ getStatus()
calls reduced by 7495 gas per call
⇒ isDefaulted()
lookup reduced by 93 gas per call
⇒ isFinished()
lookup reduced by 93 gas per call
⇒ isRunning()
lookup reduced by 93 gas per call
⇒ proxyDeposit()
calls reduced by 382 gas per call
⇒ reservedPrice()
lookup increased by 7 gas per call
⇒sellerCollectPayout()
calls reduced by 3285 gas per call
Total SilicaV2_1
gas saved = 36103 gas
- Once-off-payments =
11207 gas
- Recurring calls =
24896 gas
- In
AbstractSilicaV2_1
the constant state variableDAYS_BETWEEN_DD_AND_FDD
was auint8
- In
SilicaV2_1
&SilicaEthStaking
the commodity_type variable was stored as auint8
In Solidity, smaller variable types only save gas when they are packed together into shared storage slots. The above unit8
variables were not packed with anything, making them more expensive than uint256
because for Solidity to read those variable values, it still access a whole uint256
slot which requires casting the smaller variable into a uint256
one.
Before
SilicaV2_1
deployment cost reduced by 1200 gas
(Once off expense)
deposit()
calls reduced by 21 gas per call
getStatus()
calls reduced by 51 gas per call
isOpen()
calls reduced by 26 gas per call
lastDueDay()
calls reduced by 26 gas per call
proxyDeposit()
calls reduced by 21 gas per call
- Total Silica gas saved =
1345 gas
- Deployment gas saved =
1200 gas
- Recurring calls gas saved =
145 gas
*SilicaEthStakingV2_1
would have had similar gas reductions but no gas report could be produced.
Instead of copying function parameters to memory
, it is typically more cost-effective to load them immediately from calldata.
If all you need to do is read
data from function params, you can conserve gas by saving the data in calldata
instead.
⇒ Oracle.sol
’s read only function function parameters that were stored in memory
were changed to be read straight from calldata
⇒ OracleEthStaking.sol
’s read only function function parameters that were stored in memory
were changed to be read straight from calldata
⇒ RewardsProxy.sol
’s read only function function parameters that were stored in memory
were changed to be read straight from calldata
⇒ SwapProxy.sol
’s read only function function parameters that were stored in memory
were changed to be read straight from calldata
Before
FillBuyOrder:testAdditionalCollateral(uint256) (runs: 256, μ: 156644, ~: 54359)
OracleEthStakingUpdate:testInvalidPublisherSignature() (gas: 30968)
RewardsProxyTest:testStreamRewards() (gas: 222921)
CancelBuyOrder:testCancelledOrderCannotBeFulfilled() (gas: 45966)
After
CancelBuyOrder:testCancelledOrderCannotBeFulfilled() (gas: 45966)
⇒ 9589 gas saved
OracleEthStakingUpdate:testInvalidPublisherSignature() (gas: 30814)
⇒ updateOracleETHStaking calls (on revert) reduced by 154 gas
RewardsProxyTest:testStreamRewards() (gas: 222710)
⇒ RewardsProxy
deployment cost reduced by 12613 gas
(once-off payment)
⇒ streamRewards()
calls reduced by 211 gas per call
CancelBuyOrder:testCancelledOrderCannotBeFulfilled() (gas: 45930)
⇒ SwapProxy
deploy cost increased by 23629 gas
(once-off payment)
⇒ fillBuyOrder()
calls decreased by 44807 gas per call
⇒ cancelBuyOrder()
calls (with revert) decreased by 36 gas per call
⇒ 11016 gas cost incurred for deployment
(once-off payment)
⇒ 54797 gas saved on recurring calls
In OrderLib
there were both internal
and external
functions. Solidity libraries can either be embedded (all internal functions) or extended (all external functions) Embedded libraries inherit the library code into their bytecode
, so calling the library functions requires only a JUMP
instruction instead of a delegatecall
which reduces runtime gas usage for function calls.
- The external functions were made internal to make the library embedded and not extended as well, reducing gas costs of function calls by negating the need of excess
delegatecalls
.
Before
CancelBuyOrder:testCancelledOrderCannotBeFulfilled() (gas: 45966)
CancelBuyOrder:testOrderMarkedAsCancelled() (gas: 29735)
CancelSellOrder:testCancelledOrderCannotBeFulfilled() (gas: 45764)
CancelSellOrder:testOrderMarkedAsCancelled() (gas: 29624)
RouteBuy:testOrderMarkedAsCreated() (gas: 144035)
After
CancelBuyOrder:testCancelledOrderCannotBeFulfilled() (gas: 45966)
CancelBuyOrder:testOrderMarkedAsCancelled() (gas: 25653)
CancelSellOrder:testCancelledOrderCannotBeFulfilled() (gas: 45764)
CancelSellOrder:testOrderMarkedAsCancelled() (gas: 25643)
RouteBuy:testOrderMarkedAsCreated() (gas: 143963)
cancelBuyOrder()
calls cost reduced by 4082 gas per call
cancelSellOrder()
calls decreased by 3981 ****gas per call
routeBuy()
calls decreased by 72 gas per call
8135 gas
saved on recurring function calls
In SilicaV2_1 storage
, some variables were not packed into shared storage slots
.
The most gas costly operation in Solidity is a write to storage
, so these variables were reordered to enable variable packing which reduced the storage size by two slots.
Before
BuyerCollectPayout:testBuyerTriesCollectPayoutWhenOpen() (gas: 151502)
BuyerCollectPayout:testBuyerTriesCollectPayoutWhenRunning() (gas: 176324)
BuyerCollectPayout:testBuyerTriesToCollectTwice() (gas: 76221)
BuyerCollectPayout:testBuyersCollectPayoutOnFinishedContractWithExcess() (gas: 108261)
After
⇒ createEthStakingSilicaV2_1()
calls reduced by 536 gas per call
⇒ createSilicaV2_1()
calls reduced by 564 gas per call
⇒ proxyCreateSilicaV2_1()
calls reduced by 14694 gas per call
⇒ 15794 gas saved on recurring function calls
In Solidity, if a function is only to be called externally
but is defined with a visibility of public
, gas can be saved by changing the visibility to external
explicitly.
Unnecessary local variables were removed from both the RewardsProxy
and SwapProxy
contracts.
(These changes were rolled back for V2 release due to existing Silica contracts relying on them, but will be implemented in V3. The associated gas reductions are therefore not included in the tally)
⇒ Removal of Unnecessary Write Operations
-
In
Oracle.sol
, the parameterdifficulty
was written to but never read from, this parameter was removed, negating a daily storage write inAlkimiyaIndex
, and reads from it inSilicaV2_1
. -
In
OracleEthStaking.sol
, the parameterspriorityFee, burnFee, priorityFeeNormalized and burnFeeNormalized
were written to but never read from within the smart contracts, removal negated several daily storage writes inAlkimiyaEthStakingIndex
, and reads from it inSilicaEthStaking
.
Before
getRewardDueNextOracleUpdate:testWhenRunning() (gas: 5516249)
E2E_DefaultedEthStaking:testE2EToDefault() (gas: 836119)
E2E_Expired:testE2EToExpiry() (gas: 341256)
E2E_ExpiredEthStaking:testE2EToExpiry() (gas: 338865)
E2E_Finish:testE2E120DaysToFinish() (gas: 73017087)
E2E_Finish:testE2E3DayToFinish() (gas: 788709)
E2E_Finish:testE2E60DaysToFinish() (gas: 20091143)
E2E_FinishedEthStaking:testE2EFinished() (gas: 840026)
OracleUpdate:testUpdateRewardTokenOracleIndex() (gas: 110983)
After
getRewardDueNextOracleUpdate:testWhenRunning() (gas: 5481525)
E2E_DefaultedEthStaking:testE2EToDefault() (gas: 836119)
E2E_Expired:testE2EToExpiry() (gas: 341008)
E2E_ExpiredEthStaking:testE2EToExpiry() (gas: 338865)
E2E_Finish:testE2E120DaysToFinish() (gas: 72445622)
E2E_Finish:testE2E3DayToFinish() (gas: 786746)
E2E_Finish:testE2E60DaysToFinish() (gas: 19947822)
E2E_FinishedEthStaking:testE2EFinished() (gas: 837408)
OracleUpdate:testUpdateRewardTokenOracleIndex() (gas: 110193)
⇒ SilicaFactory.sol
deployment cost increased by 14613 gas
(once-off payment)
⇒ createSilicaV2_1()
calls reduced by 109 gas per call
⇒ proxyCreateSilicaV2_1()
calls reduced by 14335 gas per call
⇒ fillBuyOrder()
calls reduced by 44896 gas per call
⇒ fillSellOrder()
calls reduced by 76 gas per call
⇒ routeBuy()
calls reduced by 55 gas per call
⇒ getRewardDueNextOracleUpdate()
reduced by 34724 gas
(when in running state)
⇒ E2E-Expiry()
test was reduced by 248 gas
⇒ E2E-120DaysToFinish()
test was reduced by 571465 gas
⇒ E2E-3DaysToFinish()
test was reduced by 1963 gas
⇒ E2E-60DaysToFinish()
test was reduced by 143321 gas
⇒ E2E-Finish()
test was reduced by 2618 gas
⇒ updateIndex()
test was reduced by 790 gas
Before
OracleEthStakingUpdate:testGetInRange() (gas: 796954)
OracleEthStakingUpdate:testUpdateIndex() (gas: 200950)
E2E_FinishedEthStaking:testE2EFinished() (gas: 840026)
After
OracleEthStakingUpdate:testGetInRange() (gas: 796954)
OracleEthStakingUpdate:testUpdateIndex() (gas: 200950)
E2E_FinishedEthStaking:testE2EFinished() (gas: 840026)
⇒ SilicaFactory
deployment cost reduced by 8607 gas
(once-off payment)
⇒ createSilicaEthStakingV2_1()
calls reduced by 8687 gas per call
⇒ testGetInRange()
test reduced by 467700 gas per call
⇒testUpdateIndex()
test reduced by 100310 gas per call
⇒ testE2EFinish()
test reduced by 2618 gas per call
⇒ Parameter Resizing & Packing
- In
Oracle.sol
, the parameters ofAlkimiyaIndex
are not all of size256
, the variablesreferenceBlock
,timestamp
&hashrate
are smaller integer types which allows for them to all be packing into one storage slot. - In
OracleEthStaking.sol
, the parameters ofAlkimiyaEthStakingIndex
are both of size256
even though one is atimestamp
These variables were resized to save on a storage slot write and reads of it inAlkimiyaEthStakingIndex
Removed (comment out) unused Vault
code in SwapProxy
.
⇒ SwapProxy
deployment cost decreased by 271309 gas
(Once off expense)
⇒ buyOrderCancelled
lookup decreased by 11 gas per call
⇒ fillBuyOrder()
calls decreased by 11 gas per call
⇒ sellOrdersCancelled
lookup decreased by 56 gas per call
⇒ setSilicaFactory()
calls decreased by 12 gas per call
⇒ routeBuy()
calls increased by 9 gas per call
⇒ sellOrderToSilica
lookup increased by 11 gas per call
Total SwapProxy gas saved = 271379 gas
- Once-off calls =
271309 gas
- Recurring calls =
70 gas
Removed all unused imports.
Alkimiya V2.1 Smart Contract Audit Process Outline
-
Protocol Investigation (Build mental model of protocol as a whole)
- Architectural outline
- Review all current documentation
- Follow user flows
- Create call graphs & UML with Surya & Sol2UML
- List invariants
-
Scope Definitions
- Solidify which contracts are in and out of scope
- Remove out of scope contracts, if possible
-
Review Prior Security Work
- Examine previous audits
- Find any attack vectors the team has already found and review their patches
-
Audit Preparation
- Examine SLOC to estimate audit duration
- Source lines of code (SLOC) in source files: 3020
- Source lines of code (SLOC) in tests : 5654
- Tooling setup (Foundry, Slither, MythX, Olympix)
- Run tests and examine coverage for any gaps
- Read through code at a high level, tagging initial thoughts to come back to in more detail later
- Examine SLOC to estimate audit duration
-
In-Depth Code Review
- Read each line in detail
- Review previous tags in more detail
-
Write More Tests
- Fill in any coverage gaps found
- Add fuzz testing
-
Static Analysis
- Use Slither’s CLI to catch common issues
- Use Olympix to run high level security review
- Generate detailed static-analysis report using MythX
- Use Slither to write custom static checks
- Tag all static analysis findings in code to review again later
- Make necessary changes to the code based on static analysis findings
-
Attack Vector Ideation
- Enumerate all of the knobs an attacker can control.
- What public/external functions are there?
- What state could they affect?
- Does it matter if some of these transactions are front-run?
- Can sending ERC20 tokens or Ether to an address change the behaviour of the contracts?
- Investigate how all listed invariants could possibly be broken
-
Write Proof of Concepts for any Suspected Attack Vectors
- Write commented code to execute any suspected attack vectors, documenting results of their feasibility
-
Review All Audit Tags & Comments
- Go over all tags made in audit again and review them once more
- Remove all audit tags once accuracy of each has been determined
-
Gas Investigation
- Gas snapshots
- Gas optimisation review
- Implement gas optimisations
-
Write up Audit Report
- For each valid finding, populate a slide in the report with the description, file/line, criticality, status, and remediation
- Deliver the report along with the test suite
Static analysis was conducted useing Slither, Olympix and MythX.
The following reports were generated: