Skip to content

Conversation

sunmoon11100
Copy link
Contributor

@sunmoon11100 sunmoon11100 commented Aug 15, 2022

The purpose of gauge votes is to determine which pools receive emission allocations for each epoch. Each epoch ranges over a one week period Wed 5pm PST - Wed 5:59PM PST the following week.

If user vote for pools offering bribes, he will receive a portion of these bribes proportionate to his vote weight / global vote weights on that pool.

https://resources.curve.fi/reward-gauges/understanding-gauges
https://andrecronje.medium.com/bribing-vecrv-gauges-101-8f6e4506bb62
https://docs.maiadao.io/hermes-token/claiming-trading-fees-bribes
https://templecodex.link/eli5-vetokens-gauges-and-bribes/

@mkurnikov
Copy link
Contributor

mkurnikov commented Aug 16, 2022

@sunmoon11100 Could you explain the purpose of splitting the module? It's an additional complexity of having to think about access permissions, without any benefits in this case, as pool_vote_id and gauge is inherently dependent on each other.

UPD. Just because those has been split in the Solidity version, doesn't mean it has to be in Move, we have much stricter scoping rules.

It looks like bribe.move does need to be in a separate module though, as it's an entirely different functionality.

let item = iterable_table::borrow(&votes.items, pool_id);
let voted = iterable_table::borrow(item, ve_token_id);
let pool_rewards = gauge::get_rewards(pool_id);
gauge::withdraw_rewards(pool_id, pool_rewards * *voted / *total_voted )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhere here there's a problem: we have only integers, so the sum of all
pool_rewards * *voted / * total_voted over the all ve_nft tokens in the pool voting table can be a bit less than pool_rewards. We need to do something with that last bit of rewards, maybe give it to the last user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining rewards are summed and distributed again next week, so it's not obligatory.

@borispovod borispovod changed the base branch from main to staking August 18, 2022 16:24
sunmoon11100 and others added 10 commits August 18, 2022 20:02
* Covered each functions with negative and positive scenarios.
Added comments to code.

* Changed amount of test coins near to real values.

* Changed comment for deposit function
* fix: add more tests for getter functions

* fix: remove debug dep

* fix: resolve PR comments
use test_helpers::test_account::create_account;
use test_pool_owner::test_lp::LP;

#[test(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs much more tests, really as much tests as possible!

# Conflicts:
#	Move.toml
#	sources/swap/dao_storage.move
#	sources/swap/liquidity_pool.move
#	tests/dao_storage_tests.move
#	tests/liquidity_pool_tests.move
#	tests/router_tests.move
Copy link
Member

@borispovod borispovod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also won't compile with latest Aptos CLI, fix please.

pool_owner = @test_pool_owner,
gov_admin = @gov_admin
)]
fun test_vote_for_liquidity_pool(coin_admin: signer, pool_owner: signer, gov_admin: signer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still small amount of tests.

@mkurnikov
Copy link
Contributor

@sunmoon11100 Could you add a high-level explanation of how it should work on top of files?

# Conflicts:
#	sources/swap/liquidity_pool.move
#	tests/liquidity_pool_tests.move
@sunmoon11100 sunmoon11100 changed the title Gauges and bribes [WIP] Gauges and bribes Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants