Total mix stake accounting #794

durch · 2021-09-30T10:29:41Z

Adds total mix stake value to the contract, and threads it all the way to the validator-api. Total mix stake is increased on mixnode bonding and delegation and decreased on unbonding and undelegation.

implement total mix stake
implement total gateway stake
tests

* Sqlx struct stub * Initial schema * Initial error enum * Managed for persisted shared keys * Initial inbox manager * Comments * Using new database in clients handler * Extending gateway storage API * tokio::main + placeholder values * Removed old client store * Simplified logic of async packet processing * Renamed table + not null restriction * BandwidthManager * Removed sled dependency * Using centralised storage for bandwidth * Dead code removal * WIP connection_handler split and simplification Maybe it doesn't look like it right now, but once completed it will remove bunch of redundant checks for Nones etc * Further more explicit clients handler split * Minor cleanup * Temporary store for active client handles * Fixed error types * Error trait on iv and encrypted address * Authentication and registration moved to the handler * Removal of clients handler * Further logic simplification + returned explicit bandwidth values * Further cleanup and comments * Updated config with relevant changes * Basic bandwidth tracking in client * FreshHandle doc comments + fixed stagger issue * Removed side-effects from .map * More doc comments * Database migration on build * Increased default claimed bandwidth * Renaming * Fixed client determining available bandwidth * Removed dead sql table that might be used in the future * Windows workaround * Comment * Return error rather than cap credential

jstuczyn

Looks rather good to me - there are only some tiny thing I've mentioned in the code plus the additional two suggestions:

perhaps introduce the same concept for total gateway stake for consistency and because presumably you will need it for rewards calculations?
some tests to make sure the global value is always kept in sync in the contract, especially in the presence of the "shadow" delegations. I think it's fine, but I'd feel more confident if it was actually tested

contracts/mixnet/src/contract.rs

contracts/mixnet/src/queries.rs

contracts/mixnet/src/storage.rs

validator-api/src/cache/mod.rs

* Remove migration code * Added function to iterate over delegation of variable type * Add unit tests * Refactored some naming and reused mix/gateway functionality * Borrow bucket instead of move * Linked with existing delegations function * Migration of left-over delegations * Remove unused imports * Put a gateway test as well, next to the mix one * Expose queries for all delegations * Change break point * Added client side calls to the new queries * Fix clippy * Added pagination and read check tests * Fix gateway test from the last commit * Test functions for (de)serialization of identity and owner (in)to storage keys * Add delegation function unit test * Feature guard import * Changed UnpackedDelegation from type to struct * Remove mutable parameter and put start_after in returned value * Made all delegations into iterator for OOM safety * Fix clippy * Add test for delegations iterator size in memory * Change map with if let for ease of read * Use DENOM instead of hardcoded value

common/client-libs/validator-client/src/client.rs

common/client-libs/validator-client/src/nymd/mod.rs

jstuczyn · 2021-10-01T09:14:42Z

contracts/mixnet/src/contract.rs

-        reverse_gateway_delegations(deps.storage, &owner).save(node_identity.as_bytes(), &())?;
-    }
+pub fn migrate(_deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result<Response, ContractError> {
+    todo!("Calculate initial total mix and gateway stake after initial deployment");


Actually I never asked - is this going to be in another pull request or something? Because technically after this PR is merged, any other contract changes will be blocked until the migration is implemented

I think I'll merge this into the tokenomics branch first, then once we're ready to get that done we can preform the migration

jstuczyn · 2021-10-01T09:16:31Z

contracts/mixnet/src/storage.rs

+pub fn total_mix_stake_value(storage: &dyn Storage) -> Uint128 {
+    match total_mix_stake(storage).load() {
+        Ok(value) => value,
+        Err(_e) => Uint128(0),


Under what conditions can it produce an error? And if it produces an error isn't it technically some undefined behaviour?

This handles read in case no values have yet been written to storage, primarily for tests, although it will be useful for migration as well

jstuczyn · 2021-10-01T09:19:22Z

contracts/mixnet/src/storage.rs

+    amount: Uint128,
+    storage: &mut dyn Storage,
+) -> Result<Uint128, ContractError> {
+    let stake = total_mix_stake_value(storage).checked_add(amount)?;


Sorry to go back to this after we have already resolved it, but upon further thinking the original unwrap here is absolutely fine. The reason being is that it's impossible to have higher total mix stake than our entire token supply which is guaranteed to easily fit in an uint128

It resulted in much nicer code, so I think it was a good comment regardless :)

validator-api/src/cache/mod.rs

validator-api/src/nymd_client.rs

* Feature/persistent gateway storage (#784) * Sqlx struct stub * Initial schema * Initial error enum * Managed for persisted shared keys * Initial inbox manager * Comments * Using new database in clients handler * Extending gateway storage API * tokio::main + placeholder values * Removed old client store * Simplified logic of async packet processing * Renamed table + not null restriction * BandwidthManager * Removed sled dependency * Using centralised storage for bandwidth * Dead code removal * WIP connection_handler split and simplification Maybe it doesn't look like it right now, but once completed it will remove bunch of redundant checks for Nones etc * Further more explicit clients handler split * Minor cleanup * Temporary store for active client handles * Fixed error types * Error trait on iv and encrypted address * Authentication and registration moved to the handler * Removal of clients handler * Further logic simplification + returned explicit bandwidth values * Further cleanup and comments * Updated config with relevant changes * Basic bandwidth tracking in client * FreshHandle doc comments + fixed stagger issue * Removed side-effects from .map * More doc comments * Database migration on build * Increased default claimed bandwidth * Renaming * Fixed client determining available bandwidth * Removed dead sql table that might be used in the future * Windows workaround * Comment * Return error rather than cap credential * Feature/migrate hidden delegations (#786) * Remove migration code * Added function to iterate over delegation of variable type * Add unit tests * Refactored some naming and reused mix/gateway functionality * Borrow bucket instead of move * Linked with existing delegations function * Migration of left-over delegations * Remove unused imports * Put a gateway test as well, next to the mix one * Expose queries for all delegations * Change break point * Added client side calls to the new queries * Fix clippy * Added pagination and read check tests * Fix gateway test from the last commit * Test functions for (de)serialization of identity and owner (in)to storage keys * Add delegation function unit test * Feature guard import * Changed UnpackedDelegation from type to struct * Remove mutable parameter and put start_after in returned value * Made all delegations into iterator for OOM safety * Fix clippy * Add test for delegations iterator size in memory * Change map with if let for ease of read * Use DENOM instead of hardcoded value * Total mixnode stake accounting in the contract * GetTotalMixState query * Add total_mix_stake to validator api * cargo fmt * Handle errors properly * Gateway stake accounting * tests * Naming :) Co-authored-by: Jędrzej Stuczyński <jedrzej.stuczynski@gmail.com> Co-authored-by: Bogdan-Ștefan Neacşu <bogdan@nymtech.net> Co-authored-by: Drazen Urch <durch@users.noreply.guthub.com>

durch requested review from futurechimp, jstuczyn and neacsu as code owners September 30, 2021 10:29

jstuczyn requested changes Sep 30, 2021

View reviewed changes

neacsu and others added 7 commits September 30, 2021 15:04

Total mixnode stake accounting in the contract

6df23a8

GetTotalMixState query

314e0e5

Add total_mix_stake to validator api

ab0ac2f

cargo fmt

3289fe2

Handle errors properly

bf6eb1a

Gateway stake accounting

525da69

durch force-pushed the total-stake-accounting branch from 2b604d7 to 525da69 Compare September 30, 2021 14:23

tests

79a911f

durch requested a review from jstuczyn October 1, 2021 09:08

jstuczyn reviewed Oct 1, 2021

View reviewed changes

durch changed the base branch from develop to tokenomics-rewards October 1, 2021 09:31

Naming :)

efcfae4

durch merged commit 90f4c37 into tokenomics-rewards Oct 4, 2021

durch deleted the total-stake-accounting branch October 4, 2021 08:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Total mix stake accounting #794

Total mix stake accounting #794

durch commented Sep 30, 2021 •

edited

Loading

jstuczyn left a comment

jstuczyn Oct 1, 2021

durch Oct 1, 2021

jstuczyn Oct 1, 2021

durch Oct 1, 2021

jstuczyn Oct 1, 2021

durch Oct 1, 2021

Total mix stake accounting #794

Total mix stake accounting #794

Conversation

durch commented Sep 30, 2021 • edited Loading

jstuczyn left a comment

Choose a reason for hiding this comment

jstuczyn Oct 1, 2021

Choose a reason for hiding this comment

durch Oct 1, 2021

Choose a reason for hiding this comment

jstuczyn Oct 1, 2021

Choose a reason for hiding this comment

durch Oct 1, 2021

Choose a reason for hiding this comment

jstuczyn Oct 1, 2021

Choose a reason for hiding this comment

durch Oct 1, 2021

Choose a reason for hiding this comment

durch commented Sep 30, 2021 •

edited

Loading