Skip to content

Commit

Permalink
review: add GBP & registry update
Browse files Browse the repository at this point in the history
  • Loading branch information
mkflow27 committed Jun 10, 2024
1 parent 356e2ac commit e7c0857
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
- Checked by: @\<GitHub handle of secondary reviewer\>
- Deployed at:
- [gnosis:0xdc90e2680094314CEaB45CE15100F6e02cEB7ceD](https://gnosisscan.io/address/0xdc90e2680094314ceab45ce15100f6e02ceb7ced#code)
- [gnosis:0x92320D3C8Fd6BE59b22eB0eEe330901Fe4617f33](https://gnosisscan.io/address/0x92320D3C8Fd6BE59b22eB0eEe330901Fe4617f33#code)
- Audit report(s):
- [Chronicle Oracles audits](\<link to audit\>)

## Context
This rate Provider bridges the eth/reth exchange rate from Mainnet to gnosis chain. This is done via an oracle solution developed by chronicle.
This rate Provider bridges the eth/reth exchange rate & GBP/USD to gnosis chain. This is done via an oracle solution developed by chronicle.

## Review Checklist: Bare Minimum Compatibility
Each of the items below represents an absolute requirement for the Rate Provider. If any of these is unchecked, the Rate Provider is unfit to use.
Expand All @@ -29,12 +30,20 @@ If none of these is checked, then this might be a pretty great Rate Provider! If

### Oracles
- [x] Price data is provided by an off-chain source (e.g., a Chainlink oracle, a multisig, or a network of nodes).
- source: Chronicle protocol Oracle
- source address: [gnosis:0xE04a8f725b49c9D36C0fD3495F4a792056374847](https://gnosisscan.io/address/0xe04a8f725b49c9d36c0fd3495f4a792056374847)
- any protections? YES
- The rate data's supplied age must be greater than the timestamp of last successful update
- the rate data's age must not be greater than current time
- The rate data's integrity is verified by the supplied signature. Currently `bar` (7) signers verify the rate's integrity. For more information see `_poke` and `isAcceptableSchnorrSignatureNow` as part of the PriceFeed `Chronicle_RETH_ETH_1` contract deployed at [gnosis:0x7706A143c750aDfc2196c4Bf84e6BB012Aed1182](https://gnosisscan.io/address/0x7706a143c750adfc2196c4bf84e6bb012aed1182#code)
- reth/eth:
- source: Chronicle protocol Oracle
- source address: [gnosis:0xE04a8f725b49c9D36C0fD3495F4a792056374847](https://gnosisscan.io/address/0xe04a8f725b49c9d36c0fd3495f4a792056374847)
- any protections? YES
- The rate data's supplied age must be greater than the timestamp of last successful update
- the rate data's age must not be greater than current time
- The rate data's integrity is verified by the supplied signature. Currently `bar` (7) signers verify the rate's integrity. For more information see `_poke` and `isAcceptableSchnorrSignatureNow` as part of the PriceFeed `Chronicle_RETH_ETH_1` contract deployed at [gnosis:0x7706A143c750aDfc2196c4Bf84e6BB012Aed1182](https://gnosisscan.io/address/0x7706a143c750adfc2196c4bf84e6bb012aed1182#code)
- GBP/USD:
- source: Chronicle protocol Oracle
- source address: [gnosis:0x0E418d54863a3fAfeC9e96a358795f0f236f5f66](https://gnosisscan.io/address/0x0E418d54863a3fAfeC9e96a358795f0f236f5f66)
- any protections? YES
- The rate data's supplied age must be greater than the timestamp of last successful update
- the rate data's age must not be greater than current time
- The rate data's integrity is verified by the supplied signature. Currently `bar` (7) signers verify the rate's integrity. For more information see `_poke` and `isAcceptableSchnorrSignatureNow` as part of the PriceFeed `Chronicle_GBP_USD_1` contract deployed at [gnosis:0x0E418d54863a3fAfeC9e96a358795f0f236f5f66](https://gnosisscan.io/address/0x0E418d54863a3fAfeC9e96a358795f0f236f5f66#code)

- [ ] Price data is expected to be volatile (e.g., because it represents an open market price instead of a (mostly) monotonically increasing price).

Expand All @@ -44,21 +53,8 @@ If none of these is checked, then this might be a pretty great Rate Provider! If
## Additional Findings
To save time, we do not bother pointing out low-severity/informational issues or gas optimizations (unless the gas usage is particularly egregious). Instead, we focus only on high- and medium-severity findings which materially impact the contract's functionality and could harm users.

### M-01: `owner` is an EOA and can make Rate Provider revert.
The current `owner` of the RateProvider can call `enableTollgate` which would in its current form make the RateProvider revert.
```solidity
function getRate() public view override returns (uint256) {
if (tollgateEnabled && !tolled(msg.sender)) {
revert NotTolled(msg.sender);
}
return super.getRate();
}
```
Currently `tollgateEnabled` is false and the msg.sender (the pool) is not tolled, skipping the if condition. However if the tollgate was enabled, the rateProvider would revert. Currently the `owner` has this capability.

Suggestion: The `owner` should call `transferOwnership` with the `newOwner` being a multisig.

## Conclusion
**Summary judgment: \<SAFE/UNSAFE\>**
**Summary judgment: **

This rate provider should work well with Balancer pools. The oracle providing the rate data has various guardrails in place ensuring the integrity of the rate being provided. The `owner` of the rate provider has the capability to revert the call to `getRate`. However this potential revert scenario is guarded behind a Multisig of [1/4] for reth and [2/6] for GBP.
11 changes: 10 additions & 1 deletion rate-providers/registry.json
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,16 @@
"asset": "0xc791240d1f2def5938e2031364ff4ed887133c3d",
"name": "TollgateChronicleRateProvider",
"summary": "",
"review": "./rETHRateProviderGnosis.md",
"review": "./TollgateChronicleRateProvider.md",
"warnings": [],
"factory": "",
"upgradeableComponents": []
},
"0x92320D3C8Fd6BE59b22eB0eEe330901Fe4617f33": {
"asset": "",
"name": "TollgateChronicleRateProvider",
"summary": "",
"review": "./TollgateChronicleRateProvider.md",
"warnings": [],
"factory": "",
"upgradeableComponents": []
Expand Down

0 comments on commit e7c0857

Please sign in to comment.