From a9ad4655e050df20c6b80b0778c09b756c21c1c5 Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Wed, 16 Oct 2024 05:35:15 +0000 Subject: [PATCH 1/4] forge install: openzeppelin-foundry-upgrades v0.3.6 --- .gitmodules | 3 +++ lib/openzeppelin-foundry-upgrades | 1 + 2 files changed, 4 insertions(+) create mode 160000 lib/openzeppelin-foundry-upgrades diff --git a/.gitmodules b/.gitmodules index 888d42dc..7d79667c 100644 --- a/.gitmodules +++ b/.gitmodules @@ -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 diff --git a/lib/openzeppelin-foundry-upgrades b/lib/openzeppelin-foundry-upgrades new file mode 160000 index 00000000..16e0ae21 --- /dev/null +++ b/lib/openzeppelin-foundry-upgrades @@ -0,0 +1 @@ +Subproject commit 16e0ae21e0e39049f619f2396fa28c57fad07368 From a39e5f4ec2a8f84a12b8a7193928c6277c4c9545 Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Thu, 17 Oct 2024 08:13:10 +0000 Subject: [PATCH 2/4] feat: change min ONO requirement --- contracts/StaderOracle.sol | 5 ++- test/foundry_tests/StaderOracle.t.sol | 51 +++++++++++++++------------ 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/contracts/StaderOracle.sol b/contracts/StaderOracle.sol index bec4b6e2..b90cd898 100644 --- a/contracts/StaderOracle.sol +++ b/contracts/StaderOracle.sol @@ -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 @@ -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; diff --git a/test/foundry_tests/StaderOracle.t.sol b/test/foundry_tests/StaderOracle.t.sol index b818690d..8af812af 100644 --- a/test/foundry_tests/StaderOracle.t.sol +++ b/test/foundry_tests/StaderOracle.t.sol @@ -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)); @@ -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); @@ -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(); } @@ -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); @@ -330,7 +335,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); @@ -800,7 +805,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); @@ -1007,7 +1012,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); @@ -1082,7 +1087,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); @@ -1162,7 +1167,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); @@ -1232,7 +1237,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); From 11c68d04b5950000c5428b209a3cc78cc7371ab4 Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Thu, 17 Oct 2024 10:23:41 +0000 Subject: [PATCH 3/4] chore: update equality check to more broader --- contracts/StaderOracle.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/StaderOracle.sol b/contracts/StaderOracle.sol index b90cd898..1342e2a0 100644 --- a/contracts/StaderOracle.sol +++ b/contracts/StaderOracle.sol @@ -119,7 +119,7 @@ contract StaderOracle is IStaderOracle, AccessControlUpgradeable, PausableUpgrad if (block.number < lastTrustedNodeCountChangeBlock + trustedNodeChangeCoolingPeriod) { revert CooldownNotComplete(); } - if (trustedNodesCount == MIN_TRUSTED_NODES) { + if (trustedNodesCount <= MIN_TRUSTED_NODES) { revert InsufficientTrustedNodes(); } lastTrustedNodeCountChangeBlock = block.number; From 2d6a43ff9e077c383e0583fef64b870d9d9c2520 Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Thu, 24 Oct 2024 05:07:57 +0000 Subject: [PATCH 4/4] feat: update consensus of sd price match --- contracts/StaderOracle.sol | 3 +-- test/foundry_tests/StaderOracle.t.sol | 15 ++------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/contracts/StaderOracle.sol b/contracts/StaderOracle.sol index 1342e2a0..60f9f0c2 100644 --- a/contracts/StaderOracle.sol +++ b/contracts/StaderOracle.sol @@ -307,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); diff --git a/test/foundry_tests/StaderOracle.t.sol b/test/foundry_tests/StaderOracle.t.sol index 8af812af..bb5f2c30 100644 --- a/test/foundry_tests/StaderOracle.t.sol +++ b/test/foundry_tests/StaderOracle.t.sol @@ -312,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; @@ -360,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; @@ -374,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;