M-10 Unmitigated

[code423n4]

Lines of code

https://github.com/reserve-protocol/protocol/blob/31394fdd52e2f16595dff36949076804b85e3f81/contracts/plugins/assets/OracleLib.sol#L24-L26

Vulnerability details

Comments

I think it's not a good idea to waste gas on that act of having no definite effect. Firstly, you can fetch the deprecating state by tracking the chainlink official data https://reference-data-directory.vercel.app/feeds-mainnet.json (It's the data source of the https://docs.chain.link/ but as soon the URLs are not documented anywhere, there is a chance they won't work in long term, but you could save/cache that data and update it from time to time.), which includes deprecating feeds, such as :

{
"compareOffchain": "",
"contractAddress": "0x299e74895b4De8dF505C43146D0555983859034B",
"contractType": "",
"contractVersion": 4,
"decimalPlaces": 9,
"ens": "gno-eth",
"formatDecimalPlaces": 0,
"healthPrice": "",
"heartbeat": 86400,
"history": false,
"multiply": "1000000000000000000",
"name": "GNO / ETH",
"pair": [
"GNO",
"ETH"
],
"path": "gno-eth",
"proxyAddress": "0xA614953dF476577E90dcf4e3428960e221EA4727",
"threshold": 2,
"valuePrefix": "",
"assetName": "Gnosis",
"feedCategory": "deprecating",
"feedType": "Crypto",
"docs": {
"assetClass": "Crypto",
"assetName": "Gnosis",
"baseAsset": "GNO",
"baseAssetClic": "GNO_CR",
"blockchainName": "Ethereum",
"clicProductName": "GNO/ETH-RefPrice-DF-Ethereum-001",
"deliveryChannelCode": "DF",
"feedCategory": "deprecating",
"feedType": "Crypto",
"hidden": true,
"productSubType": "Reference",
"productType": "Price",
"quoteAsset": "ETH",
"quoteAssetClic": "ETH_CR",
"shutdownDate": "March 8th, 2023"
},
"decimals": 18
},

And based on my observation, most of the deprecated feeds are still running normally. The chainlink does not specify the on-chain behavior when deprecation occurs. Maybe it will set the aggregator to address zero, or maybe other changes will also lead to a non-message revert, such as an address like 0xdEaD . So I think it's not a good idea to add the check EACAggregatorProxy(address(chainlinkFeed)).aggregator() == address(0) to every oracle query, which is a waste of gas.

Just let governance handle these rare events IMO.

Assessed type

Context

[thebrittfactor]

For transparency, the warden was unable to edit submission title. C4 staff has updated from M-11 to M-10.

[tbrent]

Anticipating removing the up-front call to .aggregator() and instead try-catch'ing the .latestRoundData() call. In the catch, we can make the .aggregator() call.

[tbrent]

I'm having trouble adding the Confirmed label to this one...

[liveactionllama]

Hi @tbrent - I've added sponsor confirmed on your behalf, but please let me know if that is incorrect.

Note: will also flag this for our dev team.

[c4-judge]

0xean marked the issue as satisfactory

[5z1punch]

Does it also need a "grade-x" tag? Im not sure for these scoring rules, just a remind.