New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: MMD-1309 10-to-1 #225
Draft
ltfschoen
wants to merge
73
commits into
luke/MMD-1309/update-substrate-3
Choose a base branch
from
luke/rewards-allowance-new
base: luke/MMD-1309/update-substrate-3
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
feat: MMD-1309 10-to-1 #225
ltfschoen
wants to merge
73
commits into
luke/MMD-1309/update-substrate-3
from
luke/rewards-allowance-new
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ltfschoen
force-pushed
the
luke/rewards-allowance-new
branch
from
September 8, 2021 03:57
4195838
to
7c2490c
Compare
…erflow and overflow
…o customize democracy tests
…e. add full rewards test that passes
…e are giving them each a proportion of the daily rewards available instead of first processed first served
…gging the Balance type issue, where it should have been u128 instead of u64 in the mocks
…r_date and remove BondedDHXData type alias
… use safe mult. remove unncessary Set event
…oth u128 and BalanceOf types
…and rewards_allowance_dhx_daily so not reducing remaining. fix by funding treasury in tests so rewards paid
…and cool down starts. initialise default bonded amounts to 0 in implementation
… mPower less than daily minimum
ltfschoen
force-pushed
the
luke/rewards-allowance-new
branch
from
October 19, 2021 12:11
be42e5b
to
586dd35
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: To debug this branch run the following:
IMPORTANT: If you get error
duplicate lang item in crate
, it's likely because I've addedprintln!
statements in a pallet for debugging during testing when runningcargo test...
, so you'll have to temporarily comment thoseprintln!
statements out to buildprintln!
statements to view debug logs during testing)and use println in implementation to add variables to log i.e.
println!("block: {:#?}, miner: {:#?}, date_start: {:#?}", _n, miner_count, start_of_requested_date_millis);
Configure runtime for faster debugging to check functionality still works
Since we want simulate bonding for debugging, we want to the sessions and eras to go by faster, so we can change these runtime values:
See #218
For faster democracy proposals and referendums:
After making changes, build
cargo build --release
Outstanding issues
cargo build --release
Update: Thanks to Guillaume I discovered this was because I was incorrectly storing the data into
RewardsAllowanceDHXForDate
when i meant to be storing it inBondedDHXForAccountForDate
, and this was accepting aVec
, which i didn't needcargo +nightly-2021-08-31 test -p mining-rewards-allowance
. To generate the error it's first necessary to comment out the extrinsic function calledset_bonded_dhx_of_account_for_date
Update: Thanks to Guillaume this error was fixed in this commit 1b410fc
Update: the error i was getting was because i needed to change
ScheduleOrigin = EnsureRoot<u64>;
to typeScheduleOrigin = EnsureRoot<u128>;
in pallet_scheduler. i guess that's because we're using an AccountId of u128 in frame_system since according to the Substrate comments u64 is not enough to hold bytes used to generate bounty account. See how Bryan Chen did it here https://github.com/open-web3-stack/open-runtime-module-library/blob/master/authority/src/mock.rs#L62- Mock the following pallets in the mining-rewards-allowances pallet. Reason: we want to write tests that simulates users participating in democracy (using democracy pallet) by locking a deposit of tokens for proposal voting, or participating in elections (using elections-phragmen pallet) by bonding a deposit of tokens to back some candidates (i.e. election member deposit, candidate deposit, or a stake placed on a vote), then i want to query that data since our goal is to reward users that are "participating" through these means.
Draft: 71b716a
Fixed the test suite which was using u64 for the Balance type, which differed from the implementation that was using u128 for the Balance type, so conversion between u128 to Balance type when running tests was failing
Wrote a test for the full flow of distributing rewards to registered DHX miners after satisfying their cooling off period and checking storage was updated with reward information. This is to protect the code so it doesn't break when I start introducing future changes like integrating mPower rewards.
check that daily reward quotas are reset each day so eligible receive rewards two days or more in a row
check that if they unbond so insufficient bonded then they stop receiving the rewards the next day onward and cooldown period starts
check that after 3 months (we’ll make it say 3 days in the tests) the multiplier doubles
check that after the multiplier doubles, they are no longer eligible to receive the rewards if they have the same amount bonded (since they’d need twice the amount bonded), even if they have sufficient mpower, and that multiplier period days total and remaining are reset. note: we've just added a test that sets their bonded amount to 0 and checked that their accumulated and aggregated rewards become 0 and their cool down period starts.
check that if insufficient mPower then they stop receiving rewards the next day and cooldown period starts
check that pause/reset works
- Write tests for the flow of simulating user accounts participating by bonding DHX tokens. Update: it appears to be sufficient to check Balance::locks, this should do for the moment
i.e.
- every 3 months initially we'll change from 10:1 to 20:1 and so on, it may be changed by governance proposal. this is part of migrating the dhx center temporary solution onto blockchain so it's decentralised. the bulk of this functionality and tests to provide robustness was finished in commit 2211881
check that rewards are distributed after cooling off period.
note: this will be done after we've implemented off-chain workers for mpower, and after moving the distribution of rewards into a "claim" extrinsic that the user needs to call (instead of being in the on_initialize or on_finalize function)
-
issue of if set mpower in storage near end of date, then set bonded dhx shortly after, then they are stored under
keys on different days and doesn't match for rewards!!
issue of if we can't get their off-chain mPower today due to network issues retrieving it, then we'll need to still store a "pending" status until we receive the mPower for that date, and when we do we'll calculate and update the accumulated/aggregated for that day and let them be eligible to claim rewards if there were still dhx remaining for that date.
so we need to also loop through the flagged historic dates for all the miners were we couldn't get their mpower too somehow.
in the meantime we'll assume we always get the mpower in time
why do i have to do
change_mpower_for_each_miner
on the day beforehand i do accum/aggreg for it to work?we need to set the mPower for the next start date so it's available from off-chain in time,
we really should store the next start date as the day that we actually process the
previous day's eligibility for rewards that were accum/aggregated since we need to allow time to get the mPower first
- Instead of storing data on chain, just emit events, and use SubQuery tool as an indexer, which is like TheGraph to do things like taking snapshots every date (instead of having to use Polkadot.js), so we only need to write the indexer itself, then query data + storage at a block, so we could run the indexer all the time, and get data we want at end of the month to distribute rewards based on events that were emitted. It's like substrate-archive, you go through blocks to get the data, then query the database (like you would with RDMS) so it's better than an API and you can do aggregation of the data. There may even be SaaS services that we could use to do the queries.
Note: Each pallet can have different locks (i.e. Democracy::locks) that apply in parallel to prevent the user spending above the max. lock, where those locks share the same balance of Balances::locks
Note: As a rule of thumb, don't store anything non-consensus on-chain. (i.e. balances related to consensus, need read both balances since its the input, but system > remark isn't related to consensus, so we don't want to trash the blockchain by storing that information on the chain.
Note: to test that the functionality works using log library whilst running a node where it reduces remaining days and issues rewards all on the same day in subsequent blocks (instead of only doing it once per day, which takes too long to check that it works), then just comment out the lines like
cooling_off_period_days_remaining.0 != start_of_requested_date_millis.clone() &&
andif rm_current_period_days_remaining.1 != start_of_requested_date_millis.clone() {
(including this line's end brace), and change fromlet mut locks_first_amount_as_u128 = 0u128.clone();
tolet mut locks_first_amount_as_u128 = default_bonded_amount_u128.clone();
- add
offchain_worker
function to runtime, since we'll need to use this instead ofon_initialize
andon_finalize
for some stuff- in the Substrate fork that we're using as a dependency, we're exposing some functions publicly only for testing that we don't want to be exposed in production (i.e. the ability to just call a function to create a referendum directly), how do we restrict it so they may only be used for testing? (i.e. use our Substrate fork commit dependencies only for testing, and use the normal Substrate commit dependencies for production)???