Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline toll access control and defend against malicious Chronicle diss #23

Merged
merged 6 commits into from
Mar 28, 2024
Merged

Inline toll access control and defend against malicious Chronicle diss #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
759d8d6
Inline toll access control
pmerkleplant Mar 27, 2024
a573e02
Defend against malicious chronicle diss
pmerkleplant Mar 27, 2024
1394c82
Formatting
pmerkleplant Mar 27, 2024
0eb572f
Fix integration test having erroneous aggor.kiss call
pmerkleplant Mar 27, 2024
8a40065
Silence compiler warnings
pmerkleplant Mar 28, 2024
048e425
Use non-named function argument if not used
pmerkleplant Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 91 additions & 24 deletions src/Aggor.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity ^0.8.16;

import {Auth} from "chronicle-std/auth/Auth.sol";
import {Toll} from "chronicle-std/toll/Toll.sol";
import {IToll} from "chronicle-std/toll/IToll.sol";

import {IChronicle} from "chronicle-std/IChronicle.sol";
import {IChainlinkAggregatorV3} from
Expand All @@ -22,10 +22,17 @@ import {LibMedian} from "./libs/LibMedian.sol";
*
* @notice Oracle aggregator distributing trust among different oracle providers
*
* @dev While Chronicle oracle's normaly use the chronicle-std/Toll module for
* access controlling the read functions this implementation uses a
* non-configurable, inlined, approach. Two addresses are granted read access,
* the zero address and `_bud` being an immutable set during deployment.
*
* Nevertheless, the full IToll interface is implemented to ensure compatibility.
*
* @author Chronicle Labs, Inc
* @custom:security-contact [email protected]
*/
contract Aggor is IAggor, Auth, Toll {
contract Aggor is IAggor, IToll, Auth {
using LibUniswapOracles for address;

// -- Internal Constants --
Expand All @@ -43,6 +50,11 @@ contract Aggor is IAggor, Auth, Toll {
/// @inheritdoc IAggor
uint8 public constant decimals = 8;

// -- Toll

/// @dev Bud is the only non-zero address being toll'ed.
address internal immutable _bud;

// -- Oracles

/// @inheritdoc IAggor
Expand Down Expand Up @@ -70,10 +82,20 @@ contract Aggor is IAggor, Auth, Toll {
/// @inheritdoc IAggor
uint32 public ageThreshold;

// -- Modifier --

modifier toll() {
if (msg.sender != _bud && msg.sender != address(0)) {
revert NotTolled(msg.sender);
}
_;
}

// -- Constructor --

constructor(
address initialAuthed,
address bud_,
address chronicle_,
address chainlink_,
address uniswapPool_,
Expand All @@ -94,6 +116,7 @@ contract Aggor is IAggor, Auth, Toll {
);

// Set immutables.
_bud = bud_;
chronicle = chronicle_;
chainlink = chainlink_;
uniswapPool = uniswapPool_;
Expand All @@ -102,6 +125,12 @@ contract Aggor is IAggor, Auth, Toll {
uniswapBaseTokenDecimals = uniswapBaseTokenDecimals_;
uniswapLookback = uniswapLookback_;

// Emit events indicating address(0) and _bud are tolled.
// Note to use address(0) as caller to indicate address was toll'ed
// during deployment.
emit TollGranted(address(0), address(0));
emit TollGranted(address(0), _bud);

// Set configurations.
_setAgreementDistance(agreementDistance_);
_setAgeThreshold(ageThreshold_);
Expand Down Expand Up @@ -159,7 +188,7 @@ contract Aggor is IAggor, Auth, Toll {
/// @dev Note that the value's age is always block.timestamp except if the
/// value itself is invalid.
function _read() internal view returns (uint128, uint, Status memory) {
// Read chronicle and chainlink oracles.
// Read Chronicle and Chainlink oracles.
(bool okChr, uint128 valChr) = _readChronicle();
(bool okChl, uint128 valChl) = _readChainlink();

Expand Down Expand Up @@ -188,10 +217,10 @@ contract Aggor is IAggor, Auth, Toll {

// Otherwise not possible to decide which oracle is ok.
} else if (okChr) {
// If only chronicle ok, use chronicle's value.
// If only Chronicle ok, use Chronicle's value.
return (valChr, age, Status({path: 4, goodOracleCtr: 1}));
} else if (okChl) {
// If only chainlink ok, use chainlink's value.
// If only Chainlink ok, use Chainlink's value.
return (valChl, age, Status({path: 4, goodOracleCtr: 1}));
}

Expand All @@ -205,32 +234,40 @@ contract Aggor is IAggor, Auth, Toll {
return (0, 0, Status({path: 6, goodOracleCtr: 0}));
}

/// @dev Reads the chronicle oracle.
/// @dev Reads the Chronicle oracle.
///
/// @dev Note that while chronicle uses 18 decimals, the returned value is
/// already scaled to `decimals`.
///
/// @return bool Whether oracle is ok.
/// @return uint128 The oracle's val.
function _readChronicle() internal view returns (bool, uint128) {
(bool ok, uint val, uint age) = IChronicle(chronicle).tryReadWithAge();
// assert(val <= type(uint128).max);
// assert(!ok || val != 0); // ok -> val != 0
// assert(age <= block.timestamp);
// Note that Chronicle's `try...` functions revert iff the caller is not
// toll'ed.
try IChronicle(chronicle).tryReadWithAge() returns (
bool ok, uint val, uint age
) {
// assert(val <= type(uint128).max);
// assert(!ok || val != 0); // ok -> val != 0
// assert(age <= block.timestamp);

// Fail if not ok or value stale.
if (!ok || age + ageThreshold < block.timestamp) {
return (false, 0);
}
// Fail if not ok or value stale.
if (!ok || age + ageThreshold < block.timestamp) {
return (false, 0);
}

// Scale value down from chronicle decimals to aggor decimals.
// assert(_DECIMALS_CHRONICLES >= decimals).
val /= 10 ** (_DECIMALS_CHRONICLE - decimals);
// Scale value down from Chronicle decimals to Aggor decimals.
// assert(_DECIMALS_CHRONICLES >= decimals).
val /= 10 ** (_DECIMALS_CHRONICLE - decimals);

return (true, uint128(val));
return (true, uint128(val));
} catch {
// assert(!IToll(chronicle).tolled(address(this)));
return (false, 0);
}
}

/// @dev Reads the chainlink oracle.
/// @dev Reads the Chainlink oracle.
///
/// @return bool Whether oracle is ok.
/// @return uint128 The oracle's val.
Expand Down Expand Up @@ -382,6 +419,39 @@ contract Aggor is IAggor, Auth, Toll {
}
}

// -- IToll Functionality --

/// @inheritdoc IToll
/// @dev Function is disabled!
function kiss(address /*who*/ ) external view auth {
revert();
}

/// @inheritdoc IToll
/// @dev Function is disabled!
function diss(address /*who*/ ) external view auth {
revert();
}

/// @inheritdoc IToll
function tolled(address who) public view returns (bool) {
return who == _bud || who == address(0);
}

/// @inheritdoc IToll
function tolled() external view returns (address[] memory) {
address[] memory result = new address[](2);
result[0] = address(0);
result[1] = _bud;

return result;
}

/// @inheritdoc IToll
function bud(address who) external view returns (uint) {
return tolled(who) ? 1 : 0;
}

// -- Internal Helpers --

function _inAgreementDistance(uint128 a, uint128 b)
Expand All @@ -395,11 +465,6 @@ contract Aggor is IAggor, Auth, Toll {
return uint(a) * 1e18 >= agreementDistance * uint(b);
}
}

// -- Overridden Toll Functions --

/// @dev Defines authorization for IToll's authenticated functions.
function toll_auth() internal override(Toll) auth {}
}

/**
Expand All @@ -411,6 +476,7 @@ contract Aggor_BASE_QUOTE_COUNTER is Aggor {
// @todo ^^^^ ^^^^^ ^^^^^^^ Adjust name of Aggor instance
constructor(
address initialAuthed,
address bud_,
address chronicle_,
address chainlink_,
address uniswapPool_,
Expand All @@ -423,6 +489,7 @@ contract Aggor_BASE_QUOTE_COUNTER is Aggor {
)
Aggor(
initialAuthed,
bud_,
chronicle_,
chainlink_,
uniswapPool_,
Expand Down
Loading
Loading