L2: linear rewards, and minting in L2 #700

pcarranzav · 2022-09-01T19:14:21Z

No description provided.

codecov · 2022-09-01T21:53:30Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.07 🎉

Comparison is base (3fe26fa) 91.59% compared to head (dc40106) 91.66%.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #700      +/-   ##
==========================================
+ Coverage   91.59%   91.66%   +0.07%     
==========================================
  Files          42       41       -1     
  Lines        1999     2016      +17     
  Branches      361      365       +4     
==========================================
+ Hits         1831     1848      +17     
  Misses        168      168

Flag	Coverage Δ
unittests	`91.66% <100.00%> (+0.07%)`	⬆️

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

Impacted Files	Coverage Δ
contracts/gateway/L1GraphTokenGateway.sol	`100.00% <100.00%> (ø)`
contracts/rewards/RewardsManager.sol	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

abarmat

I've did a first pass on the RewardsManager, haven't checked the tests nor the Gateway yet. Overall, I'm loving how simple it looking

abarmat · 2022-09-07T16:56:49Z

contracts/gateway/L1GraphTokenGateway.sol

+     * @param _block Block at which allowance will be computed
+     * @return The accumulated GRT amount that can be minted from L2 at the specified block
+     */
+    function accumulatedL2MintAllowanceAtBlock(uint256 _block) public view returns (uint256) {


What do you think if we use _blockNum to make it consistent with the _updateBlockNum in updateL2MintAllowance()?

abarmat · 2022-09-07T17:02:07Z

contracts/rewards/RewardsManager.sol

@@ -209,28 +202,18 @@ contract RewardsManager is RewardsManagerV3Storage, GraphUpgradeable, IRewardsMa
            return 0;
        }

-        // Zero issuance under a rate of 1.0


Just an observation: we could still keep this check to avoid the CALL to graphToken() and curation() contract if issuance rate is zero. However, as we are deploying it in L2 initially, it will be cheaper and don't have a big effect.

Good point, adding it back

abarmat · 2022-09-07T17:02:16Z

contracts/rewards/RewardsManager.sol

-
-        // New issuance of tokens during time steps
-        uint256 x = a.sub(p);
+        uint256 x = issuancePerBlock.mul(t);


I love how simple this is

abarmat · 2022-09-07T17:02:48Z

contracts/rewards/RewardsManager.sol

    using SafeMath for uint256;

-    uint256 private constant TOKEN_DECIMALS = 1e18;
-    uint256 private constant MIN_ISSUANCE_RATE = 1e18;
+    uint256 private constant FIXED_POINT_SCALING_FACTOR = 1e18;


This name is more precise

Yeah, the auditors had commented TOKEN_DECIMALS could be confusing in #571 so I used this name there as well

abarmat · 2022-09-07T17:03:33Z

contracts/rewards/RewardsManager.sol

     * Called from the Curation contract on mint() and burn()
     * @return Accumulated rewards per signal
     */
    function updateAccRewardsPerSignal() public override returns (uint256) {
        accRewardsPerSignal = getAccRewardsPerSignal();
        accRewardsPerSignalLastBlockUpdated = block.number;
-        tokenSupplySnapshot = graphToken().totalSupply();


And we could get rid of this 👍

abarmat · 2022-09-07T17:04:10Z

contracts/rewards/RewardsManagerStorage.sol

@@ -8,7 +8,7 @@ import "../governance/Managed.sol";
 contract RewardsManagerV1Storage is Managed {
    // -- State --

-    uint256 public issuanceRate;
+    uint256 public issuanceRateDeprecated;


Small optimization: we can make this variable private to avoid including a getter in the bytecode

abarmat

Just some small comments

abarmat · 2022-09-07T20:48:03Z

test/gateway/l1GraphTokenGateway.test.ts

+        await expect(
+          await l1GraphTokenGateway.accumulatedL2MintAllowanceAtBlock(await latestBlock()),
+        ).to.eq(issuancePerBlock.mul(3))
+        await expect(await l1GraphTokenGateway.accumulatedL2MintAllowanceSnapshot()).to.eq(0)


We can remove the outer await as the internal one returns the promised value

abarmat · 2022-09-07T20:49:55Z

test/gateway/l1GraphTokenGateway.test.ts

+          .emit(l1GraphTokenGateway, 'L2MintAllowanceUpdated')
+          .withArgs(snapshotValue, issuancePerBlock, issuanceUpdatedAtBlock)
+        // Now the mint allowance should be 10 + issuancePerBlock * 3
+        await expect(


no-op await

abarmat · 2022-09-07T20:56:42Z

test/gateway/l1GraphTokenGateway.test.ts

+            toBN('0'),
+            encodedCalldata,
+          )
+        await expect(tx)


You should be able to combine this like:

await expect(tx) .emit(l1GraphTokenGateway, 'WithdrawalFinalized') .withArgs(grt.address, l2Receiver.address, tokenSender.address, toBN('0'), toGRT('18')) .emit(l1GraphTokenGateway, 'TokensMintedFromL2') .withArgs(toGRT('8'))

…hange

…Block

…consistency

…ent assertions

#700)

…OZ L-01 from #700)

The base branch was changed.

Conflicts fixed for: - contracts/gateway/L1GraphTokenGateway.sol - test/gateway/l1GraphTokenGateway.test.ts - test/rewards/rewards.test.ts

…e-dev-2 linear rewards: merge dev and fix goerli config

config/graph.mainnet.yml

abarmat

⚡️

tmigone

🚀

pcarranzav force-pushed the pcv/l2-bridge branch from 2ab1048 to e0e2a88 Compare September 1, 2022 19:16

pcarranzav force-pushed the pcv/l2-linear-rewards branch from f0f91ad to 31e0a55 Compare September 1, 2022 19:17

pcarranzav mentioned this pull request Sep 1, 2022

[Experimental] Arbitrum bridge + rewards distribution: alternative approach #697

Closed

pcarranzav force-pushed the pcv/l2-linear-rewards branch 2 times, most recently from d8d2f74 to ba07101 Compare September 1, 2022 21:29

tmigone added the L2 label Sep 2, 2022

pcarranzav force-pushed the pcv/l2-linear-rewards branch from ba07101 to 8b9efa8 Compare September 2, 2022 19:05

tmigone force-pushed the pcv/l2-bridge branch 2 times, most recently from ae18852 to 5073920 Compare September 7, 2022 10:15

pcarranzav force-pushed the pcv/l2-linear-rewards branch from 6d96ff5 to 08617fa Compare September 7, 2022 16:48

abarmat reviewed Sep 7, 2022

View reviewed changes

abarmat previously approved these changes Sep 7, 2022

View reviewed changes

tmigone force-pushed the pcv/l2-bridge branch from 8c8f6aa to 3f6c55e Compare September 23, 2022 18:19

pcarranzav force-pushed the pcv/l2-linear-rewards branch from 7a8f68b to d8d009b Compare September 26, 2022 16:59

tmigone mentioned this pull request Sep 26, 2022

L2 testnet branch #659

Closed

pcarranzav force-pushed the pcv/l2-linear-rewards branch from d8d009b to 0f9bca8 Compare September 27, 2022 00:36

pcarranzav marked this pull request as ready for review September 27, 2022 00:44

pcarranzav added 11 commits September 28, 2022 14:43

feat: linear indexer rewards

1142934

feat: minting in L1 gateway with allowance

5d20c86

test: avoid issues with epoch boundary in allocation tests

cc4a2dd

chore: add comment on setIssuancePerBlock to remind of L1 allowance c…

51cc8cd

…hange

fix(RewardsManager): skip calculations if issuance is zero

06b9774

fix(RewardsManagerStorage): make deprecated variables private

bbe2cff

fix(cli): replace issuanceRate getter/setter with one for issuancePer…

b77f501

…Block

fix(e2e): issuanceRate replaced by issuancePerBlock

5124371

chore(L1GraphTokenGateway): rename _block parameter to _blockNum for …

4f2262d

…consistency

test(L1GraphTokenGateway): remove redundant awaits and combine two ev…

6351786

…ent assertions

fix: add RewardsManager as minter in L2

263355d

pcarranzav force-pushed the pcv/l2-linear-rewards branch from 0f9bca8 to 263355d Compare September 29, 2022 12:20

abarmat added the protocol change label Sep 30, 2022

pcarranzav added a commit that referenced this pull request Oct 20, 2022

fix: prevent unpausing gateways if they're not configured (OZ M-01 from

1d5dde2

#700)

pcarranzav mentioned this pull request Oct 20, 2022

fix: prevent unpausing gateways if they're not configured (OZ M-01 from #700) #733

Merged

pcarranzav added a commit that referenced this pull request Oct 20, 2022

fix: validate Arbitrum addresses and callhook senders are contracts (…

2e0038b

…OZ L-01 from #700)

pcarranzav added a commit that referenced this pull request Oct 21, 2022

fix: validate Arbitrum addresses and callhook senders are contracts (…

16dc44e

…OZ L-01 from #700)

pcarranzav added a commit that referenced this pull request Oct 21, 2022

fix: remove unnecessary L2 gas check (OZ N-01 from #700)

885013b

pcarranzav added a commit that referenced this pull request Oct 21, 2022

fix: remove unnecessary L2 gas check (OZ N-01 from #700)

2dfacb6

pcarranzav added 2 commits October 21, 2022 12:10

fix: use strict < for block in L2 mint allowance (OZ N-02)

dbdda2f

fix: remove the deprecated pow from RewardsManager (OZ N-03)

1845259

pcarranzav changed the base branch from pcv/l2-bridge to dev November 24, 2022 00:40

Merge branch 'dev' into pcv/l2-linear-rewards

f340c3a

Conflicts fixed for: - contracts/gateway/L1GraphTokenGateway.sol - test/gateway/l1GraphTokenGateway.test.ts - test/rewards/rewards.test.ts

pcarranzav mentioned this pull request Nov 24, 2022

Merge dev into pcv/l2-linear-rewards #752

Merged

pcarranzav added 3 commits December 23, 2022 12:15

Merge branch 'dev' into pcv/l2-linear-rewards-merge-dev-2

ce1de68

fix: set issuancePerBlock on goerli config

69e542d

Merge pull request #780 from graphprotocol/pcv/l2-linear-rewards-merg…

c0383e1

…e-dev-2 linear rewards: merge dev and fix goerli config

pcarranzav added the audited label Feb 3, 2023

tmigone previously approved these changes Mar 9, 2023

View reviewed changes

config/graph.mainnet.yml Show resolved Hide resolved

config/graph.mainnet.yml Show resolved Hide resolved

fix(config): set L1 gateway as minter on localhost and goerli

dc40106

pcarranzav dismissed tmigone’s stale review via dc40106 March 15, 2023 14:40

pcarranzav mentioned this pull request Mar 15, 2023

Scratch testnet 3 (for testing only) #800

Closed

abarmat approved these changes Mar 27, 2023

View reviewed changes

tmigone approved these changes Mar 27, 2023

View reviewed changes

pcarranzav merged commit 23611af into dev Mar 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

L2: linear rewards, and minting in L2 #700

L2: linear rewards, and minting in L2 #700

pcarranzav commented Sep 1, 2022

codecov bot commented Sep 1, 2022 •

edited

Loading

abarmat left a comment

abarmat Sep 7, 2022

abarmat Sep 7, 2022

pcarranzav Sep 7, 2022

abarmat Sep 7, 2022

abarmat Sep 7, 2022

pcarranzav Sep 7, 2022

abarmat Sep 7, 2022

abarmat Sep 7, 2022

abarmat left a comment

abarmat Sep 7, 2022

abarmat Sep 7, 2022

abarmat Sep 7, 2022

abarmat left a comment

tmigone left a comment

L2: linear rewards, and minting in L2 #700

L2: linear rewards, and minting in L2 #700

Conversation

pcarranzav commented Sep 1, 2022

codecov bot commented Sep 1, 2022 • edited Loading

Codecov Report

abarmat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abarmat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abarmat left a comment

Choose a reason for hiding this comment

tmigone left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 1, 2022 •

edited

Loading