threshold-network / merkle-distribution

Threshold Network rewards generation and distribution
https://threshold.network
1 stars 6 forks source link

New MerkleDistribution contract that can trigger withdrawing TACoApp rewards #136

Open cygnusv opened 4 months ago

cygnusv commented 4 months ago

Once we introduce rewards in the TACoApp contract, this will produce a fragmentation of the rewards between other apps (tBTC & RB, currently on the MerkleDistribution contract) and TACo. In order to eliminate this friction from a staker perspective, we should allow the MerkleDistribution contract to trigger withdrawing rewards to the beneficiary.

In addition to the changes mentioned in https://github.com/nucypher/nucypher-contracts/issues/244 to the TACoApp, we'd need a new MerkleDistribution contract that performs this call. This issue includes also the migration of approvals from the ClaimableRewards proxy contract owned by Threshold.

manumonti commented 1 week ago

The current approach is to have three claim functions that a stake can call:

But this can be problematic:

The claim(...) function has as arguments those values related to a Merkle distribution: Merkle root, Merkle proof, and cumulative amount. But it may happen that a stake only runs a TACo application, so this stake won't have tBTC rewards. If no tBTC rewards are calculated, this stake won't be part of the Merkle distribution, so there won't be a Merkle proof to be passed as an argument to the claim function.

Two solutions are possible (if not more):

cc @cygnusv

manumonti commented 1 week ago

About claiming from the dashboard:

The idea is that the dashboard calls the claim() function in the RewardsAggregator contract. This function will:

  1. Send the Merkle rewards (i.e. tBTC rewards)
  2. Call the application's withdrawRewards() function (TACo at this moment). This function will send to the beneficiary the TACo rewards.

But what happens if the stake is not earning TACo rewards at all? This require will not be satisfied so the transaction will be reverted, and this leads to a bad user experience. Am I right?

Can we adapt the dashboard to ignore this? Probably not, because the revert message will be shown in the wallet (Metamask or whatever) and not in the dashboard, right?

cc @cygnusv

EDIT: solved by adding canClaimApps() function: https://github.com/threshold-network/merkle-distribution/commit/f20637415c8c9661763d17913d66e31197811e60