New Threshold rewards distribution contract #149

cygnusv · 2024-05-16T15:03:16Z

A new Rewards distribution contract is developed as part of the effort to develop an automatic reward mechanism.

The previous contract

CumulativeMerkleDrop contract (aka Merkle Distribution contract) has proven to be useful since the release of the first Threshold Network rewards distribution in July 2022. Since then, every month a reward calculation script has been executed, generating a Merkle tree with the rewards earned by each stake. The Merkle root of this tree is updated on the contract every time a new Merkle tree is generated, so then, stakers can claim their rewards by calling the appropriate method on the contract.

Although useful, this process is pretty manual and error-prone, so a new mechanism to distribute the rewards is being developed.

The new contract: `RewardsAggregator`

Note that in the Threshold Network, each application (TACo and tBTC/RandomBeacon) implements its own reward distribution mechanism. The first application to implement automatic distribution is TACo.

So, this new rewards distribution contract has the ability to allow the stakers to claim the rewards from two different sources:

tBTC/RandomBeacon using the usual mechanism of generating a new Merkle reward distribution periodically (monthly).
TACo application. The stake will receive the rewards generated from the last time they claimed the rewards until the moment of this new claim.

Work done:

Developed the new reward distribution contract RewardsAggregator.sol.
Added tests for checking the behavior of this new contract.
Applied formatting to existing tests and contracts.

Running the tests

npm install
npx hardhat compile
npx hardhat test

…nterface

cygnusv

Some initial comments. A lot to unfold here!

cygnusv · 2024-09-05T15:44:07Z

contracts/RewardsAggregator.sol

+/**
+ * @title Rewards Aggregator
+ * @notice RewardsAggregator is the contract responsible for the distribution
+ *         of the Threshold Network staking rewards. The rewards are generated


Suggested change

* of the Threshold Network staking rewards. The rewards are generated

* of Threshold Network staking rewards, which are generated

✅ addressed on 28df6ba

cygnusv · 2024-09-06T10:33:37Z

contracts/RewardsAggregator.sol

@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.9;


@vzotova Do you think we should increase the version here?

good question, I'd use same as in threshold staking (which is 0.8.9 not ^0.8.9) or latest available

✅ addressed on 6aa0a8e

cygnusv · 2024-09-06T10:38:20Z

package.json

    "prettier": "2.7.1",
    "typescript": "^4.8.4"
  },
  "dependencies": {
    "@octokit/core": "^5.0.2",
    "@thesis/solidity-contracts": "github:thesis/solidity-contracts#4985bcf",
+    "@threshold-network/solidity-contracts": "github:threshold-network/solidity-contracts#19afdea",


Can we use an NPM dependency?

✅ Addressed on 747b3fa

cygnusv · 2024-09-06T10:40:00Z

contracts/RewardsAggregator.sol

+
+import "@openzeppelin/contracts/access/Ownable.sol";
+import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol";


I don't see the open zeppelin contract dependency in package.json.

✅ addressed on dd53240

cygnusv · 2024-09-06T10:41:42Z

contracts/RewardsAggregator.sol

+import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol";
+import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import "@threshold-network/solidity-contracts/contracts/staking/IApplication.sol";


If this is the only thing we're using from this repo, I'd suggest we just copy the interface here and remove the dependency, so we don't pollute ours with their subdependencies. In fact, that would allow us to use OZ v5, otherwise we'd be stuck with v4.

✅ Addressed on 747b3fa

Note that we are using v4 of OpenZeppelin contracts because v5 is not compatible with CumulativeMerkleDrop contract.

contracts/RewardsAggregator.sol

contracts/interfaces/IRewardsAggregator.sol

contracts/RewardsAggregator.sol

derekpierre

Some outstanding RFCs from @cygnusv . Once addressed, I'm happy to approve.

derekpierre

🎸 - nice work! One minor question/comment.

derekpierre · 2024-09-11T15:07:15Z

contracts/RewardsAggregator.sol

+        oldCumulativeMerkleDrop = _oldCumulativeMerkleDrop;
+    }
+
+    function setMerkleRoot(bytes32 merkleRoot_) external override onlyOwner {


Did you want to keep the _ prefix consistent here, and in the function below as well i.e. _merkleRoot , and _rewardsHolder?

Good catch. Done in a390c28 ✅

cygnusv · 2024-09-16T09:02:47Z

Since I'm the original pr author I can't do it explicitly, but approved! Let's test it out on testnet 💪

cygnusv added 4 commits May 16, 2024 16:42

Add Threshold IApplication interface

584e57a

Add IApplication reference in constructor

ad1311d

Add methods that trigger withdraw in application

3bd7087

Add TACo app to deployment script

32e4131

cygnusv mentioned this pull request May 16, 2024

Transition Path to Automatic Rewards Distribution & Penalizations nucypher/nucypher-contracts#268

Open

manumonti self-assigned this May 21, 2024

manumonti added 4 commits June 11, 2024 15:06

Fix taco app mainnet address

ee547c3

Add TACo app address for Lynx testnet

a4f6b71

Replace Merkle verify function for OZ one

63d1651

Format code

29a80a2

manumonti force-pushed the with-taco branch from 1af8c83 to 29a80a2 Compare June 12, 2024 06:40

manumonti added 12 commits June 12, 2024 08:40

Add commit to .git-blame-ignore-revs

b1840d2

Rename Merkle Distribution contract

e4ed12b

Move mock contracts to test folder

b8add39

Remove test function syntax errors

a2cdc77

Add ignore-warnings support to compilations

9f737df

Add Application mock contract

0a47f40

Adapt test to new MerkleDistributor contract

d7b6cc6

Use imported IApplication instead of local copy

0cb969f

Add token transfer to ApplicationMock

f71afb0

Add MerkleDistributor deployment tests

64ff146

Change test file name

62d28d6

Add constructor parameter for application in tests

11b5af1

cygnusv mentioned this pull request Jun 17, 2024

Read and withdraw available TACo rewards from app contract threshold-network/token-dashboard#756

Open

cygnusv and others added 6 commits June 17, 2024 13:25

Introduce cumulativeClaimed view function in the IMerkleDistributor i…

a2355aa

…nterface

Consider claimed balance in old MerkleDistribution contract

1f29f4a

Rename claiming functions (withoutApps instead of withApps angle)

53ccbf8

Fix compilation errors

092580a

Add efficiency to tests not deploying repetitively

e61887c

Add old Merkle Distribution contract to tests

f4236f9

manumonti added 3 commits September 5, 2024 09:18

Add instructions to README

b6d8db5

Format code of contracts and tests files

48d5295

Add format commit to git-blame-ingore-revs

687da6d

manumonti linked an issue Sep 5, 2024 that may be closed by this pull request

New MerkleDistribution contract that can trigger withdrawing TACoApp rewards #136

Closed

manumonti changed the title ~~[WIP] Trigger withdraw rewards in TACo app~~ New Threshold rewards distribution contract Sep 5, 2024

manumonti marked this pull request as ready for review September 5, 2024 09:35

manumonti requested review from KPrasch, derekpierre, vzotova and theref September 5, 2024 09:36

Add inline comment to ignore warning

41a1c92

vzotova approved these changes Sep 5, 2024

View reviewed changes

Use functions on utils instead of redeclaring them

8846ab7

cygnusv commented Sep 6, 2024

View reviewed changes

derekpierre reviewed Sep 9, 2024

View reviewed changes

contracts/RewardsAggregator.sol Show resolved Hide resolved

derekpierre reviewed Sep 9, 2024

View reviewed changes

manumonti added 5 commits September 10, 2024 13:40

Set a fixed version for Solidity compiler

6aa0a8e

Fix comment typo

28df6ba

Add Merkle word to avoid confusions

7694f23

Unify notation

86c1ecc

Add explanatory comment to function

a906b84

manumonti force-pushed the with-taco branch from aa6f9b0 to a906b84 Compare September 11, 2024 08:59

manumonti added 4 commits September 11, 2024 12:38

Remove unused dependencies

69f3da9

Add IApplication file instead of Node dependency

747b3fa

Use NPM package instead of github ref for TokenMock

2f30fc2

Add NPM package for OpenZeppelin contracts

dd53240

derekpierre approved these changes Sep 11, 2024

View reviewed changes

Unify prefix notation

a390c28

cygnusv merged commit 3f1f33a into main Sep 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Threshold rewards distribution contract #149

New Threshold rewards distribution contract #149

cygnusv commented May 16, 2024 •

edited by manumonti

Loading

cygnusv left a comment

cygnusv Sep 5, 2024

manumonti Sep 10, 2024

cygnusv Sep 6, 2024

vzotova Sep 6, 2024

manumonti Sep 10, 2024

cygnusv Sep 6, 2024

manumonti Sep 11, 2024

cygnusv Sep 6, 2024

manumonti Sep 11, 2024

cygnusv Sep 6, 2024

manumonti Sep 11, 2024 •

edited

Loading

derekpierre left a comment

derekpierre left a comment

derekpierre Sep 11, 2024

manumonti Sep 11, 2024

cygnusv commented Sep 16, 2024

	* of the Threshold Network staking rewards. The rewards are generated
	* of Threshold Network staking rewards, which are generated

		@@ -0,0 +1,262 @@
		// SPDX-License-Identifier: MIT
		pragma solidity ^0.8.9;

New Threshold rewards distribution contract #149

New Threshold rewards distribution contract #149

Conversation

cygnusv commented May 16, 2024 • edited by manumonti Loading

The previous contract

The new contract: RewardsAggregator

Work done:

Running the tests

cygnusv 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manumonti Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

derekpierre left a comment

Choose a reason for hiding this comment

derekpierre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cygnusv commented Sep 16, 2024

cygnusv commented May 16, 2024 •

edited by manumonti

Loading

The new contract: `RewardsAggregator`

manumonti Sep 11, 2024 •

edited

Loading