feat: subgraph availability manager contract #882

Maikol · 2023-11-16T21:53:24Z

No description provided.

openzeppelin-code · 2023-11-16T22:02:24Z

feat: subgraph availability manager contract

Generated at commit: bcf69579662fcf3e2fee87cf3a0aa5ee4196cf6d

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	2 4 0 15 37 58
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

For more details view the full report in OpenZeppelin Code Inspector

codecov · 2023-11-16T22:11:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.83%. Comparing base (59154f7) to head (bcf6957).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #882      +/-   ##
==========================================
+ Coverage   92.70%   92.83%   +0.13%     
==========================================
  Files          46       47       +1     
  Lines        2370     2416      +46     
  Branches      426      439      +13     
==========================================
+ Hits         2197     2243      +46     
  Misses        173      173

Flag	Coverage Δ
unittests	`92.83% <100.00%> (+0.13%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tmigone

Looking good! left a few comments

contracts/rewards/SubgraphAvailabilityManager.sol

test/rewards/subgraphAvailability.test.ts

contracts/rewards/SubgraphAvailabilityManager.sol

pcarranzav

Some details while I'm still reviewing

contracts/rewards/SubgraphAvailabilityManager.sol

pcarranzav · 2023-12-08T03:05:36Z

Finished reviewing, I'm ready to approve after hearing what you think about the previous comments.
Nice work, and very good test suite 🔥

Maikol · 2023-12-08T20:36:50Z

@pcarranzav thanks for the review, addressed comments!

pcarranzav

One remaining detail but LGTM

contracts/rewards/SubgraphAvailabilityManager.sol

tmigone

Looks good, just one comment.

tmigone · 2023-12-21T20:08:59Z

contracts/rewards/SubgraphAvailabilityManager.sol

+
+        executionThreshold = _executionThreshold;
+        voteTimeLimit = _voteTimeLimit;
+        oracles = _oracles;


hmm i think we should set the oracles inside the for loop above and emit OracleSet for each one.

Otherwise the subgraph will have a tough time getting the initial list of oracles

Nice catch. I guess we should update the subgraph when we're ready to release this, I'll note that.

…ssess

tmigone · 2024-01-02T18:22:20Z

packages/contracts/config/graph.sepolia.yml

@@ -159,3 +159,13 @@ contracts:
      controller: "${{Controller.address}}"
    calls:
      - fn: "syncAllContracts"
+  SubgraphAvailabilityManager:
+    init:
+      governor: "${{Env.deployer}}"


Is there a reason we don't set the council as the governor from the get go?

I was using the deployer while I'm testing so I don't have to use the multisig.

pcarranzav · 2024-01-09T20:09:13Z

packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol

+
+    /// @notice Mapping of current nonce to subgraph deployment ID to oracle index to timestamp of last deny vote
+    /// currentNonce => subgraphDeploymentId => oracleIndex => timestamp
+    mapping(uint256 => mapping(bytes32 => mapping(uint256 => uint256))) public lastDenyVote;


I think this could be mapping(uint256 => mapping(bytes32 => uint256[NUM_ORACLES]) now? which I think would be a bit cheaper?

Definitely, good idea!

pcarranzav · 2024-01-09T20:09:28Z

packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol

+
+    /// @notice Mapping of  current nonce to subgraph deployment ID to oracle index to timestamp of last allow vote
+    /// currentNonce => subgraphDeploymentId => oracleIndex => timestamp
+    mapping(uint256 => mapping(bytes32 => mapping(uint256 => uint256))) public lastAllowVote;


Same as above, the last mapping could be an array now

packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol

fix: added input validation for executionThreshold (OZ L-01)

docs: added missing docstrings for events (OZ L-03)

fix: change OracleVote event order (OZ N-01)

fix: gas optimization (OZ N-02)

fix: clear vote for earlier rounds (OZ L-02)

fix: remove setDeniedMany since it's unused (OZ N-03)

tmigone requested changes Nov 22, 2023

View reviewed changes

Maikol requested review from tmigone and pcarranzav November 29, 2023 12:10

Maikol marked this pull request as ready for review December 1, 2023 21:12

pcarranzav reviewed Dec 7, 2023

View reviewed changes

pcarranzav previously approved these changes Dec 8, 2023

View reviewed changes

contracts/rewards/SubgraphAvailabilityManager.sol Outdated Show resolved Hide resolved

Maikol dismissed pcarranzav’s stale review via a4f0362 December 11, 2023 13:08

tmigone requested changes Dec 21, 2023

View reviewed changes

Maikol added 11 commits December 21, 2023 19:01

feat: subgraph availability manager contract

ad358fb

feat: vote denied many

77e8dc3

fix: pr feedback changes

1805cd9

fix: use natspec for state variables comments

f4942f2

fix: use currentNonce to refresh votes, init oracles with valid addre…

fa9b153

…ssess

fix: update functions docs

08e269f

fix: removed unnecessary code in tests

35f6392

fix: typo, comments and changed maxOracles to be a constant

0896337

fix: change constant variable name to match style guide

ad4baec

chore: deploy to staging

a67ad20

fix: emit oracle set event on init

ddbcc86

Maikol force-pushed the mde/sam-contract branch from a4f0362 to ddbcc86 Compare December 21, 2023 22:07

tmigone reviewed Jan 2, 2024

View reviewed changes

pcarranzav reviewed Jan 9, 2024

View reviewed changes

Maikol added 2 commits January 9, 2024 17:22

Merge remote-tracking branch 'origin/main' into mde/sam-contract

48ffb3a

fix: change mapping for an array to make storage cheaper

26f1c7a

pcarranzav reviewed Jan 10, 2024

View reviewed changes

packages/contracts/contracts/rewards/SubgraphAvailabilityManager.sol Outdated Show resolved Hide resolved

fix: update comments

eeeb314

fix: make currentNonce public

1ad7935

pcarranzav previously approved these changes Jan 16, 2024

View reviewed changes

Maikol and others added 8 commits February 1, 2024 15:08

fix: added input validation for executionThreshold (OZ L-01)

fbdf6af

docs: added missing docstrings for events (OZ L-03)

94e6d05

fix: change OracleVote event order (OZ N-01)

9f9db2e

fix: gas optimization (OZ N-02)

f2aa2fa

fix: clear vote for earlier rounds (OZ L-02)

a52066c

fix: change to if/else

1ce46f0

fix: remove setDeniedMany since it's unused (OZ N-03)

ef35364

Merge pull request #935 from graphprotocol/mde/pr935-fix-oz-l-01

9562b28

fix: added input validation for executionThreshold (OZ L-01)

Maikol dismissed pcarranzav’s stale review via 9562b28 February 20, 2024 13:16

Maikol and others added 7 commits February 21, 2024 00:18

Merge pull request #936 from graphprotocol/mde/pr936-fix-oz-l-03

fa9d0cb

docs: added missing docstrings for events (OZ L-03)

Merge pull request #937 from graphprotocol/mde/pr937-fix-oz-n-01

0c547c4

fix: change OracleVote event order (OZ N-01)

Merge pull request #938 from graphprotocol/mde/pr938-fix-oz-n-02

0698989

fix: gas optimization (OZ N-02)

Merge pull request #939 from graphprotocol/mde/pr939-fix-oz-l-02

374e387

fix: clear vote for earlier rounds (OZ L-02)

Merge pull request #940 from graphprotocol/mde/pr400-fix-ox-n-03

58463a8

fix: remove setDeniedMany since it's unused (OZ N-03)

Merge branch 'main' into mde/sam-contract

06885f3

fix: indentation

bcf6957

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: subgraph availability manager contract #882

feat: subgraph availability manager contract #882

Maikol commented Nov 16, 2023

openzeppelin-code bot commented Nov 16, 2023 •

edited

codecov bot commented Nov 16, 2023 •

edited

tmigone left a comment

pcarranzav left a comment

pcarranzav commented Dec 8, 2023

Maikol commented Dec 8, 2023

pcarranzav left a comment

tmigone left a comment

tmigone Dec 21, 2023

Maikol Dec 21, 2023

tmigone Jan 2, 2024

Maikol Jan 5, 2024

pcarranzav Jan 9, 2024

Maikol Jan 9, 2024

pcarranzav Jan 9, 2024

feat: subgraph availability manager contract #882

Are you sure you want to change the base?

feat: subgraph availability manager contract #882

Conversation

Maikol commented Nov 16, 2023

openzeppelin-code bot commented Nov 16, 2023 • edited

feat: subgraph availability manager contract

🚨 Report Summary

codecov bot commented Nov 16, 2023 • edited

Codecov Report

tmigone left a comment

Choose a reason for hiding this comment

pcarranzav left a comment

Choose a reason for hiding this comment

pcarranzav commented Dec 8, 2023

Maikol commented Dec 8, 2023

pcarranzav left a comment

Choose a reason for hiding this comment

tmigone left a comment

Choose a reason for hiding this comment

tmigone Dec 21, 2023

Choose a reason for hiding this comment

Maikol Dec 21, 2023

Choose a reason for hiding this comment

tmigone Jan 2, 2024

Choose a reason for hiding this comment

Maikol Jan 5, 2024

Choose a reason for hiding this comment

pcarranzav Jan 9, 2024

Choose a reason for hiding this comment

Maikol Jan 9, 2024

Choose a reason for hiding this comment

pcarranzav Jan 9, 2024

Choose a reason for hiding this comment

openzeppelin-code bot commented Nov 16, 2023 •

edited

codecov bot commented Nov 16, 2023 •

edited