add stake-cw20-external-rewards #170

ben2x4 · 2022-02-09T00:31:47Z

This contract distributes external staking rewards to stakers in the stake-cw20 contract. External means the rewards is either a native or cw20 that is not the same as the cw20 being staked.

The contract calculates rewards based on blocks but only distributes them at predefined epochs. This is all configurable in the instatiation message.

0xekez

Looking good :) Just couple minor comments

contracts/stake-cw20-external-rewards/Cargo.toml

0xekez · 2022-02-09T02:02:35Z

contracts/stake-cw20-external-rewards/src/contract.rs

+    if env.block.height > msg.start_block {
+        return Err(ContractError::StartBlockAlreadyOccurred {});
+    }
+    if msg.start_block > msg.end_block {


Should this be >=? Currently could instantiate a contract that starts and ends at the same block and it would be valid so long as that block was a multiple of payment_block_delta.

yeah, this would would result in one payment. Probable not the best practice and a seperate contract should be written for these cases, but nothing invalid about this.

Is there a test case for this? If not we should make a TODO / issue. Can be added in a separate PR.

contracts/stake-cw20-external-rewards/src/contract.rs

0xekez · 2022-02-09T02:20:33Z

contracts/stake-cw20-external-rewards/src/contract.rs

+    Ok(match config.clone().denom {
+        Denom::Native(denom) => BankMsg::Send {
+            to_address: info.sender.to_string(),
+            amount: vec![Coin { amount, denom }],
+        }
+        .into(),


Suggested change

Ok(match config.clone().denom {

Denom::Native(denom) => BankMsg::Send {

to_address: info.sender.to_string(),

amount: vec![Coin { amount, denom }],

}

.into(),

Ok(match &config.denom {

Denom::Native(denom) => BankMsg::Send {

to_address: info.sender.to_string(),

amount: vec![Coin { amount, denom: denom.to_string() }],

}

.into(),

contracts/stake-cw20-external-rewards/src/contract.rs

vernonjohnson · 2022-02-09T02:43:38Z

contracts/stake-cw20-external-rewards/src/contract.rs

+) -> Result<Response, ContractError> {
+    validate_instantiate_msg(&env, &msg)?;
+    let config = Config {
+        start_block: msg.start_block,


What's the rationale for having the user pass in start_block, end_block, and payment per block? It would be more elegant to accept a duration and an amount, and then calculate payment per unit time within the contract. I do something similar here.

I like that the parameters are explicit and the instatiator is required to double check everything will function as expected.

contracts/stake-cw20-external-rewards/src/contract.rs

vernonjohnson · 2022-02-09T02:51:25Z

contracts/stake-cw20-external-rewards/src/contract.rs

+    if config.denom != Denom::Cw20(info.sender) {
+        return Err(ContractError::IncorrectDenom {});
+    }
+    if amount != config.total_amount {


Maybe change to amount >= config.total_amount, and refund any extra funds that were sent

I really like require the instatiator to be precise with their inputs. Failing the instatiate requires them to revaluate the inputs and ensure they are doing what they intend to do.

Personally, agree with @ben2x4 on this one.

vernonjohnson · 2022-02-09T02:55:21Z

contracts/stake-cw20-external-rewards/src/contract.rs

+    let delta = config.payment_block_delta;
+    let mut current_block = last_claimed;
+    let mut amount = Uint128::zero();
+    while current_block <= min(up_to_block, config.end_block) {


Oh I see. ClaimUpToBlock can be used to prevent out of gas issue here

vernonjohnson · 2022-02-09T03:12:15Z

contracts/stake-cw20-external-rewards/src/contract.rs

+    amount: Uint128,
+) -> Result<Response, ContractError> {
+    let mut config = CONFIG.load(deps.storage)?;
+    if config.funded {


Shouldn't you be able to top up this contract whenever the rewards get depleted. Otherwise, you'd have to instantiate a new contract every time you want to allocate more rewards

yeah this is a good additional feature

vernonjohnson · 2022-02-09T03:24:02Z

This is a solid start. There are a few major issues though:

The contract has to loop over every unclaimed block height to redeem all rewards for an account. So stakers would potentially have to submit several transactions to claim all of their rewards (otherwise their transactions will run out of gas). This UX is not ideal IMO. This is primarily due to the limitations of querying the stake cw20 contract, which indexes staking info based on block height. If you want to accommodate more traditional staking mechanisms (similar to Synthetix staking rewards), which provide better UX, you may want to consider a different staking contract implementation.
The contract can't be topped up, so you can't allocate more funds to issue additional rewards. You'd have to deploy an entirely new rewards contract to do so.

Co-authored-by: zeke <[email protected]>

ben2x4 · 2022-02-10T03:05:22Z

This is a solid start. There are a few major issues though:

The contract has to loop over every unclaimed block height to redeem all rewards for an account. So stakers would potentially have to submit several transactions to claim all of their rewards (otherwise their transactions will run out of gas). This UX is not ideal IMO. This is primarily due to the limitations of querying the stake cw20 contract, which indexes staking info based on block height. If you want to accommodate more traditional staking mechanisms (similar to Synthetix staking rewards), which provide better UX, you may want to consider a different staking contract implementation.

The contract can't be topped up, so you can't allocate more funds to issue additional rewards. You'd have to deploy an entirely new rewards contract to do so.

I agree that point 2 is a great feature to implement before production deployment.

In regards to point 1, need to look a little more deeply into how they implement this. Would love to chat.

…ao into stake-cw20-external-rewards

JakeHartnell · 2022-02-11T21:14:40Z

Can we please rebase this branch to make it easier to merge in PRs potentially? 🙏

JakeHartnell · 2022-02-11T21:15:39Z

contracts/stake-cw20-external-rewards/LICENSE

+      same "printed page" as the copyright notice for easier
+      identification within third-party archives.
+
+   Copyright [yyyy] [name of copyright owner]


Update this to be either your name or DAO DAO please. : )

JakeHartnell · 2022-02-11T21:16:35Z

contracts/stake-cw20-external-rewards/README.md

@@ -0,0 +1,108 @@
+# CosmWasm Starter Pack


The most basic README would be nice. Can remove all of this template stuff.

JakeHartnell · 2022-02-11T21:33:32Z

contracts/stake-cw20-external-rewards/src/contract.rs

+    let (amount, _) = get_amount_owed(
+        deps,
+        &address,
+        &config,
+        last_claimed.block_height,
+        env.block.height,
+    )?;


This could get expensive?

donkey-donkey · 2022-02-17T18:59:52Z

have you all checked out the repo from cw20 - reward. its just like anchor on terra and yield protocol on ethereum
https://github.com/CosmWasm/cw-tokens/tree/cw20-reward/contracts/cw20-reward

vernonjohnson · 2022-02-17T20:44:30Z

have you all checked out the repo from cw20 - reward. its just like anchor on terra and yield protocol on ethereum https://github.com/CosmWasm/cw-tokens/tree/cw20-reward/contracts/cw20-reward

Ben also needs it to support multiple reward tokens for a single staking contract. But I agree, this is a good start.

ben2x4 · 2022-03-06T21:37:39Z

This approach is flawed, implementing this with a different method

ben2x4 added 3 commits February 8, 2022 16:29

add stake-cw20-external-rewards

1a821f7

fix clippy issues

f2785e6

cargo fmt

81545b3

0xekez reviewed Feb 9, 2022

View reviewed changes

contracts/stake-cw20-external-rewards/src/contract.rs Outdated Show resolved Hide resolved

0xekez reviewed Feb 9, 2022

View reviewed changes

contracts/stake-cw20-external-rewards/src/contract.rs Show resolved Hide resolved

vernonjohnson reviewed Feb 9, 2022

View reviewed changes

contracts/stake-cw20-external-rewards/src/contract.rs Outdated Show resolved Hide resolved

vernonjohnson reviewed Feb 9, 2022

View reviewed changes

ben2x4 and others added 2 commits February 9, 2022 18:42

remove unnecssary profile.release in cargo.toml

7f3f5fa

Co-authored-by: zeke <[email protected]>

_env -> env

4d69b91

Co-authored-by: zeke <[email protected]>

ben2x4 added 4 commits February 9, 2022 19:13

respond to comments

9f044c1

cargo fmt

e2dfbdf

cargo schema

260d7d0

Merge branch 'stake-cw20-external-rewards' of github.com:DA0-DA0/cw-d…

32c1eb5

…ao into stake-cw20-external-rewards

JakeHartnell reviewed Feb 11, 2022

View reviewed changes

JakeHartnell mentioned this pull request Feb 12, 2022

Shareholder Distribution Contract #57

Closed

0xekez mentioned this pull request Feb 13, 2022

Stake CW20 Rewards #173

Merged

ben2x4 closed this Mar 6, 2022

JakeHartnell deleted the stake-cw20-external-rewards branch November 21, 2022 23:40

JakeHartnell restored the stake-cw20-external-rewards branch November 21, 2022 23:41

JakeHartnell deleted the stake-cw20-external-rewards branch August 20, 2023 03:16

JakeHartnell restored the stake-cw20-external-rewards branch August 20, 2023 03:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add stake-cw20-external-rewards #170

add stake-cw20-external-rewards #170

ben2x4 commented Feb 9, 2022

0xekez left a comment

0xekez Feb 9, 2022

ben2x4 Feb 9, 2022

JakeHartnell Feb 11, 2022

0xekez Feb 9, 2022

JakeHartnell Feb 11, 2022

vernonjohnson Feb 9, 2022

ben2x4 Feb 10, 2022

vernonjohnson Feb 9, 2022

ben2x4 Feb 10, 2022

JakeHartnell Feb 11, 2022

vernonjohnson Feb 9, 2022

vernonjohnson Feb 9, 2022

ben2x4 Feb 10, 2022

vernonjohnson commented Feb 9, 2022 •

edited

Loading

ben2x4 commented Feb 10, 2022

JakeHartnell commented Feb 11, 2022

JakeHartnell Feb 11, 2022

JakeHartnell Feb 11, 2022

JakeHartnell Feb 11, 2022

donkey-donkey commented Feb 17, 2022

vernonjohnson commented Feb 17, 2022

ben2x4 commented Mar 6, 2022

add stake-cw20-external-rewards #170

add stake-cw20-external-rewards #170

Conversation

ben2x4 commented Feb 9, 2022

0xekez 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vernonjohnson commented Feb 9, 2022 • edited Loading

ben2x4 commented Feb 10, 2022

JakeHartnell commented Feb 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donkey-donkey commented Feb 17, 2022

vernonjohnson commented Feb 17, 2022

ben2x4 commented Mar 6, 2022

vernonjohnson commented Feb 9, 2022 •

edited

Loading