Skip to content

Commit

Permalink
Upgrade StaderOracle implementation (#258)
Browse files Browse the repository at this point in the history
* forge install: openzeppelin-foundry-upgrades

v0.3.6

* feat: change min ONO requirement

* chore: update equality check to more broader

* feat: update consensus of sd price match
  • Loading branch information
blockgroot authored Nov 5, 2024
1 parent c2b620a commit deef735
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 39 deletions.
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
[submodule "lib/forge-std"]
path = lib/forge-std
url = https://github.com/foundry-rs/forge-std
[submodule "lib/openzeppelin-foundry-upgrades"]
path = lib/openzeppelin-foundry-upgrades
url = https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades
8 changes: 5 additions & 3 deletions contracts/StaderOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ contract StaderOracle is IStaderOracle, AccessControlUpgradeable, PausableUpgrad
uint256 public constant MAX_ER_UPDATE_FREQUENCY = 7200 * 7; // 7 days
uint256 public constant ER_CHANGE_MAX_BPS = 10_000;
uint256 public override erChangeLimit;
uint256 public constant MIN_TRUSTED_NODES = 5;
uint256 public constant MIN_TRUSTED_NODES = 3;
uint256 public override trustedNodeChangeCoolingPeriod;

/// @inheritdoc IStaderOracle
Expand Down Expand Up @@ -119,6 +119,9 @@ contract StaderOracle is IStaderOracle, AccessControlUpgradeable, PausableUpgrad
if (block.number < lastTrustedNodeCountChangeBlock + trustedNodeChangeCoolingPeriod) {
revert CooldownNotComplete();
}
if (trustedNodesCount <= MIN_TRUSTED_NODES) {
revert InsufficientTrustedNodes();
}
lastTrustedNodeCountChangeBlock = block.number;

isTrustedNode[_nodeAddress] = false;
Expand Down Expand Up @@ -304,8 +307,7 @@ contract StaderOracle is IStaderOracle, AccessControlUpgradeable, PausableUpgrad
// Emit SD Price submitted event
emit SDPriceSubmitted(msg.sender, _sdPriceData.sdPriceInETH, _sdPriceData.reportingBlockNumber, block.number);

// price can be derived once more than 66% percent oracles have submitted price
if ((submissionCount >= (2 * trustedNodesCount) / 3 + 1)) {
if ((submissionCount >= trustedNodesCount / 2 + 1)) {
lastReportedSDPriceData = _sdPriceData;
lastReportedSDPriceData.sdPriceInETH = getMedianValue(sdPrices);

Expand Down
1 change: 1 addition & 0 deletions lib/openzeppelin-foundry-upgrades
66 changes: 30 additions & 36 deletions test/foundry_tests/StaderOracle.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ contract StaderOracleTest is Test {
}

function test_add_remove_trustedNode() public {
// Tests for adding nodes
address trustedNode = vm.addr(123);
assertEq(staderOracle.trustedNodesCount(), 0);
assertFalse(staderOracle.isTrustedNode(trustedNode));
Expand All @@ -139,16 +140,6 @@ contract StaderOracleTest is Test {
assertEq(staderOracle.trustedNodesCount(), 1);
assertTrue(staderOracle.isTrustedNode(trustedNode));

vm.expectRevert(IStaderOracle.NodeNotTrusted.selector);
vm.prank(staderManager);
staderOracle.removeTrustedNode(vm.addr(567));

vm.prank(staderManager);
staderOracle.removeTrustedNode(trustedNode);

assertEq(staderOracle.trustedNodesCount(), 0);
assertFalse(staderOracle.isTrustedNode(trustedNode));

// lets update trustedNode cooling period
vm.expectRevert(UtilLib.CallerNotManager.selector);
staderOracle.updateTrustedNodeChangeCoolingPeriod(100);
Expand All @@ -157,22 +148,36 @@ contract StaderOracleTest is Test {
staderOracle.updateTrustedNodeChangeCoolingPeriod(100);

vm.expectRevert(IStaderOracle.CooldownNotComplete.selector);
staderOracle.addTrustedNode(vm.addr(78));
staderOracle.addTrustedNode(vm.addr(77));

// wait for 100 blocks
// wait for 100 blocks each time to add node
vm.roll(block.number + 100);
staderOracle.addTrustedNode(vm.addr(77));
vm.roll(block.number + 100);
staderOracle.addTrustedNode(vm.addr(78));
assertEq(staderOracle.trustedNodesCount(), 1);
vm.roll(block.number + 100);
staderOracle.addTrustedNode(vm.addr(79));
assertEq(staderOracle.trustedNodesCount(), 4);
assertTrue(staderOracle.isTrustedNode(vm.addr(77)));
assertTrue(staderOracle.isTrustedNode(vm.addr(78)));
assertTrue(staderOracle.isTrustedNode(vm.addr(79)));

// Tests for removing nodes
vm.expectRevert(IStaderOracle.NodeNotTrusted.selector);
staderOracle.removeTrustedNode(vm.addr(567));

vm.expectRevert(IStaderOracle.CooldownNotComplete.selector);
staderOracle.removeTrustedNode(vm.addr(78));
staderOracle.removeTrustedNode(vm.addr(77));

// wait for 100 blocks
vm.roll(block.number + 100);
staderOracle.removeTrustedNode(vm.addr(77));
assertEq(staderOracle.trustedNodesCount(), 3);
assertFalse(staderOracle.isTrustedNode(vm.addr(77)));

vm.roll(block.number + 100);
vm.expectRevert(IStaderOracle.InsufficientTrustedNodes.selector);
staderOracle.removeTrustedNode(vm.addr(78));
assertEq(staderOracle.trustedNodesCount(), 0);
assertFalse(staderOracle.isTrustedNode(vm.addr(78)));
vm.stopPrank();
}

Expand All @@ -189,7 +194,7 @@ contract StaderOracleTest is Test {
vm.expectRevert(IStaderOracle.InsufficientTrustedNodes.selector);
staderOracle.submitSDPrice(sdPriceData);

assertEq(staderOracle.MIN_TRUSTED_NODES(), 5);
assertEq(staderOracle.MIN_TRUSTED_NODES(), 3);
address trustedNode2 = vm.addr(702);
address trustedNode3 = vm.addr(703);
address trustedNode4 = vm.addr(704);
Expand Down Expand Up @@ -307,17 +312,12 @@ contract StaderOracleTest is Test {
vm.prank(trustedNode3);
staderOracle.submitSDPrice(sdPriceData);

sdPriceData.reportingBlockNumber = 5 * 7200;
sdPriceData.sdPriceInETH = 4;
vm.prank(trustedNode4);
staderOracle.submitSDPrice(sdPriceData);

// now consensus is met for reporting block num 5 * 7200
// trustedNode1 manipulated the sd price if other oracles are not wrking properly
// sdPrice submited were [1,6,2,4] => hence median = (2+4)/2 = 3
// sdPrice submited were [1,6,2] => hence median = 2
(lastSDReportingBlockNumber, lastSDPrice) = staderOracle.lastReportedSDPriceData();
assertEq(lastSDReportingBlockNumber, 5 * 7200);
assertEq(lastSDPrice, 3);
assertEq(lastSDPrice, 2);

// trusted node 5 tries to submit at reportable block 5 * 7200
sdPriceData.reportingBlockNumber = 5 * 7200;
Expand All @@ -330,7 +330,7 @@ contract StaderOracleTest is Test {
function test_submitSDPrice_manipulation_not_possible_by_minority_malicious_oracles() public {
SDPriceData memory sdPriceData = SDPriceData({ reportingBlockNumber: 1212, sdPriceInETH: 1 });

assertEq(staderOracle.MIN_TRUSTED_NODES(), 5);
assertEq(staderOracle.MIN_TRUSTED_NODES(), 3);
address trustedNode1 = vm.addr(701);
address trustedNode2 = vm.addr(702);
address trustedNode3 = vm.addr(703);
Expand All @@ -355,9 +355,6 @@ contract StaderOracleTest is Test {
vm.prank(trustedNode2);
staderOracle.submitSDPrice(sdPriceData);

vm.prank(trustedNode3);
staderOracle.submitSDPrice(sdPriceData);

// cycle 2
vm.roll(2 * 7200 + 1);
sdPriceData.reportingBlockNumber = 2 * 7200;
Expand All @@ -369,9 +366,6 @@ contract StaderOracleTest is Test {
vm.prank(trustedNode2);
staderOracle.submitSDPrice(sdPriceData);

vm.prank(trustedNode3);
staderOracle.submitSDPrice(sdPriceData);

// trustedNode4 submits for cycle 1
sdPriceData.reportingBlockNumber = 1 * 7200;
sdPriceData.sdPriceInETH = 1;
Expand Down Expand Up @@ -800,7 +794,7 @@ contract StaderOracleTest is Test {
totalETHXSupply: 100
});

assertEq(staderOracle.MIN_TRUSTED_NODES(), 5);
assertEq(staderOracle.MIN_TRUSTED_NODES(), 3);
address trustedNode1 = vm.addr(701);
address trustedNode2 = vm.addr(702);
address trustedNode3 = vm.addr(703);
Expand Down Expand Up @@ -1007,7 +1001,7 @@ contract StaderOracleTest is Test {
sortedInvalidSignaturePubkeys: invalidSignaturePubkeys
});

assertEq(staderOracle.MIN_TRUSTED_NODES(), 5);
assertEq(staderOracle.MIN_TRUSTED_NODES(), 3);
address trustedNode1 = vm.addr(701);
address trustedNode2 = vm.addr(702);
address trustedNode3 = vm.addr(703);
Expand Down Expand Up @@ -1082,7 +1076,7 @@ contract StaderOracleTest is Test {
sortedPubkeys: sortedPubkeys
});

assertEq(staderOracle.MIN_TRUSTED_NODES(), 5);
assertEq(staderOracle.MIN_TRUSTED_NODES(), 3);
address trustedNode1 = vm.addr(701);
address trustedNode2 = vm.addr(702);
address trustedNode3 = vm.addr(703);
Expand Down Expand Up @@ -1162,7 +1156,7 @@ contract StaderOracleTest is Test {
sortedPubkeys: sortedPubkeys
});

assertEq(staderOracle.MIN_TRUSTED_NODES(), 5);
assertEq(staderOracle.MIN_TRUSTED_NODES(), 3);
address trustedNode1 = vm.addr(701);
address trustedNode2 = vm.addr(702);
address trustedNode3 = vm.addr(703);
Expand Down Expand Up @@ -1232,7 +1226,7 @@ contract StaderOracleTest is Test {
slashedValidatorsCount: 4
});

assertEq(staderOracle.MIN_TRUSTED_NODES(), 5);
assertEq(staderOracle.MIN_TRUSTED_NODES(), 3);
address trustedNode1 = vm.addr(701);
address trustedNode2 = vm.addr(702);
address trustedNode3 = vm.addr(703);
Expand Down

0 comments on commit deef735

Please sign in to comment.