| Number | Details | Instances |
|---|---|---|
| [Medium-1] | Chainlink oracle will return the wrong price if the aggregator hits minAnswer | 1 |
| [Medium-2] | Privileged functions can create points of failure | 4 |
| Number | Details | Instances |
|---|---|---|
| [Low-1] | Code does not follow the best practice of check-effects-interaction | 2 |
| [Low-2] | State variable can be updated more than once in a function | 1 |
| [Low-3] | Solidity version 0.8.20 won't work on all chains due to PUSH0 | 3 |
| [Low-4] | Using > when declaring solidity version without specifying an upperbound can cause future vulnerabilities | 1 |
| [Low-5] | The function decimals() is not part of the ERC20 standard | 2 |
| [Low-6] | Loss of precision | 1 |
| [Low-7] | Missing zero address check in constructor | 6 |
| [Low-8] | Using zero as a parameter | 1 |
| [Low-9] | Chainlink price feed decimals not checked | 1 |
| [Low-10] | Chainlink answer is not compared against min/max values | 1 |
| [Low-11] | Division in comparison | 1 |
| [Low-12] | Consider a uptime feed on L2 deployments to prevent issues caused by downtime | 10 |
| [Low-13] | Remove forge-std import | 2 |
| [Low-14] | Unsafe uint to int conversion | 2 |
| Number | Details | Instances |
|---|---|---|
| [NonCritical-1] | Addresses shouldn't be hard-coded | 2 |
| [NonCritical-2] | Default address(0) can be returned | 2 |
| [NonCritical-3] | Floating pragma should be avoided | 3 |
| [NonCritical-4] | Events regarding state variable changes should emit the previous state variable value | 4 |
| [NonCritical-5] | In functions which accept an address as a parameter, there should be a zero address check to prevent bugs | 20 |
| [NonCritical-6] | Enum values should be used in place of constant array indexes | 2 |
| [NonCritical-7] | Revert statements within external and public functions can be used to perform DOS attacks | 4 |
| [NonCritical-8] | Private and internal state variables should have a preceding _ in their name unless they are constants | 3 |
| [NonCritical-9] | Contract lines should not be longer than 120 characters for readability | 4 |
| [NonCritical-10] | Old Solidity version | 3 |
| [NonCritical-11] | Not all event definitions are utilizing indexed variables. | 1 |
| [NonCritical-12] | Contracts should have all public/external functions exposed by interfaces | 11 |
| [NonCritical-13] | Constants should be on the left side of the comparison | 2 |
| [NonCritical-14] | Interface names should have an I as the first character | 1 |
| [NonCritical-15] | Use of non-named numeric constants | 4 |
| [NonCritical-16] | Long powers of ten should use scientific notation 1eX | 1 |
| [NonCritical-17] | Redundant else statement | 2 |
| [NonCritical-18] | Cyclomatic complexity in functions | 2 |
| [NonCritical-19] | Contract does not follow the Solidity style guide's suggested layout ordering | 1 |
| [NonCritical-20] | Events may be emitted out of order due to code not follow the best practice of check-effects-interaction | 1 |
| [NonCritical-21] | Avoid mutating function parameters | 2 |
| [NonCritical-22] | Immutable and constant integer state variables should not be casted | 1 |
| [NonCritical-23] | Floating pragma defined inconsistently | 3 |
| [NonCritical-24] | Use 'using' keyword when using specific imports rather than calling the specific import directly | 34 |
| [NonCritical-25] | Inconsistent checks of address params against address(0) | 1 |
| [NonCritical-26] | Constructors should emit an event | 8 |
| [NonCritical-27] | Constructor with array/string/bytes parameters has no empty array checks | 2 |
| [NonCritical-28] | Errors should have parameters | 4 |
| [NonCritical-29] | Constant state variables defined more than once | 2 |
| Number | Details | Instances | Gas |
|---|---|---|---|
| [Gas-1] | Using named returns for pure and view functions is cheaper than using regular returns | 20 | 10400 |
| [Gas-2] | Avoid caching immutable state variables | 1 | 0.0 |
| [Gas-3] | Usage of smaller uint/int types causes overhead | 28 | 43120 |
| [Gas-4] | Use != 0 instead of > 0 | 2 | 12 |
| [Gas-5] | Use assembly to check for the zero address | 1 | 0.0 |
| [Gas-6] | Divisions which do not divide by -X cannot overflow or underflow so such operations can be unchecked to save gas | 1 | 0.0 |
| [Gas-7] | Superfluous event fields | 1 | 34 |
| [Gas-8] | Consider using OZ EnumerateSet in place of nested mappings | 1 | 1000 |
| [Gas-9] | Use assembly to emit events | 5 | 950 |
| [Gas-10] | Use assembly in place of abi.decode to extract calldata values more efficiently | 1 | 0.0 |
| [Gas-11] | Using private rather than public for constants and immutables, saves gas | 10 | 0.0 |
| [Gas-12] | Simple checks for zero uint can be done using assembly to save gas | 2 | 24 |
| [Gas-13] | Using nested if to save gas | 9 | 486 |
| [Gas-14] | Stack variable cost less than state variables while used in emiting event | 1 | 9 |
| [Gas-15] | Solidity versions 0.8.19 and above are more gas efficient | 3 | 9000 |
| [Gas-16] | Internal functions only used once can be inlined to save gas | 1 | 30 |
| [Gas-17] | Constructors can be marked as payable to save deployment gas | 8 | 0.0 |
| [Gas-18] | Assigning to structs can be more efficient | 1 | 130 |
| [Gas-19] | Only emit event in setter function if the state variable was changed | 1 | 0.0 |
| [Gas-20] | Use 'storage' instead of 'memory' for struct/array state variables | 2 | 8400 |
| [Gas-21] | Use constants instead of type(uint).max | 2 | 0.0 |
Chainlink oracles use data aggregators to provide asset prices, incorporating mechanisms like circuit breakers to handle extreme market conditions. These circuit breakers, including parameters like minAnswer, are designed to prevent erratic data or manipulation. However, if an asset's price plummets rapidly beyond these predefined bands (as seen in the LUNA crash), the oracle might freeze and keep returning the minPrice, not reflecting the asset's actual market value. This discrepancy can lead to incorrect borrowing rates in DeFi platforms, as was observed in the Venus incident on BSC, where users were able to borrow against devalued collateral, leading to significant platform losses. This scenario underscores the importance of robust oracle design and risk management strategies in DeFi protocols.
Num of instances: 1
Click to show findings
['63']
63: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
64: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
65:
66: (, int256 answer,, uint256 updatedAt,) = AggregatorV3Interface(feed).latestRoundData(); // <= FOUND
67: if (answer <= 0) revert Errors.PriceOracle_InvalidAnswer();
68: uint256 staleness = block.timestamp - updatedAt;
69: if (staleness > maxStaleness) revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
70:
71: uint256 price = uint256(answer);
72: return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse);
73: }Ensure such accounts are protected and consider implementing multi sig to prevent a single point of failure
Num of instances: 4
Click to show findings
['56']
56: function govSetConfig(address base, address quote, address oracle) external onlyGovernor { // <= FOUND
57:
58: if (base == quote) revert Errors.PriceOracle_InvalidConfiguration();
59: (address asset0, address asset1) = _sort(base, quote);
60: oracles[asset0][asset1] = oracle;
61: emit ConfigSet(asset0, asset1, oracle);
62: }['69']
69: function govSetResolvedVault(address vault, bool set) external onlyGovernor { // <= FOUND
70: address asset = set ? IERC4626(vault).asset() : address(0);
71: resolvedVaults[vault] = asset;
72: emit ResolvedVaultSet(vault, asset);
73: }['78']
78: function govSetFallbackOracle(address _fallbackOracle) external onlyGovernor { // <= FOUND
79: fallbackOracle = _fallbackOracle;
80: emit FallbackOracleSet(_fallbackOracle);
81: }['26']
26: function transferGovernance(address newGovernor) external onlyGovernor { // <= FOUND
27: _setGovernor(newGovernor);
28: }The "check-effects-interaction" pattern is a best practice in smart contract development, emphasizing the order of operations in functions to prevent reentrancy attacks. Violations arise when a function interacts with external contracts before settling internal state changes or checks. This misordering can expose the contract to potential threats. To adhere to this pattern, first ensure all conditions or checks are satisfied, then update any internal states, and only after these steps, interact with external contracts or addresses. Rearranging operations to this recommended sequence bolsters contract security and aligns with established best practices in the Ethereum community.
Num of instances: 2
Click to show findings
['69']
69: function govSetResolvedVault(address vault, bool set) external onlyGovernor { // <= FOUND
70: address asset = set ? IERC4626(vault).asset() : address(0); // <= FOUND
71: resolvedVaults[vault] = asset;
72: emit ResolvedVaultSet(vault, asset);
73: }['123']
123: function resolveOracle(uint256 inAmount, address base, address quote)
124: public
125: view
126: returns (uint256, address, address, address )
127: {
128:
129: if (base == quote) return (inAmount, base, quote, address(0));
130:
131: address oracle = getConfiguredOracle(base, quote);
132: if (oracle != address(0)) return (inAmount, base, quote, oracle);
133:
134: address baseAsset = resolvedVaults[base];
135: if (baseAsset != address(0)) {
136: inAmount = IERC4626(base).convertToAssets(inAmount); // <= FOUND
137: return resolveOracle(inAmount, baseAsset, quote);
138: }
139:
140: oracle = fallbackOracle;
141: if (oracle == address(0)) revert Errors.PriceOracle_NotSupported(base, quote);
142: return (inAmount, base, quote, oracle);
143: }Updating a state variable multiple times within a function can lead to inefficiencies and unintended behaviors. Every state change in Ethereum consumes gas, increasing the transaction cost. Moreover, frequent state changes can introduce vulnerabilities if interim values can be externally observed or acted upon. To address this, one should consolidate updates to occur only once, at the end of the function. This minimizes gas usage and ensures consistent state.
Num of instances: 1
Click to show findings
['78']
78: function updatePrice(uint48 timestamp) external {
79:
80: Cache memory _cache = cache;
81: if (timestamp <= _cache.priceTimestamp) return;
82:
83: if (block.timestamp > timestamp) {
84:
85: uint256 staleness = block.timestamp - timestamp;
86: if (staleness > maxStaleness) {
87: revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
88: }
89: } else if (timestamp - block.timestamp > RedstoneDefaultsLib.DEFAULT_MAX_DATA_TIMESTAMP_AHEAD_SECONDS) {
90:
91: revert Errors.PriceOracle_InvalidAnswer();
92: }
93:
94:
95: cache = Cache({price: _cache.price, priceTimestamp: timestamp}); // <= FOUND
96:
97:
98: uint256 price = getOracleNumericValueFromTxMsg(feedId);
99: if (price == 0) revert Errors.PriceOracle_InvalidAnswer();
100: if (price > type(uint208).max) revert Errors.PriceOracle_Overflow();
101: cache = Cache({price: uint208(price), priceTimestamp: timestamp}); // <= FOUND
102: emit CacheUpdated(price, timestamp);
103: }Solidity version 0.8.20 uses the new Shanghai EVM which introduces the PUSH0 opcode, this may not be implemented on all chains and L2 thus reducing the portability and compatability of the code. Consider using a earlier solidity version.
Num of instances: 3
Click to show findings
['2']
2: pragma solidity ^0.8.0; // <= FOUND['2']
2: pragma solidity ^0.8.16; // <= FOUND['2']
2: pragma solidity >=0.8.0; // <= FOUND[Low-4] Using > when declaring solidity version without specifying an upperbound can cause future vulnerabilities
Such instances should be locked to a particular solidity version or use the ^ attribute instead of > as this is more limited in what versions can be used.
Num of instances: 1
The decimals() function in an ERC20 token contract is used to specify how many decimal places the token can be divided into. However, it should be used with caution because not all ERC20 token contracts implement decimals(), and the function isn't required by the ERC20 standard. Calling decimals() on a contract that doesn't implement it will result in a runtime error. Moreover, even when implemented, the returned value can be manipulated or artificially set, which may cause unexpected behavior. Therefore, always verify the decimal count independently if precision is crucial for your contract logic. When interacting with other ERC20 tokens, consider implementing safeguards or checks to handle potential errors from calling decimals().
Num of instances: 2
Click to show findings
['54']
54: uint8 feedDecimals = AggregatorV3Interface(feed).decimals(); // <= FOUND['52']
52: uint8 feedDecimals = IChronicle(feed).decimals(); // <= FOUNDDividing by large numbers in Solidity can cause a loss of precision due to the language's inherent integer division behavior. Solidity does not support floating-point arithmetic, and as a result, division between integers yields an integer result, truncating any fractional part. When dividing by a large number, the resulting value may become significantly smaller, leading to a loss of precision, as the fractional part is discarded.
Num of instances: 1
Click to show findings
['115']
115: function _fetchPriceStruct() internal view returns (PythStructs.Price memory) { // <= FOUND
116: PythStructs.Price memory p = IPyth(pyth).getPriceUnsafe(feedId);
117:
118: if (p.publishTime < block.timestamp) {
119:
120: uint256 staleness = block.timestamp - p.publishTime;
121: if (staleness > maxStaleness) revert Errors.PriceOracle_InvalidAnswer();
122: } else {
123:
124: uint256 aheadness = p.publishTime - block.timestamp;
125: if (aheadness > MAX_AHEADNESS) revert Errors.PriceOracle_InvalidAnswer();
126: }
127:
128:
129: if (p.price <= 0 || p.conf > uint64(p.price) * maxConfWidth / BASIS_POINTS) { // <= FOUND
130: revert Errors.PriceOracle_InvalidAnswer();
131: }
132:
133:
134: if (p.expo < MIN_EXPONENT || p.expo > MAX_EXPONENT) {
135: revert Errors.PriceOracle_InvalidAnswer();
136: }
137: return p;
138: }In Solidity, constructors often take address parameters to initialize important components of a contract, such as owner or linked contracts. However, without a check, there's a risk that an address parameter could be mistakenly set to the zero address (0x0). This could occur due to a mistake or oversight during contract deployment. A zero address in a crucial role can cause serious issues, as it cannot perform actions like a normal address, and any funds sent to it are irretrievable. Therefore, it's crucial to include a zero address check in constructors to prevent such potential problems. If a zero address is detected, the constructor should revert the transaction.
Num of instances: 6
Click to show findings
['41']
41: constructor(address _base, address _quote, address _feed, uint256 _maxStaleness) { // <= FOUND
42: if (_maxStaleness < MAX_STALENESS_LOWER_BOUND || _maxStaleness > MAX_STALENESS_UPPER_BOUND) {
43: revert Errors.PriceOracle_InvalidConfiguration();
44: }
45:
46: base = _base;
47: quote = _quote;
48: feed = _feed;
49: maxStaleness = _maxStaleness;
50:
51:
52: uint8 baseDecimals = _getDecimals(base);
53: uint8 quoteDecimals = _getDecimals(quote);
54: uint8 feedDecimals = AggregatorV3Interface(feed).decimals();
55: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, feedDecimals);
56: }['39']
39: constructor(address _base, address _quote, address _feed, uint256 _maxStaleness) { // <= FOUND
40: if (_maxStaleness < MAX_STALENESS_LOWER_BOUND || _maxStaleness > MAX_STALENESS_UPPER_BOUND) {
41: revert Errors.PriceOracle_InvalidConfiguration();
42: }
43:
44: base = _base;
45: quote = _quote;
46: feed = _feed;
47: maxStaleness = _maxStaleness;
48:
49:
50: uint8 baseDecimals = _getDecimals(base);
51: uint8 quoteDecimals = _getDecimals(quote);
52: uint8 feedDecimals = IChronicle(feed).decimals();
53: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, feedDecimals);
54: }['37']
37: constructor(address _base, address _cross, address _quote, address _oracleBaseCross, address _oracleCrossQuote) { // <= FOUND
38: base = _base;
39: cross = _cross;
40: quote = _quote;
41: oracleBaseCross = _oracleBaseCross;
42: oracleCrossQuote = _oracleCrossQuote;
43: }['19']
19: constructor(address _governor) { // <= FOUND
20: _setGovernor(_governor);
21: }['64']
64: constructor(
65: address _pyth, // <= FOUND
66: address _base, // <= FOUND
67: address _quote, // <= FOUND
68: bytes32 _feedId,
69: uint256 _maxStaleness,
70: uint256 _maxConfWidth
71: ) {
72: if (_maxStaleness > MAX_STALENESS_UPPER_BOUND) {
73: revert Errors.PriceOracle_InvalidConfiguration();
74: }
75: if (_maxConfWidth < MAX_CONF_WIDTH_LOWER_BOUND || _maxConfWidth > MAX_CONF_WIDTH_UPPER_BOUND) {
76: revert Errors.PriceOracle_InvalidConfiguration();
77: }
78:
79: pyth = _pyth;
80: base = _base;
81: quote = _quote;
82: feedId = _feedId;
83: maxStaleness = _maxStaleness;
84: maxConfWidth = _maxConfWidth;
85: baseDecimals = _getDecimals(base);
86: quoteDecimals = _getDecimals(quote);
87: }['59']
59: constructor(address _base, address _quote, bytes32 _feedId, uint8 _feedDecimals, uint256 _maxStaleness) { // <= FOUND
60: if (_maxStaleness > MAX_STALENESS_UPPER_BOUND) revert Errors.PriceOracle_InvalidConfiguration();
61:
62: base = _base;
63: quote = _quote;
64: feedId = _feedId;
65: feedDecimals = _feedDecimals;
66: maxStaleness = _maxStaleness;
67: uint8 baseDecimals = _getDecimals(base);
68: uint8 quoteDecimals = _getDecimals(quote);
69: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, _feedDecimals);
70: }Taking 0 as a valid argument in Solidity without checks can lead to severe security issues. A historical example is the infamous 0x0 address bug where numerous tokens were lost. This happens because '0' can be interpreted as an uninitialized address, leading to transfers to the '0x0' address, effectively burning tokens. Moreover, 0 as a denominator in division operations would cause a runtime exception. It's also often indicative of a logical error in the caller's code. It's important to always validate input and handle edge cases like 0 appropriately. Use require() statements to enforce conditions and provide clear error messages to facilitate debugging and safer code.
Num of instances: 1
Click to show findings
['94']
94: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) { // <= FOUND
95: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
96:
97: PythStructs.Price memory priceStruct = _fetchPriceStruct();
98:
99: uint256 price = uint256(uint64(priceStruct.price));
100: int8 feedExponent = int8(baseDecimals) - int8(priceStruct.expo);
101:
102: Scale scale;
103: if (feedExponent > 0) {
104: scale = ScaleUtils.from(quoteDecimals, uint8(feedExponent));
105: } else {
106: scale = ScaleUtils.from(quoteDecimals + uint8(-feedExponent), 0); // <= FOUND
107: }
108: return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse);
109: }Price feeds provided by Chainlink oracles are an essential resource for many DeFi applications in the Ethereum ecosystem, enabling smart contracts to access reliable, tamper-resistant price data. However, these price feeds are often normalized using a certain number of decimal places, a fact which can potentially lead to discrepancies or inaccuracies if not properly accounted for.
For example, if a Chainlink price feed returns a price with 8 decimal places, but your contract assumes the price has 18 decimal places (which is common with Ether and many ERC20 tokens), this could cause an underflow in your contract's calculations, which might lead to incorrect operations or financial discrepancies.
As a best practice, your contract should dynamically check the number of decimals that the Chainlink price feed uses. This information can be retrieved via the decimals() function provided by the Chainlink AggregatorV3Interface. Once you have this information, you can correctly scale the price data to match your contract's assumptions, ensuring that all calculations are accurate and consistent.
This practice not only makes your contract more resilient to potential discrepancies in the data feed but also enhances its interoperability by ensuring it can correctly interact with different types of price feeds, each possibly having different decimal normalization.
Num of instances: 1
Click to show findings
['63']
63: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
64: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
65:
66: (, int256 answer,, uint256 updatedAt,) = AggregatorV3Interface(feed).latestRoundData(); // <= FOUND
67: if (answer <= 0) revert Errors.PriceOracle_InvalidAnswer();
68: uint256 staleness = block.timestamp - updatedAt;
69: if (staleness > maxStaleness) revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
70:
71: uint256 price = uint256(answer);
72: return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse);
73: }Chainlink oracle provides reliable, real-time data feeds to smart contracts. However, in order to enhance security and minimize the potential impact of an oracle failure or manipulation, it's a good practice to establish minimum and maximum thresholds for these data inputs. Without this safeguard, an erroneous or maliciously manipulated data point from the oracle could potentially lead to severe consequences in contract behavior. Therefore, the values retrieved from Chainlink's oracle should be cross-verified against preset min/max boundaries to ensure they fall within the expected range. This extra layer of validation adds robustness and reduces the risk of oracle-related issues.
Num of instances: 1
Click to show findings
['63']
63: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
64: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
65:
66: (, int256 answer,, uint256 updatedAt,) = AggregatorV3Interface(feed).latestRoundData();
67: if (answer <= 0) revert Errors.PriceOracle_InvalidAnswer();
68: uint256 staleness = block.timestamp - updatedAt;
69: if (staleness > maxStaleness) revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
70:
71: uint256 price = uint256(answer);
72: return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse);
73: }To ensure accuracy in comparisons within programming, especially when dealing with integers, it's often more efficient to use multiplication rather than division. This approach stems from the fact that division operations are generally slower and more complex than multiplication. And in the context of solidity they can cause precision errors.
Suppose you want to compare if a/b is greater than c/d (where a, b, c, and d are integers). Instead of performing division, which is prone to precision errors, you can cross-multiply to avoid division. The comparison a/b > c/d is equivalent to ad > bc. This way, you only use multiplication, which is faster and avoids potential inaccuracies or complexities associated with division.
Num of instances: 1
Click to show findings
['129']
129:
130: if (p.price <= 0 || p.conf > uint64(p.price) * maxConfWidth / BASIS_POINTS) { // <= FOUNDIn L2 deployments, incorporating an uptime feed is crucial to mitigate issues arising from sequencer downtime. Downtime can disrupt services, leading to transaction failures or incorrect data readings, affecting overall system reliability. By integrating an uptime feed, you gain insight into the operational status of the L2 network, enabling proactive measures like halting sensitive operations or alerting users. This approach ensures that your contract behaves predictably and securely during network outages, enhancing the robustness and reliability of your decentralized application, which is especially important in mission-critical or high-stakes environments.
Num of instances: 10
Click to show findings
['14']
14: contract ChainlinkOracle is BaseAdapter // <= FOUND['14']
14: contract ChronicleOracle is BaseAdapter // <= FOUND['13']
13: contract CrossAdapter is BaseAdapter // <= FOUND['16']
16: contract EulerRouter is Governable, IPriceOracle // <= FOUND['11']
11: contract LidoOracle is BaseAdapter // <= FOUND['16']
16: contract PythOracle is BaseAdapter // <= FOUND['14']
14: contract RedstoneCoreOracle is PrimaryProdDataServiceConsumerBase, BaseAdapter // <= FOUND['21']
21: contract UniswapV3Oracle is BaseAdapter // <= FOUND['12']
12: abstract contract BaseAdapter is IPriceOracle // <= FOUND['10']
10: abstract contract Governable // <= FOUNDLogging and debugging libraries such as forge-std are crucial tools during the development phase of a smart contract, as they provide critical insights into the execution of the contract. However, it's essential to remove or disable these libraries in the production version of your contract. Leaving such libraries active in the final version can lead to unnecessary gas costs, as logging operations consume gas, and it can potentially expose sensitive internal state information.
Num of instances: 2
Click to show findings
['4']
4: import {IERC20} from "forge-std/interfaces/IERC20.sol"; // <= FOUND['4']
4: import {IERC4626} from "forge-std/interfaces/IERC4626.sol"; // <= FOUNDUnsafe conversion from uint to int in Solidity can lead to unexpected overflows if the unsigned integer value exceeds the positive limit of the corresponding signed integer type. This is due to the fact that signed integers use one bit for the sign, effectively halving the positive range. An example is converting a uint256 number greater than type(uint128).max to an int256, which can result in an overflow. To mitigate this risk, consider using libraries like SafeCast, which include functions specifically designed to safely cast between different numerical types. They perform necessary checks and revert the transaction if an unsafe cast is attempted, thereby enhancing the security and robustness of the code.
Num of instances: 2
Click to show findings
['94']
94: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) { // <= FOUND
95: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
96:
97: PythStructs.Price memory priceStruct = _fetchPriceStruct();
98:
99: uint256 price = uint256(uint64(priceStruct.price));
100: int8 feedExponent = int8(baseDecimals) - int8(priceStruct.expo); // <= FOUND
101:
102: Scale scale;
103: if (feedExponent > 0) {
104: scale = ScaleUtils.from(quoteDecimals, uint8(feedExponent));
105: } else {
106: scale = ScaleUtils.from(quoteDecimals + uint8(-feedExponent), 0);
107: }
108: return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse);
109: }['61']
61: function _getQuote(uint256 inAmount, address base, address quote) internal view override returns (uint256) { // <= FOUND
62: if (!((base == tokenA && quote == tokenB) || (base == tokenB && quote == tokenA))) {
63: revert Errors.PriceOracle_NotSupported(base, quote);
64: }
65:
66: if (inAmount > type(uint128).max) revert Errors.PriceOracle_Overflow();
67:
68: uint32[] memory secondsAgos = new uint32[](2);
69: secondsAgos[0] = twapWindow;
70:
71:
72: (int56[] memory tickCumulatives,) = IUniswapV3Pool(pool).observe(secondsAgos);
73: int56 tickCumulativesDelta = tickCumulatives[1] - tickCumulatives[0];
74: int24 tick = int24(tickCumulativesDelta / int56(uint56(twapWindow))); // <= FOUND
75: if (tickCumulativesDelta < 0 && (tickCumulativesDelta % int56(uint56(twapWindow)) != 0)) tick--; // <= FOUND
76: return OracleLibrary.getQuoteAtTick(tick, uint128(inAmount), base, quote);
77: }Num of instances: 2
Click to show findings
['16']
16:
18: address public constant STETH = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84; // <= FOUND['19']
19:
21: address public constant WSTETH = 0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0; // <= FOUNDAllowing a function in Solidity to return the default address (address(0)) can be problematic as it can represent uninitialized or invalid addresses. If such an address is utilized in transfer operations or other sensitive actions, it could lead to loss of funds or unpredicted behavior. It's prudent to include checks in your functions to prevent the return of the zero address, enhancing contract security.
Num of instances: 2
Click to show findings
['103']
103: function getConfiguredOracle(address base, address quote) public view returns (address) {
104: (address asset0, address asset1) = _sort(base, quote);
105: return oracles[asset0][asset1];
106: }['150']
150: function _sort(address assetA, address assetB) internal pure returns (address, address) {
151: return assetA < assetB ? (assetA, assetB) : (assetB, assetA);
152: }Num of instances: 3
Click to show findings
['2']
2: pragma solidity ^0.8.0; // <= FOUND['2']
2: pragma solidity ^0.8.16; // <= FOUND['2']
2: pragma solidity >=0.8.0; // <= FOUND[NonCritical-4] Events regarding state variable changes should emit the previous state variable value
Modify such events to contain the previous value of the state variable as demonstrated in the example below
Num of instances: 4
Click to show findings
['34']
34: event ConfigSet(address indexed asset0, address indexed asset1, address indexed oracle);['38']
38: event FallbackOracleSet(address indexed fallbackOracle);['43']
43: event ResolvedVaultSet(address indexed vault, address indexed asset);['49']
49: event CacheUpdated(uint256 price, uint256 priceTimestamp);[NonCritical-5] In functions which accept an address as a parameter, there should be a zero address check to prevent bugs
In smart contract development, especially with Solidity, it's crucial to validate inputs to functions. When a function accepts an Ethereum address as a parameter, implementing a zero address check (i.e., ensuring the address is not 0x0) is a best practice to prevent potential bugs and vulnerabilities. The zero address (0x0) is a default value and generally indicates an uninitialized or invalid state. Passing the zero address to certain functions can lead to unintended behaviors, like funds getting locked permanently or transactions failing silently. By checking for and rejecting the zero address, developers can ensure that the function operates as intended and interacts only with valid Ethereum addresses. This check enhances the contract's robustness and security.
Num of instances: 20
Click to show findings
['84']
84: function getQuote(uint256 inAmount, address base, address quote) external view returns (uint256)['92']
92: function getQuotes(uint256 inAmount, address base, address quote) external view returns (uint256, uint256)['32']
32: function _getDecimals(address asset) internal view returns (uint8)['118']
118: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256)['118']
118: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256)['118']
118: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256)['56']
56: function govSetConfig(address base, address quote, address oracle) external onlyGovernor['69']
69: function govSetResolvedVault(address vault, bool set) external onlyGovernor['78']
78: function govSetFallbackOracle(address _fallbackOracle) external onlyGovernor['84']
84: function getQuote(uint256 inAmount, address base, address quote) external view returns (uint256)['92']
92: function getQuotes(uint256 inAmount, address base, address quote) external view returns (uint256, uint256)['103']
103: function getConfiguredOracle(address base, address quote) public view returns (address)['150']
150: function _sort(address assetA, address assetB) internal pure returns (address, address)['26']
26: function transferGovernance(address newGovernor) external onlyGovernor['40']
40: function _setGovernor(address newGovernor) internal['61']
61: function _getQuote(uint256 inAmount, address base, address quote) internal view override returns (uint256)['118']
118: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256)['118']
118: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256)['38']
38: function getDirectionOrRevert(address givenBase, address base, address givenQuote, address quote)
39: internal
40: pure
41: returns (bool)
42:['61']
61: function _getQuote(uint256 inAmount, address base, address quote) internal view override returns (uint256)Create a commented enum value to use in place of constant array indexes, this makes the code far easier to understand
Num of instances: 2
Click to show findings
['69']
69: secondsAgos[0] = twapWindow; // <= FOUND['73']
73: int56 tickCumulativesDelta = tickCumulatives[1] - tickCumulatives[0]; // <= FOUND[NonCritical-7] Revert statements within external and public functions can be used to perform DOS attacks
In Solidity, 'revert' statements are used to undo changes and throw an exception when certain conditions are not met. However, in public and external functions, improper use of revert can be exploited for Denial of Service (DoS) attacks. An attacker can intentionally trigger these 'revert' conditions, causing legitimate transactions to consistently fail. For example, if a function relies on specific conditions from user input or contract state, an attacker could manipulate these to continually force reverts, blocking the function's execution. Therefore, it's crucial to design contract logic to handle exceptions properly and avoid scenarios where revert can be predictably triggered by malicious actors. This includes careful input validation and considering alternative design patterns that are less susceptible to such abuses.
Num of instances: 4
Click to show findings
['56']
56: function govSetConfig(address base, address quote, address oracle) external onlyGovernor {
57:
58: if (base == quote) revert Errors.PriceOracle_InvalidConfiguration(); // <= FOUND
59: (address asset0, address asset1) = _sort(base, quote);
60: oracles[asset0][asset1] = oracle;
61: emit ConfigSet(asset0, asset1, oracle);
62: }['78']
78: function updatePrice(uint48 timestamp) external {
79:
80: Cache memory _cache = cache;
81: if (timestamp <= _cache.priceTimestamp) return;
82:
83: if (block.timestamp > timestamp) {
84:
85: uint256 staleness = block.timestamp - timestamp;
86: if (staleness > maxStaleness) {
87: revert Errors.PriceOracle_TooStale(staleness, maxStaleness); // <= FOUND
88: }
89: } else if (timestamp - block.timestamp > RedstoneDefaultsLib.DEFAULT_MAX_DATA_TIMESTAMP_AHEAD_SECONDS) {
90:
91: revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND
92: }
93:
94:
95: cache = Cache({price: _cache.price, priceTimestamp: timestamp});
96:
97:
98: uint256 price = getOracleNumericValueFromTxMsg(feedId);
99: if (price == 0) revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND
100: if (price > type(uint208).max) revert Errors.PriceOracle_Overflow(); // <= FOUND
101: cache = Cache({price: uint208(price), priceTimestamp: timestamp});
102: emit CacheUpdated(price, timestamp);
103: }['123']
123: function resolveOracle(uint256 inAmount, address base, address quote)
124: public
125: view
126: returns (uint256, address, address, address )
127: {
128:
129: if (base == quote) return (inAmount, base, quote, address(0));
130:
131: address oracle = getConfiguredOracle(base, quote);
132: if (oracle != address(0)) return (inAmount, base, quote, oracle);
133:
134: address baseAsset = resolvedVaults[base];
135: if (baseAsset != address(0)) {
136: inAmount = IERC4626(base).convertToAssets(inAmount);
137: return resolveOracle(inAmount, baseAsset, quote);
138: }
139:
140: oracle = fallbackOracle;
141: if (oracle == address(0)) revert Errors.PriceOracle_NotSupported(base, quote); // <= FOUND
142: return (inAmount, base, quote, oracle);
143: }['108']
108: function validateTimestamp(uint256 timestampMillis) public view virtual override {
109: uint256 timestamp = timestampMillis / 1000;
110: if (timestamp != cache.priceTimestamp) revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND
111: }[NonCritical-8] Private and internal state variables should have a preceding _ in their name unless they are constants
Add a preceding underscore to the state variable name, take care to refactor where there variables are read/wrote
Num of instances: 3
Click to show findings
['41']
41: Scale internal immutable scale; // <= FOUND['50']
50: uint8 internal immutable baseDecimals; // <= FOUND['52']
52: uint8 internal immutable quoteDecimals; // <= FOUNDConsider spreading these lines over multiple lines to aid in readability and the support of VIM users everywhere.
Num of instances: 4
Click to show findings
['5']
5: /// @author chronicleprotocol (https://github.com/chronicleprotocol/chronicle-std/blob/ea9afe78a1d33245afcdbcc3f530ee9cbd7cde28/src/IChronicle.sol) // <= FOUND['5']
5: /// @author smartcontractkit (https://github.com/smartcontractkit/chainlink/blob/e87b83cd78595c09061c199916c4bb9145e719b7/contracts/src/v0.8/shared/interfaces/AggregatorV3Interface.sol) // <= FOUND['5']
5: /// @author Lido (https://github.com/lidofinance/lido-dao/blob/5fcedc6e9a9f3ec154e69cff47c2b9e25503a78a/contracts/0.6.12/interfaces/IStETH.sol) // <= FOUND['13']
13: /// @notice One-sided price: How much quote token you would get for inAmount of base token, assuming no price spread. // <= FOUNDUsing outdated Solidity versions can lead to security risks and inefficiencies. It's recommended to adopt newer versions, ideally the latest, which as of now is 0.8.24. This ensures access to the latest bug fixes, features, and optimizations, particularly crucial for Layer 2 deployments. Regular updates to versions like 0.8.19 or later, up to 0.8.24, enhance contract security and performance.
Num of instances: 3
Click to show findings
['2']
2: pragma solidity ^0.8.0; // <= FOUND['2']
2: pragma solidity >=0.8.0; // <= FOUND['2']
2: pragma solidity ^0.8.16;Try to index as much as three variables in event declarations as this is more gas efficient when done on value type variables (uint, address etc) however not for bytes and string variables
Num of instances: 1
Click to show findings
['49']
49: event CacheUpdated(uint256 price, uint256 priceTimestamp); // <= FOUNDContracts should expose all public and external functions through interfaces. This practice ensures a clear and consistent definition of how the contract can be interacted with, promoting better transparency and integration.
Num of instances: 11
Click to show findings
['84']
84: function getQuote(uint256 inAmount, address base, address quote) external view returns (uint256)['92']
92: function getQuotes(uint256 inAmount, address base, address quote) external view returns (uint256, uint256)['56']
56: function govSetConfig(address base, address quote, address oracle) external onlyGovernor['69']
69: function govSetResolvedVault(address vault, bool set) external onlyGovernor['78']
78: function govSetFallbackOracle(address _fallbackOracle) external onlyGovernor['84']
84: function getQuote(uint256 inAmount, address base, address quote) external view returns (uint256)['92']
92: function getQuotes(uint256 inAmount, address base, address quote) external view returns (uint256, uint256)['26']
26: function transferGovernance(address newGovernor) external onlyGovernor['78']
78: function updatePrice(uint48 timestamp) external['103']
103: function getConfiguredOracle(address base, address quote) public view returns (address)['123']
123: function resolveOracle(uint256 inAmount, address base, address quote)
124: public
125: view
126: returns (uint256, address, address, address )
127:Putting constants on the left side of a comparison operator like == or < is a best practice known as "Yoda conditions", which can help prevent accidental assignment instead of comparison. In some programming languages, if a variable is mistakenly put on the left with a single = instead of ==, it assigns the constant's value to the variable without any compiler error. However, doing this with the constant on the left would generate an error, as constants cannot be assigned values. Although Solidity's static typing system prevents accidental assignments within conditionals, adopting this practice enhances code readability and consistency, especially when developers are working across multiple languages that support this convention.
Num of instances: 2
Click to show findings
['129']
129: if (p.price <= 0 || p.conf > uint64(p.price) * maxConfWidth / BASIS_POINTS) // <= FOUND['103']
103: if (feedExponent > 0) // <= FOUNDModify such instances to include a capital I as the first character in the name to signify it is an interface. This improved readability during in
Num of instances: 1
Magic numbers should be avoided in Solidity code to enhance readability, maintainability, and reduce the likelihood of errors. Magic numbers are hard-coded values with no clear meaning or context, which can create confusion and make the code harder to understand for developers. Using well-defined constants or variables with descriptive names instead of magic numbers not only clarifies the purpose and significance of the value but also simplifies code updates and modifications.
Num of instances: 4
Click to show findings
['34']
34: return success && data.length == 32 ? abi.decode(data, (uint8)) : 18; // <= FOUND['109']
109: uint256 timestamp = timestampMillis / 1000; // <= FOUND['29']
29: return Scale.wrap((10 ** feedExponent << 128) | 10 ** priceExponent); // <= FOUND['69']
69: uint256 feedScale = Scale.unwrap(scale) >> 128; // <= FOUNDA large number such as 1000000 is far more readable as 1e6, this will help prevent unintended bugs in the code
Num of instances: 1
Num of instances: 2
Click to show findings
['52']
52: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) { // <= FOUND
53: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
54:
55: if (inverse) {
56: inAmount = IPriceOracle(oracleCrossQuote).getQuote(inAmount, quote, cross);
57: return IPriceOracle(oracleBaseCross).getQuote(inAmount, cross, base);
58: } else {
59: inAmount = IPriceOracle(oracleBaseCross).getQuote(inAmount, base, cross);
60: return IPriceOracle(oracleCrossQuote).getQuote(inAmount, cross, quote);
61: }
62: }[]
63: function calcOutAmount(uint256 inAmount, uint256 unitPrice, Scale scale, bool inverse)
64: internal
65: pure
66: returns (uint256)
67: {
68: uint256 priceScale = Scale.unwrap(scale) & PRICE_SCALE_MASK;
69: uint256 feedScale = Scale.unwrap(scale) >> 128;
70: if (inverse) {
71:
72: return FixedPointMathLib.fullMulDiv(inAmount, feedScale, priceScale * unitPrice);
73: } else {
74:
75: return FixedPointMathLib.fullMulDiv(inAmount, priceScale * unitPrice, feedScale);
76: }
77: }Cyclomatic complexity is a software metric used to measure the complexity of a program. It quantifies the number of linearly independent paths through a program's source code, giving an idea of how complex the control flow is. High cyclomatic complexity may indicate a higher risk of defects and can make the code harder to understand, test, and maintain. It often suggests that a function or method is trying to do too much, and a refactor might be needed. By breaking down complex functions into smaller, more focused pieces, you can improve readability, ease of testing, and overall maintainability.
Num of instances: 2
Click to show findings
['115']
115: function _fetchPriceStruct() internal view returns (PythStructs.Price memory) { // <= FOUND
116: PythStructs.Price memory p = IPyth(pyth).getPriceUnsafe(feedId);
117:
118: if (p.publishTime < block.timestamp) {
119:
120: uint256 staleness = block.timestamp - p.publishTime;
121: if (staleness > maxStaleness) revert Errors.PriceOracle_InvalidAnswer();
122: } else {
123:
124: uint256 aheadness = p.publishTime - block.timestamp;
125: if (aheadness > MAX_AHEADNESS) revert Errors.PriceOracle_InvalidAnswer();
126: }
127:
128:
129: if (p.price <= 0 || p.conf > uint64(p.price) * maxConfWidth / BASIS_POINTS) {
130: revert Errors.PriceOracle_InvalidAnswer();
131: }
132:
133:
134: if (p.expo < MIN_EXPONENT || p.expo > MAX_EXPONENT) {
135: revert Errors.PriceOracle_InvalidAnswer();
136: }
137: return p;
138: }['78']
78: function updatePrice(uint48 timestamp) external { // <= FOUND
79:
80: Cache memory _cache = cache;
81: if (timestamp <= _cache.priceTimestamp) return;
82:
83: if (block.timestamp > timestamp) {
84:
85: uint256 staleness = block.timestamp - timestamp;
86: if (staleness > maxStaleness) {
87: revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
88: }
89: } else if (timestamp - block.timestamp > RedstoneDefaultsLib.DEFAULT_MAX_DATA_TIMESTAMP_AHEAD_SECONDS) {
90:
91: revert Errors.PriceOracle_InvalidAnswer();
92: }
93:
94:
95: cache = Cache({price: _cache.price, priceTimestamp: timestamp});
96:
97:
98: uint256 price = getOracleNumericValueFromTxMsg(feedId);
99: if (price == 0) revert Errors.PriceOracle_InvalidAnswer();
100: if (price > type(uint208).max) revert Errors.PriceOracle_Overflow();
101: cache = Cache({price: uint208(price), priceTimestamp: timestamp});
102: emit CacheUpdated(price, timestamp);
103: }Contracts should define structs before modifiers and modifiers before functions.
Num of instances: 1
[NonCritical-20] Events may be emitted out of order due to code not follow the best practice of check-effects-interaction
The "check-effects-interaction" pattern also impacts event ordering. When a contract doesn't adhere to this pattern, events might be emitted in a sequence that doesn't reflect the actual logical flow of operations. This can cause confusion during event tracking, potentially leading to erroneous off-chain interpretations. To rectify this, always ensure that checks are performed first, state modifications come next, and interactions with external contracts or addresses are done last. This will ensure events are emitted in a logical, consistent manner, providing a clear and accurate chronological record of on-chain actions for off-chain systems and observers.
Num of instances: 1
Click to show findings
['69']
69: function govSetResolvedVault(address vault, bool set) external onlyGovernor { // <= FOUND
70: address asset = set ? IERC4626(vault).asset() : address(0); // <= FOUND
71: resolvedVaults[vault] = asset;
72: emit ResolvedVaultSet(vault, asset); // <= FOUND
73: }Function parameters in Solidity are passed by value, meaning they are essentially local copies. Mutating them can lead to confusion and errors because the changes don't persist outside the function. By keeping function parameters immutable, you ensure clarity in code behavior, preventing unintended side-effects. If you need to modify a value based on a parameter, use a local variable inside the function, leaving the original parameter unaltered. By adhering to this practice, you maintain a clear distinction between input data and the internal processing logic, improving code readability and reducing the potential for bugs.
Num of instances: 2
Click to show findings
['52']
52: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) { // <= FOUND
53: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
54:
55: if (inverse) {
56: inAmount = IPriceOracle(oracleCrossQuote).getQuote(inAmount, quote, cross); // <= FOUND
57: return IPriceOracle(oracleBaseCross).getQuote(inAmount, cross, base);
58: } else {
59: inAmount = IPriceOracle(oracleBaseCross).getQuote(inAmount, base, cross); // <= FOUND
60: return IPriceOracle(oracleCrossQuote).getQuote(inAmount, cross, quote);
61: }
62: }['123']
123: function resolveOracle(uint256 inAmount, address base, address quote)
124: public
125: view
126: returns (uint256, address, address, address )
127: {
128:
129: if (base == quote) return (inAmount, base, quote, address(0));
130:
131: address oracle = getConfiguredOracle(base, quote);
132: if (oracle != address(0)) return (inAmount, base, quote, oracle);
133:
134: address baseAsset = resolvedVaults[base];
135: if (baseAsset != address(0)) {
136: inAmount = IERC4626(base).convertToAssets(inAmount); // <= FOUND
137: return resolveOracle(inAmount, baseAsset, quote);
138: }
139:
140: oracle = fallbackOracle;
141: if (oracle == address(0)) revert Errors.PriceOracle_NotSupported(base, quote);
142: return (inAmount, base, quote, oracle);
143: }The definition of a constant or immutable variable is that they do not change, casting such variables can potentially push more than one 'value' to a constant, for example a uin128 constant can have its original value and that of when it's casted to uint64 (i.e it has some bytes truncated). This can create confusion and inconsistencies within the code which can inadvertently increase the attack surface of the project. It is thus advise to either change the uint byte size in the constant/immutable definition of the variable or introduce a second state variable to cover the differing casts that are expected such as having a uint128 constant and a separate uint64 constant.
Num of instances: 1
Click to show findings
['100']
100: int8 feedExponent = int8(baseDecimals) - int8(priceStruct.expo); // <= FOUNDInconsistent use of floating pragma declarations, like ^ for a specific version or >=/<= for a range, can lead to version ambiguities and potential compatibility issues. Consistency in specifying compiler versions is crucial for ensuring predictable behavior across different environments. Using one consistent method, either fixed or ranged, throughout the project is recommended for clarity and safety.
Num of instances: 3
Click to show findings
['2']
2: pragma solidity ^0.8.0; // <= FOUND['2']
2: pragma solidity ^0.8.16; // <= FOUND['2']
2: pragma solidity >=0.8.0; // <= FOUND[NonCritical-24] Use 'using' keyword when using specific imports rather than calling the specific import directly
In Solidity, the using keyword can streamline the use of library functions for specific types. Instead of calling library functions directly with their full import paths, you can declare a library once with using for a specific type. This approach makes your code more readable and concise. For example, instead of LibraryName.functionName(variable), you would first declare using LibraryName for TypeName; at the contract level. After this, you can call library functions directly on variables of TypeName like variable.functionName(). This method not only enhances code clarity but also promotes cleaner and more organized code, especially when multiple functions from the same library are used frequently.
Num of instances: 34
Click to show findings
['89']
89: } else if (timestamp - block.timestamp > RedstoneDefaultsLib.DEFAULT_MAX_DATA_TIMESTAMP_AHEAD_SECONDS) { // <= FOUND 'RedstoneDefaultsLib.'['72']
72:
73: return FixedPointMathLib.fullMulDiv(inAmount, feedScale, priceScale * unitPrice); // <= FOUND 'FixedPointMathLib.'['75']
75:
76: return FixedPointMathLib.fullMulDiv(inAmount, priceScale * unitPrice, feedScale); // <= FOUND 'FixedPointMathLib.'['76']
76: return OracleLibrary.getQuoteAtTick(tick, uint128(inAmount), base, quote); // <= FOUND 'OracleLibrary.'['46']
46: revert Errors.PriceOracle_InvalidConfiguration(); // <= FOUND 'Errors.'['67']
67: if (answer <= 0) revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND 'Errors.'['67']
67: if (staleness > maxStaleness) revert Errors.PriceOracle_TooStale(staleness, maxStaleness); // <= FOUND 'Errors.'['48']
48: if (_governor == address(0)) revert Errors.PriceOracle_InvalidConfiguration(); // <= FOUND 'Errors.'['58']
58:
59: if (base == quote) revert Errors.PriceOracle_InvalidConfiguration(); // <= FOUND 'Errors.'['141']
141: if (oracle == address(0)) revert Errors.PriceOracle_NotSupported(base, quote); // <= FOUND 'Errors.'['33']
33: revert Errors.Governance_CallerNotGovernor(); // <= FOUND 'Errors.'['63']
63: revert Errors.PriceOracle_NotSupported(base, quote); // <= FOUND 'Errors.'['121']
121: if (staleness > maxStaleness) revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND 'Errors.'['125']
125: if (aheadness > MAX_AHEADNESS) revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND 'Errors.'['91']
91: revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND 'Errors.'['60']
60: if (_maxStaleness > MAX_STALENESS_UPPER_BOUND) revert Errors.PriceOracle_InvalidConfiguration(); // <= FOUND 'Errors.'['87']
87: revert Errors.PriceOracle_TooStale(staleness, maxStaleness); // <= FOUND 'Errors.'['91']
91:
92: revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND 'Errors.'['99']
99: if (price == 0) revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND 'Errors.'['100']
100: if (price > type(uint208).max) revert Errors.PriceOracle_Overflow(); // <= FOUND 'Errors.'['110']
110: if (timestamp != cache.priceTimestamp) revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND 'Errors.'['126']
126: revert Errors.PriceOracle_TooStale(priceStaleness, maxStaleness); // <= FOUND 'Errors.'['27']
27: revert Errors.PriceOracle_Overflow(); // <= FOUND 'Errors.'['45']
45: revert Errors.PriceOracle_NotSupported(givenBase, givenQuote); // <= FOUND 'Errors.'['53']
53: if (pool == address(0)) revert Errors.PriceOracle_InvalidConfiguration(); // <= FOUND 'Errors.'['63']
63: revert Errors.PriceOracle_NotSupported(base, quote); // <= FOUND 'Errors.'['66']
66:
67: if (inAmount > type(uint128).max) revert Errors.PriceOracle_Overflow(); // <= FOUND 'Errors.'['53']
53: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, feedDecimals); // <= FOUND 'ScaleUtils.'['119']
119: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote); // <= FOUND 'ScaleUtils.'['69']
69: return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse); // <= FOUND 'ScaleUtils.'['104']
104: scale = ScaleUtils.from(quoteDecimals, uint8(feedExponent)); // <= FOUND 'ScaleUtils.'['106']
106: scale = ScaleUtils.from(quoteDecimals + uint8(-feedExponent), 0); // <= FOUND 'ScaleUtils.'['69']
69: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, _feedDecimals); // <= FOUND 'ScaleUtils.'['129']
129: return ScaleUtils.calcOutAmount(inAmount, _cache.price, scale, inverse); // <= FOUND 'ScaleUtils.'Only some address parameters are checked against address(0), to ensure consistency ensure all address parameters are checked.
Num of instances: 1
Click to show findings
['123']
123: function resolveOracle(uint256 inAmount, address base, address quote) // <= FOUND 'address quote'
124: public
125: view
126: returns (uint256, address, address, address )
127: {
128:
129: if (base == quote) return (inAmount, base, quote, address(0));
130:
131: address oracle = getConfiguredOracle(base, quote);
132: if (oracle != address(0)) return (inAmount, base, quote, oracle);
133:
134: address baseAsset = resolvedVaults[base]; // <= FOUND 'address base'
135: if (baseAsset != address(0)) {
136: inAmount = IERC4626(base).convertToAssets(inAmount);
137: return resolveOracle(inAmount, baseAsset, quote);
138: }
139:
140: oracle = fallbackOracle;
141: if (oracle == address(0)) revert Errors.PriceOracle_NotSupported(base, quote);
142: return (inAmount, base, quote, oracle);
143: }Emitting an event in a constructor of a smart contract provides transparency and traceability in blockchain applications. This event logs the contract’s creation, aiding in monitoring and verifying contract deployment. Although constructors are executed only once, the emitted event ensures the contract's initialization is recorded on the blockchain.
Num of instances: 8
Click to show findings
['41']
41: constructor(address _base, address _quote, address _feed, uint256 _maxStaleness) { // <= FOUND
42: if (_maxStaleness < MAX_STALENESS_LOWER_BOUND || _maxStaleness > MAX_STALENESS_UPPER_BOUND) {
43: revert Errors.PriceOracle_InvalidConfiguration();
44: }
45:
46: base = _base;
47: quote = _quote;
48: feed = _feed;
49: maxStaleness = _maxStaleness;
50:
51:
52: uint8 baseDecimals = _getDecimals(base);
53: uint8 quoteDecimals = _getDecimals(quote);
54: uint8 feedDecimals = AggregatorV3Interface(feed).decimals();
55: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, feedDecimals);
56: }['39']
39: constructor(address _base, address _quote, address _feed, uint256 _maxStaleness) { // <= FOUND
40: if (_maxStaleness < MAX_STALENESS_LOWER_BOUND || _maxStaleness > MAX_STALENESS_UPPER_BOUND) {
41: revert Errors.PriceOracle_InvalidConfiguration();
42: }
43:
44: base = _base;
45: quote = _quote;
46: feed = _feed;
47: maxStaleness = _maxStaleness;
48:
49:
50: uint8 baseDecimals = _getDecimals(base);
51: uint8 quoteDecimals = _getDecimals(quote);
52: uint8 feedDecimals = IChronicle(feed).decimals();
53: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, feedDecimals);
54: }['37']
37: constructor(address _base, address _cross, address _quote, address _oracleBaseCross, address _oracleCrossQuote) { // <= FOUND
38: base = _base;
39: cross = _cross;
40: quote = _quote;
41: oracleBaseCross = _oracleBaseCross;
42: oracleCrossQuote = _oracleCrossQuote;
43: }['47']
47: constructor(address _governor) Governable(_governor) { // <= FOUND
48: if (_governor == address(0)) revert Errors.PriceOracle_InvalidConfiguration();
49: }['19']
19: constructor(address _governor) { // <= FOUND
20: _setGovernor(_governor);
21: }['64']
64: constructor( // <= FOUND
65: address _pyth,
66: address _base,
67: address _quote,
68: bytes32 _feedId,
69: uint256 _maxStaleness,
70: uint256 _maxConfWidth
71: ) {
72: if (_maxStaleness > MAX_STALENESS_UPPER_BOUND) {
73: revert Errors.PriceOracle_InvalidConfiguration();
74: }
75: if (_maxConfWidth < MAX_CONF_WIDTH_LOWER_BOUND || _maxConfWidth > MAX_CONF_WIDTH_UPPER_BOUND) {
76: revert Errors.PriceOracle_InvalidConfiguration();
77: }
78:
79: pyth = _pyth;
80: base = _base;
81: quote = _quote;
82: feedId = _feedId;
83: maxStaleness = _maxStaleness;
84: maxConfWidth = _maxConfWidth;
85: baseDecimals = _getDecimals(base);
86: quoteDecimals = _getDecimals(quote);
87: }['59']
59: constructor(address _base, address _quote, bytes32 _feedId, uint8 _feedDecimals, uint256 _maxStaleness) { // <= FOUND
60: if (_maxStaleness > MAX_STALENESS_UPPER_BOUND) revert Errors.PriceOracle_InvalidConfiguration();
61:
62: base = _base;
63: quote = _quote;
64: feedId = _feedId;
65: feedDecimals = _feedDecimals;
66: maxStaleness = _maxStaleness;
67: uint8 baseDecimals = _getDecimals(base);
68: uint8 quoteDecimals = _getDecimals(quote);
69: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, _feedDecimals);
70: }['44']
44: constructor(address _tokenA, address _tokenB, uint24 _fee, uint32 _twapWindow, address _uniswapV3Factory) { // <= FOUND
45: if (_twapWindow < MIN_TWAP_WINDOW || _twapWindow > uint32(type(int32).max)) {
46: revert Errors.PriceOracle_InvalidConfiguration();
47: }
48: tokenA = _tokenA;
49: tokenB = _tokenB;
50: fee = _fee;
51: twapWindow = _twapWindow;
52: pool = IUniswapV3Factory(_uniswapV3Factory).getPool(tokenA, tokenB, _fee);
53: if (pool == address(0)) revert Errors.PriceOracle_InvalidConfiguration();
54: }Failing to validate for empty array inputs in constructors can lead to vulnerabilities or logical errors in smart contracts. Constructors often initialize key contract settings, and arrays may represent crucial data like access controls, initial states, or configuration parameters. Without empty array checks, a contract might be deployed in an unintended state, lacking necessary data for its operations or security measures.
Num of instances: 2
Click to show findings
['64']
64: constructor(
65: address _pyth,
66: address _base,
67: address _quote,
68: bytes32 _feedId,
69: uint256 _maxStaleness,
70: uint256 _maxConfWidth
71: ) {
72: if (_maxStaleness > MAX_STALENESS_UPPER_BOUND) {
73: revert Errors.PriceOracle_InvalidConfiguration();
74: }
75: if (_maxConfWidth < MAX_CONF_WIDTH_LOWER_BOUND || _maxConfWidth > MAX_CONF_WIDTH_UPPER_BOUND) {
76: revert Errors.PriceOracle_InvalidConfiguration();
77: }
78:
79: pyth = _pyth;
80: base = _base;
81: quote = _quote;
82: feedId = _feedId;
83: maxStaleness = _maxStaleness;
84: maxConfWidth = _maxConfWidth;
85: baseDecimals = _getDecimals(base);
86: quoteDecimals = _getDecimals(quote);
87: }['59']
59: constructor(address _base, address _quote, bytes32 _feedId, uint8 _feedDecimals, uint256 _maxStaleness) {
60: if (_maxStaleness > MAX_STALENESS_UPPER_BOUND) revert Errors.PriceOracle_InvalidConfiguration();
61:
62: base = _base;
63: quote = _quote;
64: feedId = _feedId;
65: feedDecimals = _feedDecimals;
66: maxStaleness = _maxStaleness;
67: uint8 baseDecimals = _getDecimals(base);
68: uint8 quoteDecimals = _getDecimals(quote);
69: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, _feedDecimals);
70: }In Solidity, custom errors with parameters offer a gas-efficient way to convey detailed information about issues encountered during contract execution. Unlike revert messages, which are strings consuming more gas, custom errors defined with parameters allow developers to specify types and details of errors succinctly. This method enhances debugging, provides clearer insights into contract failures, and improves the developer's and end-user's understanding of what went wrong, all while optimizing for gas usage and maintaining contract efficiency.
Num of instances: 4
Click to show findings
['10']
10:
11: error PriceOracle_InvalidAnswer(); // <= FOUND['12']
12:
13: error PriceOracle_InvalidConfiguration(); // <= FOUND['18']
18:
19: error PriceOracle_Overflow(); // <= FOUND['24']
24:
25: error Governance_CallerNotGovernor(); // <= FOUNDRather than redefining state variable constant, consider utilising a library to store all constants as this will prevent data redundancy
Num of instances: 2
Click to show findings
['18']
18: uint256 internal constant MAX_STALENESS_LOWER_BOUND = 1 minutes; // <= FOUND['20']
20: uint256 internal constant MAX_STALENESS_UPPER_BOUND = 72 hours; // <= FOUNDNum of instances: 20
Click to show findings
['14']
14: function getQuote(uint256 inAmount, address base, address quote) external view returns (uint256) {
15: return _getQuote(inAmount, base, quote); // <= FOUND
16: }['20']
20: function getQuotes(uint256 inAmount, address base, address quote) external view returns (uint256, uint256) {
21: uint256 outAmount = _getQuote(inAmount, base, quote);
22: return (outAmount, outAmount); // <= FOUND
23: }['32']
32: function _getDecimals(address asset) internal view returns (uint8) {
33: (bool success, bytes memory data) = asset.staticcall(abi.encodeCall(IERC20.decimals, ()));
34: return success && data.length == 32 ? abi.decode(data, (uint8)) : 18; // <= FOUND
35: }['63']
63: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
64: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
65:
66: (, int256 answer,, uint256 updatedAt,) = AggregatorV3Interface(feed).latestRoundData();
67: if (answer <= 0) revert Errors.PriceOracle_InvalidAnswer();
68: uint256 staleness = block.timestamp - updatedAt;
69: if (staleness > maxStaleness) revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
70:
71: uint256 price = uint256(answer);
72: return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse); // <= FOUND
73: }['61']
61: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
62: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
63:
64:
65: (uint256 price, uint256 age) = IChronicle(feed).readWithAge();
66: uint256 staleness = block.timestamp - age;
67: if (staleness > maxStaleness) revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
68:
69: return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse); // <= FOUND
70: }['52']
52: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
53: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
54:
55: if (inverse) {
56: inAmount = IPriceOracle(oracleCrossQuote).getQuote(inAmount, quote, cross);
57: return IPriceOracle(oracleBaseCross).getQuote(inAmount, cross, base); // <= FOUND
58: } else {
59: inAmount = IPriceOracle(oracleBaseCross).getQuote(inAmount, base, cross);
60: return IPriceOracle(oracleCrossQuote).getQuote(inAmount, cross, quote); // <= FOUND
61: }
62: }['84']
84: function getQuote(uint256 inAmount, address base, address quote) external view returns (uint256) {
85: address oracle;
86: (inAmount, base, quote, oracle) = resolveOracle(inAmount, base, quote);
87: if (base == quote) return inAmount; // <= FOUND
88: return IPriceOracle(oracle).getQuote(inAmount, base, quote); // <= FOUND
89: }['92']
92: function getQuotes(uint256 inAmount, address base, address quote) external view returns (uint256, uint256) {
93: address oracle;
94: (inAmount, base, quote, oracle) = resolveOracle(inAmount, base, quote);
95: if (base == quote) return (inAmount, inAmount); // <= FOUND
96: return IPriceOracle(oracle).getQuotes(inAmount, base, quote); // <= FOUND
97: }['103']
103: function getConfiguredOracle(address base, address quote) public view returns (address) {
104: (address asset0, address asset1) = _sort(base, quote);
105: return oracles[asset0][asset1]; // <= FOUND
106: }['123']
123: function resolveOracle(uint256 inAmount, address base, address quote)
124: public
125: view
126: returns (uint256, address, address, address )
127: {
128:
129: if (base == quote) return (inAmount, base, quote, address(0)); // <= FOUND
130:
131: address oracle = getConfiguredOracle(base, quote);
132: if (oracle != address(0)) return (inAmount, base, quote, oracle); // <= FOUND
133:
134: address baseAsset = resolvedVaults[base];
135: if (baseAsset != address(0)) {
136: inAmount = IERC4626(base).convertToAssets(inAmount);
137: return resolveOracle(inAmount, baseAsset, quote); // <= FOUND
138: }
139:
140: oracle = fallbackOracle;
141: if (oracle == address(0)) revert Errors.PriceOracle_NotSupported(base, quote);
142: return (inAmount, base, quote, oracle); // <= FOUND
143: }['27']
27: function _getQuote(uint256 inAmount, address base, address quote) internal view override returns (uint256) {
28: if (base == STETH && quote == WSTETH) {
29: return IStEth(STETH).getSharesByPooledEth(inAmount); // <= FOUND
30: } else if (base == WSTETH && quote == STETH) {
31: return IStEth(STETH).getPooledEthByShares(inAmount); // <= FOUND
32: }
33: revert Errors.PriceOracle_NotSupported(base, quote);
34: }['94']
94: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
95: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
96:
97: PythStructs.Price memory priceStruct = _fetchPriceStruct();
98:
99: uint256 price = uint256(uint64(priceStruct.price));
100: int8 feedExponent = int8(baseDecimals) - int8(priceStruct.expo);
101:
102: Scale scale;
103: if (feedExponent > 0) {
104: scale = ScaleUtils.from(quoteDecimals, uint8(feedExponent));
105: } else {
106: scale = ScaleUtils.from(quoteDecimals + uint8(-feedExponent), 0);
107: }
108: return ScaleUtils.calcOutAmount(inAmount, price, scale, inverse); // <= FOUND
109: }['115']
115: function _fetchPriceStruct() internal view returns (PythStructs.Price memory) {
116: PythStructs.Price memory p = IPyth(pyth).getPriceUnsafe(feedId);
117:
118: if (p.publishTime < block.timestamp) {
119:
120: uint256 staleness = block.timestamp - p.publishTime;
121: if (staleness > maxStaleness) revert Errors.PriceOracle_InvalidAnswer();
122: } else {
123:
124: uint256 aheadness = p.publishTime - block.timestamp;
125: if (aheadness > MAX_AHEADNESS) revert Errors.PriceOracle_InvalidAnswer();
126: }
127:
128:
129: if (p.price <= 0 || p.conf > uint64(p.price) * maxConfWidth / BASIS_POINTS) {
130: revert Errors.PriceOracle_InvalidAnswer();
131: }
132:
133:
134: if (p.expo < MIN_EXPONENT || p.expo > MAX_EXPONENT) {
135: revert Errors.PriceOracle_InvalidAnswer();
136: }
137: return p; // <= FOUND
138: }['118']
118: function _getQuote(uint256 inAmount, address _base, address _quote) internal view override returns (uint256) {
119: bool inverse = ScaleUtils.getDirectionOrRevert(_base, base, _quote, quote);
120:
121: Cache memory _cache = cache;
122: if (block.timestamp > _cache.priceTimestamp) {
123:
124: uint256 priceStaleness = block.timestamp - _cache.priceTimestamp;
125: if (priceStaleness > maxStaleness) {
126: revert Errors.PriceOracle_TooStale(priceStaleness, maxStaleness);
127: }
128: }
129: return ScaleUtils.calcOutAmount(inAmount, _cache.price, scale, inverse); // <= FOUND
130: }['61']
61: function _getQuote(uint256 inAmount, address base, address quote) internal view override returns (uint256) {
62: if (!((base == tokenA && quote == tokenB) || (base == tokenB && quote == tokenA))) {
63: revert Errors.PriceOracle_NotSupported(base, quote);
64: }
65:
66: if (inAmount > type(uint128).max) revert Errors.PriceOracle_Overflow();
67:
68: uint32[] memory secondsAgos = new uint32[](2);
69: secondsAgos[0] = twapWindow;
70:
71:
72: (int56[] memory tickCumulatives,) = IUniswapV3Pool(pool).observe(secondsAgos);
73: int56 tickCumulativesDelta = tickCumulatives[1] - tickCumulatives[0];
74: int24 tick = int24(tickCumulativesDelta / int56(uint56(twapWindow)));
75: if (tickCumulativesDelta < 0 && (tickCumulativesDelta % int56(uint56(twapWindow)) != 0)) tick--;
76: return OracleLibrary.getQuoteAtTick(tick, uint128(inAmount), base, quote); // <= FOUND
77: }['150']
150: function _sort(address assetA, address assetB) internal pure returns (address, address) {
151: return assetA < assetB ? (assetA, assetB) : (assetB, assetA); // <= FOUND
152: }['25']
25: function from(uint8 priceExponent, uint8 feedExponent) internal pure returns (Scale) {
26: if (priceExponent > MAX_EXPONENT || feedExponent > MAX_EXPONENT) {
27: revert Errors.PriceOracle_Overflow();
28: }
29: return Scale.wrap((10 ** feedExponent << 128) | 10 ** priceExponent); // <= FOUND
30: }['38']
38: function getDirectionOrRevert(address givenBase, address base, address givenQuote, address quote)
39: internal
40: pure
41: returns (bool)
42: {
43: if (givenBase == base && givenQuote == quote) return false; // <= FOUND
44: if (givenBase == quote && givenQuote == base) return true; // <= FOUND
45: revert Errors.PriceOracle_NotSupported(givenBase, givenQuote);
46: }['53']
53: function calcScale(uint8 baseDecimals, uint8 quoteDecimals, uint8 feedDecimals) internal pure returns (Scale) {
54: return from(quoteDecimals, feedDecimals + baseDecimals); // <= FOUND
55: }['63']
63: function calcOutAmount(uint256 inAmount, uint256 unitPrice, Scale scale, bool inverse)
64: internal
65: pure
66: returns (uint256)
67: {
68: uint256 priceScale = Scale.unwrap(scale) & PRICE_SCALE_MASK;
69: uint256 feedScale = Scale.unwrap(scale) >> 128;
70: if (inverse) {
71:
72: return FixedPointMathLib.fullMulDiv(inAmount, feedScale, priceScale * unitPrice); // <= FOUND
73: } else {
74:
75: return FixedPointMathLib.fullMulDiv(inAmount, priceScale * unitPrice, feedScale); // <= FOUND
76: }
77: }Caching immutable state variables results in storing data which is already known at compile time. As such, developers should call immutable state variables directly to save computation and storage which will result in gas savings.
Num of instances: 1
Click to show findings
['61']
61: function _getQuote(uint256 inAmount, address base, address quote) internal view override returns (uint256) { // <= FOUND
62: if (!((base == tokenA && quote == tokenB) || (base == tokenB && quote == tokenA))) {
63: revert Errors.PriceOracle_NotSupported(base, quote);
64: }
65:
66: if (inAmount > type(uint128).max) revert Errors.PriceOracle_Overflow();
67:
68: uint32[] memory secondsAgos = new uint32[](2); // <= FOUND
69: secondsAgos[0] = twapWindow; // <= FOUND
70:
71:
72: (int56[] memory tickCumulatives,) = IUniswapV3Pool(pool).observe(secondsAgos);
73: int56 tickCumulativesDelta = tickCumulatives[1] - tickCumulatives[0]; // <= FOUND
74: int24 tick = int24(tickCumulativesDelta / int56(uint56(twapWindow))); // <= FOUND
75: if (tickCumulativesDelta < 0 && (tickCumulativesDelta % int56(uint56(twapWindow)) != 0)) tick--; // <= FOUND
76: return OracleLibrary.getQuoteAtTick(tick, uint128(inAmount), base, quote); // <= FOUND
77: }When using a smaller int/uint type it first needs to be converted to it's 258 bit counterpart to be operated, this increases the gass cost and thus should be avoided. However it does make sense to use smaller int/uint values within structs provided you pack the struct properly.
Num of instances: 28
Click to show findings
['67']
67:
68: uint8 baseDecimals = _getDecimals(base); // <= FOUND['68']
68: uint8 quoteDecimals = _getDecimals(quote); // <= FOUND['54']
54: uint8 feedDecimals = AggregatorV3Interface(feed).decimals(); // <= FOUND['52']
52: uint8 feedDecimals = IChronicle(feed).decimals(); // <= FOUND['50']
50:
51: uint8 internal immutable baseDecimals; // <= FOUND['52']
52:
53: uint8 internal immutable quoteDecimals; // <= FOUND['100']
100: int8 feedExponent = int8(baseDecimals) - int8(priceStruct.expo); // <= FOUND['36']
36:
38: uint8 public immutable feedDecimals; // <= FOUND['59']
59:
67: constructor(address _base, address _quote, bytes32 _feedId, uint8 _feedDecimals, uint256 _maxStaleness) { // <= FOUND['67']
67: uint8 baseDecimals = _getDecimals(base); // <= FOUND['25']
25:
31: function from(uint8 priceExponent, uint8 feedExponent) internal pure returns (Scale) { // <= FOUND['53']
53:
58: function calcScale(uint8 baseDecimals, uint8 quoteDecimals, uint8 feedDecimals) internal pure returns (Scale) { // <= FOUND['50']
50: uint8 internal immutable baseDecimals; // <= FOUND['52']
52: uint8 internal immutable quoteDecimals; // <= FOUND['36']
36: uint8 public immutable feedDecimals; // <= FOUND['31']
31:
32: uint24 public immutable fee; // <= FOUND['44']
44:
51: constructor(address _tokenA, address _tokenB, uint24 _fee, uint32 _twapWindow, address _uniswapV3Factory) { // <= FOUND['74']
74: int24 tick = int24(tickCumulativesDelta / int56(uint56(twapWindow))); // <= FOUND['31']
31: uint24 public immutable fee; // <= FOUND['23']
23:
24: uint32 internal constant MIN_TWAP_WINDOW = 5 minutes; // <= FOUND['33']
33:
34: uint32 public immutable twapWindow; // <= FOUND['23']
23: uint32 internal constant MIN_TWAP_WINDOW = 5 minutes; // <= FOUND['33']
33: uint32 public immutable twapWindow; // <= FOUND['20']
20:
21: uint48 priceTimestamp; // <= FOUND['78']
78:
84: function updatePrice(uint48 timestamp) external { // <= FOUND['73']
73: int56 tickCumulativesDelta = tickCumulatives[1] - tickCumulatives[0]; // <= FOUND['19']
19:
26: function latestRoundData()
27: external
28: view
29: returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); // <= FOUND['18']
18:
19: uint208 price; // <= FOUNDReplace spotted instances with != 0 for uints as this uses less gas
Num of instances: 2
Click to show findings
['103']
103: if (feedExponent > 0) { // <= FOUND[]
}
pragma solidity >=0.8.0; // <= FOUNDUsing assembly for address comparisons in Solidity can save gas because it allows for more direct access to the Ethereum Virtual Machine (EVM), reducing the overhead of higher-level operations. Solidity's high-level abstraction simplifies coding but can introduce additional gas costs. Using assembly for simple operations like address comparisons can be more gas-efficient.
Num of instances: 1
Click to show findings
['123']
123: function resolveOracle(uint256 inAmount, address base, address quote)
124: public
125: view
126: returns (uint256, address, address, address )
127: {
128:
129: if (base == quote) return (inAmount, base, quote, address(0));
130:
131: address oracle = getConfiguredOracle(base, quote);
132: if (oracle != address(0)) return (inAmount, base, quote, oracle); // <= FOUND
133:
134: address baseAsset = resolvedVaults[base];
135: if (baseAsset != address(0)) { // <= FOUND
136: inAmount = IERC4626(base).convertToAssets(inAmount);
137: return resolveOracle(inAmount, baseAsset, quote);
138: }
139:
140: oracle = fallbackOracle;
141: if (oracle == address(0)) revert Errors.PriceOracle_NotSupported(base, quote); // <= FOUND
142: return (inAmount, base, quote, oracle);
143: }[Gas-6] Divisions which do not divide by -X cannot overflow or underflow so such operations can be unchecked to save gas
Make such found divisions are unchecked when ensured it is safe to do so
Num of instances: 1
Click to show findings
['108']
108: function validateTimestamp(uint256 timestampMillis) public view virtual override { // <= FOUND
109: uint256 timestamp = timestampMillis / 1000; // <= FOUND
110: if (timestamp != cache.priceTimestamp) revert Errors.PriceOracle_InvalidAnswer();
111: }The Ethereum network automatically appends certain data, including block.number and block.timestamp, to every event emitted by smart contracts. This inherent feature means that explicitly adding these parameters to your event logs isn't necessary, as they're already accessible through the transaction receipt. In fact, manually adding these details to your events can lead to wasted gas because it increases the data payload of the transaction, thereby upping the associated gas cost. For gas-efficient coding, it's best to rely on the default inclusions of block.number and block.timestamp rather than duplicating these in your events.
Num of instances: 1
Click to show findings
['49']
49: event CacheUpdated(uint256 price, uint256 priceTimestamp); // <= FOUNDNested mappings and multi-dimensional arrays in Solidity operate through a process of double hashing, wherein the original storage slot and the first key are concatenated and hashed, and then this hash is again concatenated with the second key and hashed. This process can be quite gas expensive due to the double-hashing operation and subsequent storage operation (sstore).
A possible optimization involves manually concatenating the keys followed by a single hash operation and an sstore. However, this technique introduces the risk of storage collision, especially when there are other nested hash maps in the contract that use the same key types. Because Solidity is unaware of the number and structure of nested hash maps in a contract, it follows a conservative approach in computing the storage slot to avoid possible collisions.
OpenZeppelin's EnumerableSet provides a potential solution to this problem. It creates a data structure that combines the benefits of set operations with the ability to enumerate stored elements, which is not natively available in Solidity. EnumerableSet handles the element uniqueness internally and can therefore provide a more gas-efficient and collision-resistant alternative to nested mappings or multi-dimensional arrays in certain scenarios.
Num of instances: 1
Click to show findings
['26']
26: mapping(address asset0 => mapping(address asset1 => address oracle)) internal oracles; // <= FOUNDWith the use of inline assembly in Solidity, we can take advantage of low-level features like scratch space and the free memory pointer, offering more gas-efficient ways of emitting events. The scratch space is a certain area of memory where we can temporarily store data, and the free memory pointer indicates the next available memory slot. Using these, we can efficiently assemble event data without incurring additional memory expansion costs. However, safety is paramount: to avoid overwriting or leakage, we must cache the free memory pointer before use and restore it afterward, ensuring that it points to the correct memory location post-operation.
Num of instances: 5
Click to show findings
['61']
61: emit ConfigSet(asset0, asset1, oracle); // <= FOUND['72']
72: emit ResolvedVaultSet(vault, asset); // <= FOUND['80']
80: emit FallbackOracleSet(_fallbackOracle); // <= FOUND['41']
41: emit GovernorSet(governor, newGovernor); // <= FOUND['102']
102: emit CacheUpdated(price, timestamp); // <= FOUNDUsing inline assembly to extract calldata values can be more gas-efficient than using abi.decode in Solidity. Inline assembly gives more direct access to EVM operations, enabling optimized usage of calldata. However, assembly should be used judiciously as it's more prone to errors. Opt for this approach when performance is critical and the complexity it introduces is manageable.
Num of instances: 1
Click to show findings
['34']
34: return success && data.length == 32 ? abi.decode(data, (uint8)) : 18; // <= FOUNDUsing private visibility for constants and immutables in Solidity instead of public can save gas. This is because private elements are not included in the contract's ABI, reducing the deployment and interaction costs. To achieve better efficiency, it is recommended to use private visibility when external access is not needed.
Num of instances: 10
Click to show findings
['16']
16: string public constant name = "ChainlinkOracle"; // <= FOUND['16']
16: string public constant name = "ChronicleOracle"; // <= FOUND['15']
15: string public constant name = "CrossAdapter"; // <= FOUND['18']
18: string public constant name = "EulerRouter"; // <= FOUND['13']
13: string public constant name = "LidoOracle"; // <= FOUND['16']
16: address public constant STETH = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84; // <= FOUND['19']
19: address public constant WSTETH = 0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0; // <= FOUND['34']
34: string public constant name = "PythOracle"; // <= FOUND['26']
26: string public constant name = "RedstoneCoreOracle"; // <= FOUND['25']
25: string public constant name = "UniswapV3Oracle"; // <= FOUNDUsing assembly for simple zero checks on unsigned integers can save gas due to lower-level, optimized operations.
Resolution: Implement inline assembly with Solidity's assembly block to perform zero checks. Ensure thorough testing and verification, as assembly lacks the safety checks of high-level Solidity, potentially introducing vulnerabilities if not used carefully.
Num of instances: 2
Click to show findings
['99']
99: if (price == 0) revert Errors.PriceOracle_InvalidAnswer(); // <= FOUND['67']
67: if (answer <= 0) revert Errors.PriceOracle_InvalidAnswer(); // <= FOUNDUsing nested if statements instead of logical AND (&&) operators can potentially save gas in Solidity contracts. When a series of conditions are connected with &&, all conditions must be evaluated even if the first one fails. In contrast, nested if statements allow for short-circuiting; if the first condition fails, the rest are skipped, saving gas. This approach is more gas-efficient, especially when dealing with complex or gas-intensive conditions. However, it's crucial to balance gas savings with code readability and maintainability, ensuring that the code remains clear and easy to understand.
Num of instances: 9
Click to show findings
['28']
28: if (base == STETH && quote == WSTETH) { // <= FOUND
29: return IStEth(STETH).getSharesByPooledEth(inAmount);
30: }['62']
62: if (!((base == tokenA && quote == tokenB) || (base == tokenB && quote == tokenA))) { // <= FOUND
63: revert Errors.PriceOracle_NotSupported(base, quote);
64: }['34']
34: return success && data.length == 32 ? abi.decode(data, (uint8)) : 18; // <= FOUND['28']
28: if (base == STETH && quote == WSTETH) { // <= FOUND['30']
30: } else if (base == WSTETH && quote == STETH) { // <= FOUND['43']
43: if (givenBase == base && givenQuote == quote) return false; // <= FOUND['44']
44: if (givenBase == quote && givenQuote == base) return true; // <= FOUND['62']
62: if (!((base == tokenA && quote == tokenB) || (base == tokenB && quote == tokenA))) { // <= FOUND['75']
75: if (tickCumulativesDelta < 0 && (tickCumulativesDelta % int56(uint56(twapWindow)) != 0)) tick--; // <= FOUNDWhen emitting events in Solidity, using stack variables (local variables within a function) instead of state variables can lead to significant gas savings. Stack variables reside in memory only for the duration of the function execution and are less costly to access compared to state variables, which are stored on the blockchain. When an event is emitted, accessing these stack variables requires less gas than fetching data from state variables, which involves reading from the contract's storage - a more expensive operation. Thus, for efficiency, prefer using local variables within functions for event emission, especially in functions that are called frequently.
Num of instances: 1
Solidity version 0.8.19 introduced a array of gas optimizations which make contracts which use it more efficient. Provided compatability it may be beneficial to upgrade to this version
Num of instances: 3
Click to show findings
['2']
2: pragma solidity ^0.8.0;['2']
2: pragma solidity ^0.8.16;['2']
2: pragma solidity >=0.8.0;If a internal function is only used once it doesn't make sense to modularise it unless the function which does call the function would be overly long and complex otherwise
Num of instances: 1
Click to show findings
['115']
115: function _fetchPriceStruct() internal view returns (PythStructs.Price memory) // <= FOUNDNum of instances: 8
Click to show findings
['41']
41: constructor(address _base, address _quote, address _feed, uint256 _maxStaleness) {
42: if (_maxStaleness < MAX_STALENESS_LOWER_BOUND || _maxStaleness > MAX_STALENESS_UPPER_BOUND) {
43: revert Errors.PriceOracle_InvalidConfiguration();
44: }
45:
46: base = _base;
47: quote = _quote;
48: feed = _feed;
49: maxStaleness = _maxStaleness;
50:
51:
52: uint8 baseDecimals = _getDecimals(base);
53: uint8 quoteDecimals = _getDecimals(quote);
54: uint8 feedDecimals = AggregatorV3Interface(feed).decimals();
55: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, feedDecimals);
56: }['39']
39: constructor(address _base, address _quote, address _feed, uint256 _maxStaleness) {
40: if (_maxStaleness < MAX_STALENESS_LOWER_BOUND || _maxStaleness > MAX_STALENESS_UPPER_BOUND) {
41: revert Errors.PriceOracle_InvalidConfiguration();
42: }
43:
44: base = _base;
45: quote = _quote;
46: feed = _feed;
47: maxStaleness = _maxStaleness;
48:
49:
50: uint8 baseDecimals = _getDecimals(base);
51: uint8 quoteDecimals = _getDecimals(quote);
52: uint8 feedDecimals = IChronicle(feed).decimals();
53: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, feedDecimals);
54: }['37']
37: constructor(address _base, address _cross, address _quote, address _oracleBaseCross, address _oracleCrossQuote) {
38: base = _base;
39: cross = _cross;
40: quote = _quote;
41: oracleBaseCross = _oracleBaseCross;
42: oracleCrossQuote = _oracleCrossQuote;
43: }['47']
47: constructor(address _governor) Governable(_governor) {
48: if (_governor == address(0)) revert Errors.PriceOracle_InvalidConfiguration();
49: }['19']
19: constructor(address _governor) {
20: _setGovernor(_governor);
21: }['64']
64: constructor(
65: address _pyth,
66: address _base,
67: address _quote,
68: bytes32 _feedId,
69: uint256 _maxStaleness,
70: uint256 _maxConfWidth
71: ) {
72: if (_maxStaleness > MAX_STALENESS_UPPER_BOUND) {
73: revert Errors.PriceOracle_InvalidConfiguration();
74: }
75: if (_maxConfWidth < MAX_CONF_WIDTH_LOWER_BOUND || _maxConfWidth > MAX_CONF_WIDTH_UPPER_BOUND) {
76: revert Errors.PriceOracle_InvalidConfiguration();
77: }
78:
79: pyth = _pyth;
80: base = _base;
81: quote = _quote;
82: feedId = _feedId;
83: maxStaleness = _maxStaleness;
84: maxConfWidth = _maxConfWidth;
85: baseDecimals = _getDecimals(base);
86: quoteDecimals = _getDecimals(quote);
87: }['59']
59: constructor(address _base, address _quote, bytes32 _feedId, uint8 _feedDecimals, uint256 _maxStaleness) {
60: if (_maxStaleness > MAX_STALENESS_UPPER_BOUND) revert Errors.PriceOracle_InvalidConfiguration();
61:
62: base = _base;
63: quote = _quote;
64: feedId = _feedId;
65: feedDecimals = _feedDecimals;
66: maxStaleness = _maxStaleness;
67: uint8 baseDecimals = _getDecimals(base);
68: uint8 quoteDecimals = _getDecimals(quote);
69: scale = ScaleUtils.calcScale(baseDecimals, quoteDecimals, _feedDecimals);
70: }['44']
44: constructor(address _tokenA, address _tokenB, uint24 _fee, uint32 _twapWindow, address _uniswapV3Factory) {
45: if (_twapWindow < MIN_TWAP_WINDOW || _twapWindow > uint32(type(int32).max)) {
46: revert Errors.PriceOracle_InvalidConfiguration();
47: }
48: tokenA = _tokenA;
49: tokenB = _tokenB;
50: fee = _fee;
51: twapWindow = _twapWindow;
52: pool = IUniswapV3Factory(_uniswapV3Factory).getPool(tokenA, tokenB, _fee);
53: if (pool == address(0)) revert Errors.PriceOracle_InvalidConfiguration();
54: }Rather defining the struct in a single line, it is more efficient to declare an empty struct and then assign each struct element individually. This can net quite a large gas saving of 130 per instance.
Num of instances: 1
Click to show findings
['78']
78: function updatePrice(uint48 timestamp) external { // <= FOUND
79:
80: Cache memory _cache = cache;
81: if (timestamp <= _cache.priceTimestamp) return;
82:
83: if (block.timestamp > timestamp) {
84:
85: uint256 staleness = block.timestamp - timestamp;
86: if (staleness > maxStaleness) {
87: revert Errors.PriceOracle_TooStale(staleness, maxStaleness);
88: }
89: } else if (timestamp - block.timestamp > RedstoneDefaultsLib.DEFAULT_MAX_DATA_TIMESTAMP_AHEAD_SECONDS) {
90:
91: revert Errors.PriceOracle_InvalidAnswer();
92: }
93:
94:
95: cache = Cache({price: _cache.price, priceTimestamp: timestamp}); // <= FOUND
96:
97:
98: uint256 price = getOracleNumericValueFromTxMsg(feedId);
99: if (price == 0) revert Errors.PriceOracle_InvalidAnswer();
100: if (price > type(uint208).max) revert Errors.PriceOracle_Overflow();
101: cache = Cache({price: uint208(price), priceTimestamp: timestamp}); // <= FOUND
102: emit CacheUpdated(price, timestamp);
103: }Emitting events in setter functions of smart contracts only when state variables change saves gas. This is because emitting events consumes gas, and unnecessary events, where no actual state change occurs, lead to wasteful consumption.
Num of instances: 1
Click to show findings
['40']
40: function _setGovernor(address newGovernor) internal { // <= FOUND
41: emit GovernorSet(governor, newGovernor); // <= FOUND
42: governor = newGovernor;
43: }In Solidity, choosing between memory and storage for variables, especially when dealing with structs or arrays, is crucial for optimizing gas costs. Variables declared as storage are pointers to the blockchain data, leading to lower gas consumption when fields are accessed or modified, as they don't require reading the entire structure. In contrast, memory variables copy the entire struct or array from storage, incurring significant gas costs, especially for large or complex structures. Therefore, use storage for state variables or when working within functions to manipulate existing contract data. Reserve memory for temporary data or when data needs to be passed to external functions as copies, ensuring efficient use of gas and avoiding unnecessary costs.
Num of instances: 2
Click to show findings
['80']
80:
81: Cache memory _cache = cache; // <= FOUND['80']
80: Cache memory _cache = cache; // <= FOUNDIn smart contract development, utilizing constants for known maximum or minimum values, rather than computing type(uint<n>).max or assuming 0 for .min, can significantly reduce gas costs. Constants require less runtime computation and storage, optimizing contract efficiency—a crucial strategy for developers aiming for cost-effective and performant code.
Num of instances: 2