From a52066c325bf0d0dd37d162918f3ec9587a014aa Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 2 Feb 2024 12:09:40 -0300 Subject: [PATCH 1/2] fix: clear vote for earlier rounds (OZ L-02) --- .../rewards/SubgraphAvailabilityManager.sol | 6 +++++ .../test/rewards/subgraphAvailability.test.ts | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol b/packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol index 349415283..9e562b6fe 100644 --- a/packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol +++ b/packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol @@ -203,6 +203,12 @@ contract SubgraphAvailabilityManager is Governed { : lastAllowVote[currentNonce][_subgraphDeploymentID]; lastVoteForSubgraph[_oracleIndex] = timestamp; + // clear opposite vote for a subgraph deployment if it exists + uint256[NUM_ORACLES] storage oppositeVoteForSubgraph = _deny + ? lastAllowVote[currentNonce][_subgraphDeploymentID] + : lastDenyVote[currentNonce][_subgraphDeploymentID]; + oppositeVoteForSubgraph[_oracleIndex] = 0; + emit OracleVote(_subgraphDeploymentID, _deny, _oracleIndex, timestamp); // check if execution threshold is reached, if it is call the RewardsManager diff --git a/packages/contracts/test/rewards/subgraphAvailability.test.ts b/packages/contracts/test/rewards/subgraphAvailability.test.ts index 0dbbd1abb..1f04a3bbc 100644 --- a/packages/contracts/test/rewards/subgraphAvailability.test.ts +++ b/packages/contracts/test/rewards/subgraphAvailability.test.ts @@ -329,6 +329,31 @@ describe('SubgraphAvailabilityManager', () => { // previous votes are no longer valid expect(await rewardsManager.isDenied(subgraphDeploymentID1)).to.be.false }) + + it('clears opposite vote when voting', async () => { + const denied = true + await subgraphAvailabilityManager.connect(oracleOne).vote(subgraphDeploymentID1, denied, 0) + await subgraphAvailabilityManager.connect(oracleTwo).vote(subgraphDeploymentID1, denied, 1) + await subgraphAvailabilityManager.connect(oracleThree).vote(subgraphDeploymentID1, denied, 2) + + // 3/5 oracles vote denied = true so subgraph is denied + expect(await rewardsManager.isDenied(subgraphDeploymentID1)).to.be.true + + // oracleOne changes its vote to denied = false + await subgraphAvailabilityManager.connect(oracleOne).vote(subgraphDeploymentID1, false, 0) + + // last deny vote should be 0 for oracleOne + expect( + await subgraphAvailabilityManager.lastDenyVote(0, subgraphDeploymentID1, 0), + ).to.be.equal(0) + + // executionThreshold isn't met now since oracleOne changed its vote + expect(await subgraphAvailabilityManager.checkVotes(subgraphDeploymentID1, denied)).to.be + .false + + // subgraph is still denied in rewards manager because only one oracle changed its vote + expect(await rewardsManager.isDenied(subgraphDeploymentID1)).to.be.true + }) }) describe('vote many', async () => { From 1ce46f0caede4430c30ab5d597830ddf4a78e22b Mon Sep 17 00:00:00 2001 From: Miguel de Elias Date: Fri, 2 Feb 2024 12:32:14 -0300 Subject: [PATCH 2/2] fix: change to if/else --- .../rewards/SubgraphAvailabilityManager.sol | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol b/packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol index 9e562b6fe..8248f81c9 100644 --- a/packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol +++ b/packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol @@ -197,17 +197,15 @@ contract SubgraphAvailabilityManager is Governed { ) private { uint256 timestamp = block.timestamp; - // corresponding votes based on _deny for a subgraph deployment - uint256[NUM_ORACLES] storage lastVoteForSubgraph = _deny - ? lastDenyVote[currentNonce][_subgraphDeploymentID] - : lastAllowVote[currentNonce][_subgraphDeploymentID]; - lastVoteForSubgraph[_oracleIndex] = timestamp; - - // clear opposite vote for a subgraph deployment if it exists - uint256[NUM_ORACLES] storage oppositeVoteForSubgraph = _deny - ? lastAllowVote[currentNonce][_subgraphDeploymentID] - : lastDenyVote[currentNonce][_subgraphDeploymentID]; - oppositeVoteForSubgraph[_oracleIndex] = 0; + if (_deny) { + lastDenyVote[currentNonce][_subgraphDeploymentID][_oracleIndex] = timestamp; + // clear opposite vote for a subgraph deployment if it exists + lastAllowVote[currentNonce][_subgraphDeploymentID][_oracleIndex] = 0; + } else { + lastAllowVote[currentNonce][_subgraphDeploymentID][_oracleIndex] = timestamp; + // clear opposite vote for a subgraph deployment if it exists + lastDenyVote[currentNonce][_subgraphDeploymentID][_oracleIndex] = 0; + } emit OracleVote(_subgraphDeploymentID, _deny, _oracleIndex, timestamp);