Added new attributes to the `getstakinginfo` and `getstakereport` commands #485

mxaddict · 2019-05-16T02:43:44Z

Added new attributes to the getstakinginfo and getstakereport commands

getstakinginfo now has expecteddailyreward

getstakereport now has Last 7 Days Avg, Last 30 Days Avg and Last 365 Days Avg

reports looks like these:

{
  "enabled": true,
  "staking": true,
  "errors": "",
  "currentblocksize": 1000,
  "currentblocktx": 0,
  "difficulty": 537692.0995617752,
  "search-interval": 16,
  "weight": 1198583732549,
  "netstakeweight": 1339930863008790,
  "expectedtime": 33537,
  "expecteddailyreward": 5.15236448
}

{
  "2019-05-15 16:00:00": "2.00",
  "2019-05-14 16:00:00": "2.00",
  "2019-05-13 16:00:00": "2.00",
  "2019-05-12 16:00:00": "2.00",
  "2019-05-11 16:00:00": "2.00",
  "2019-05-10 16:00:00": "6.0001",
  "2019-05-09 16:00:00": "4.00",
  "2019-05-08 16:00:00": "4.00",
  "2019-05-07 16:00:00": "6.00",
  "2019-05-06 16:00:00": "4.00",
  "2019-05-05 16:00:00": "2.00",
  "2019-05-04 16:00:00": "4.00",
  "2019-05-03 16:00:00": "8.00",
  "2019-05-02 16:00:00": "4.00",
  "2019-05-01 16:00:00": "6.00",
  "2019-04-30 16:00:00": "2.00",
  "2019-04-29 16:00:00": "0.00",
  "2019-04-28 16:00:00": "4.00",
  "2019-04-27 16:00:00": "4.00",
  "2019-04-26 16:00:00": "2.00",
  "2019-04-25 16:00:00": "4.00",
  "2019-04-24 16:00:00": "4.00",
  "2019-04-23 16:00:00": "0.00",
  "2019-04-22 16:00:00": "2.00",
  "2019-04-21 16:00:00": "2.00",
  "2019-04-20 16:00:00": "2.00",
  "2019-04-19 16:00:00": "4.00",
  "2019-04-18 16:00:00": "2.00",
  "2019-04-17 16:00:00": "2.00",
  "2019-04-16 16:00:00": "4.00",
  "Last 24H": "2.00",
  "Last 7 Days": "22.0001",
  "Last 7 Days Avg": "3.14287142",
  "Last 30 Days": "98.0001",
  "Last 30 Days Avg": "3.26667",
  "Last 365 Days": "425.2213209",
  "Last 365 Days Avg": "1.16498992",
  "Last All": "438.03126175",
  "Latest Stake": "2.00",
  "Latest Time": "2019-05-15 20:53:04",
  "Stake counted": 274,
  "time took (ms)": 1
}

aguycalled · 2019-05-16T08:45:02Z

src/wallet/rpcwallet.cpp

+        // Check if we need to add the average
+        if (nDays > 0) {
+            // Add the Average
+            result.push_back(Pair(vIt->Name + " Avg", FormatMoney(vIt->Total / nDays).c_str()));


Shouldn't the average be calculated only over the amount for days where a stake happened? If my wallet is 60 days old, it'll need 305 days to show an accurate number in "365 Avg", for example.

So you mean calculate the average stake for the last X days where X Days is the number of days past since your first stake that is no older than 365 days?

Well, that would not really be a 365 day average, that would be more an average since staking.

The current formula would be 100% accurate if your wallet is 1 year old, but as you say it would be completely off if your wallet has only been staking for like 45 days.

I'll see what I can come up with for making the average more accurate.

I think I can calculate the wallet age based on first stake date and use it to adjust nDays value.

I would define “X Avg” as the daily average received as rewards over the last X days of active staking.

I think this should be clearly documented somewhere so people don't accidentally assume the averages are simple averages over the entire X timeframe regardless of initial staking activity.

mxaddict · 2019-05-16T09:20:47Z

Ahh, so just add a new attribute to the response?

…

On Thu, May 16, 2019, 17:19 alex v. ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/wallet/rpcwallet.cpp <#485 (comment)>: > result.push_back(Pair(vIt->Name, FormatMoney(vIt->Total).c_str())); + + // Get the nDays value + nDays = 0; + if (vIt->Name == "Last 7 Days") + nDays = 7; + else if (vIt->Name == "Last 30 Days") + nDays = 30; + else if (vIt->Name == "Last 365 Days") + nDays = 365; + + // Check if we need to add the average + if (nDays > 0) { + // Add the Average + result.push_back(Pair(vIt->Name + " Avg", FormatMoney(vIt->Total / nDays).c_str())); I would define “X Avg” as the daily average received as rewards over the last X days of active staking. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#485?email_source=notifications&email_token=AAIDAKMQO4DNOXU5SEPXNEDPVURKDA5CNFSM4HNIOYXKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBYZ4DAY#discussion_r284614385>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIDAKOTGBHMRNSWNVWWU63PVURKDANCNFSM4HNIOYXA> .

aguycalled · 2019-05-16T10:12:23Z

Ahh, so just add a new attribute to the response?
…

what I mean is, as a general definition, “X Avg” should show Sum(stakes[today-X:today])/min(X,n) for a wallet which is n days old

mxaddict · 2019-05-16T13:03:45Z

Ahh, so just add a new attribute to the response?
…

what I mean is, as a general definition, “X Avg” should show Sum(stakes[today-X:today])/min(X,n) for a wallet which is n days old

You mean like this:

{
  "Last 24H": "2.00",
  "Last 7 Days": "22.0001",
  "Last 7 Days Avg": "3.14287142",
  "Last 30 Days": "98.0001",
  "Last 30 Days Avg": "3.26667",
  "Last 365 Days": "425.2213209",
  "Last 365 Days Avg": "1.16498992",
  "Last X Avg": "3.14287142",
  "Last All": "438.03126175"
}

aguycalled · 2019-05-16T13:05:18Z

Ahh, so just add a new attribute to the response?
…

what I mean is, as a general definition, “X Avg” should show Sum(stakes[today-X:today])/min(X,n) for a wallet which is n days old

You mean like this:
{
  "Last 24H": "2.00",
  "Last 7 Days": "22.0001",
  "Last 7 Days Avg": "3.14287142",
  "Last 30 Days": "98.0001",
  "Last 30 Days Avg": "3.26667",
  "Last 365 Days": "425.2213209",
  "Last 365 Days Avg": "1.16498992",
  "Last X Avg": "3.14287142",
  "Last All": "438.03126175"
}

Let X be 7, 30 or 365.

aguycalled · 2019-05-19T17:25:10Z

src/wallet/rpcwallet.cpp

+    else if (tx->vout[1].scriptPubKey.IsColdStaking())
+        return tx->GetCredit(pwalletMain->IsMine(tx->vout[1])) - tx->GetDebit(pwalletMain->IsMine(tx->vout[1]));
+
+    return tx->GetCredit(ISMINE_SPENDABLE) + tx->GetCredit(ISMINE_STAKABLE) - tx->GetDebit(ISMINE_SPENDABLE) - tx->GetDebit(ISMINE_STAKABLE);


ignoring ISMINE_SPENDABLE_STAKABLE would make this to fail for wallets holding both the spending and staking key (not an ideal scenario from a security perspective but its technically possible)

I'll see if I can add more scenarios to the test case

I think this scenario was already covered by the test cases

Please check this code in the test and let me know what you think:

# Import the 2 keys into a third wallet self.nodes[0].importprivkey(spending_address_private_key) self.nodes[0].importprivkey(staking_address_private_key)

Basically in the test cases, there are 3 modes, node 1 has spending key, node 2 has staking key and node 0 has both keys imported

yes you are right, my bad as ISMINE_SPENDABLE_STAKABLE = ISMINE_SPENDABLE | ISMINE_STAKABLE

I forget why, but in the 4.6.0 fix that I created for this

This would not get the correct amounts:

tx->GetCredit(ISMINE_SPENDABLE_STAKABLE) - tx->GetDebit(ISMINE_SPENDABLE_STAKABLE);

Bu this would:

tx->GetCredit(ISMINE_SPENDABLE) + tx->GetCredit(ISMINE_STAKABLE) - tx->GetDebit(ISMINE_SPENDABLE) - tx->GetDebit(ISMINE_STAKABLE);

While in theory they should be the same, but I don't remember exactly why.

I think there was code in GetCredit and GetDebit that was causing an issue when passing ISMINE_SPENDABLE_STAKABLE

ISMINE_SPENDABLE_STAKABLE is both Spendable+Stakable

Anyways, I think the scenarios are covered in the test case

And I also added the time travel stuff that we discussed with @craigmacgregor during the 4.6.0 release which we skipped due to time constraints.

aguycalled · 2019-05-19T17:27:29Z

src/wallet/rpcwallet.cpp

+    const CWalletTx* tx;
+
+    // scan the entire wallet transactions
+    for (auto it = pwalletMain->mapWallet.end(); it != pwalletMain->mapWallet.begin(); --it)


maps are ordered per key so theres no assurance the first stake you will hit is the oldest stake, i'd say the easiest would be to iterate over wtxOrdered

ACK, I will replace this with wtxOrdered, thanks for the heads up.

I pushed a commit which addresses this.

mxaddict · 2019-05-28T19:20:36Z

Rebased the branch from master

proletesseract · 2019-06-10T09:28:26Z

Just to confirm, all the Last X Days Avg values are programmed to give the daily average amount staked within period X or the daily average amount from the first stake till the end of X period if that length is shorter than X.

proletesseract · 2019-06-10T09:29:35Z

Builds on Ubuntu 18.04. Tests pass. Tests make sense.

proletesseract · 2019-06-10T09:47:24Z

src/wallet/rpcwallet.cpp

+    const CWalletTx* tx;
+
+    // scan the entire wallet transactions
+    for (auto it = pwalletMain->wtxOrdered.begin(); it != pwalletMain->wtxOrdered.end(); ++it)


If wtxOrdered is not changing length during this loop, you could cache the end iterator to save it being calculated each time the loop iterates. I'm not sure on the exact syntax here, in javascript you can do it directly into the loop declaration like this;

for(let i=0, l=my_array.length(); i<l; i++) { console.log(my_array[i]); }

I don't think there's an inline way to do it in C++, so you'd just cache the iterator before you start the loop something like this;

std::vector<uint32_t>::const_iterator end = pwalletMain->wtxOrdered.end(); for (auto it = pwalletMain->wtxOrdered.begin(); it != end; ++it) { //do stuff }

It's probably not hugely relevant here since we're hoping the first item is the one we're looking for, but its a neat trick that can optimise loops especially when you're iterating through thousands of items like we can be doing when dealing with wallet transactions and especially long lived staking addresses. Keep it in mind if it's useful somewhere else!

:)

proletesseract · 2019-06-10T09:49:03Z

src/wallet/rpcwallet.cpp

+        // Check if we need to add the average
+        if (nDays > 0) {
+            // Add the Average
+            result.push_back(Pair(vIt->Name + " Avg", FormatMoney(vIt->Total / nDays).c_str()));


I think this should be clearly documented somewhere so people don't accidentally assume the averages are simple averages over the entire X timeframe regardless of initial staking activity.

mxaddict · 2019-06-10T13:48:57Z

@proletesseract Rebased on master and added cache for the wallet first stake value (Just in memory, so when the report is run more than once it will use the cached timestamp)

But this report will be slower for wallets with TX data but no stakes (Since it will have to loop all the TX every time in case it has staked since the last check)

…ounger

proletesseract · 2019-06-12T02:19:16Z

code looks good. need to resolve travis issues though.

mxaddict · 2019-06-12T13:35:48Z

Travis-CI Passed, merging

mxaddict force-pushed the patch-12 branch from 5d243b1 to 9eecb94 Compare May 16, 2019 02:45

mxaddict requested review from aguycalled, proletesseract, marcus290, matt-auckland and red010b37 May 16, 2019 02:45

aguycalled reviewed May 16, 2019

View reviewed changes

mxaddict added work in progress ready for review and removed work in progress labels May 18, 2019

mxaddict requested a review from aguycalled May 19, 2019 01:30

aguycalled reviewed May 19, 2019

View reviewed changes

mxaddict requested a review from aguycalled May 19, 2019 19:06

mxaddict force-pushed the patch-12 branch from 0513d24 to dd2a569 Compare May 28, 2019 19:20

proletesseract approved these changes Jun 10, 2019

View reviewed changes

mxaddict force-pushed the patch-12 branch from d14c8c6 to 19f793f Compare June 10, 2019 13:45

mxaddict added 5 commits June 12, 2019 04:34

Added new 'expecteddailyreward' value to 'getstakinginfo'

504ce53

Added 'Last X Days Avg' values to 'getstakereport'

4f72a6e

Updated the getstakereport to be more accurate for wallets that are y…

9c516a2

…ounger

Added more asserts to the getstakereport tests

e97795f

Used wtxOrderd to make sure we get the correct nTime for first stake

549b45c

mxaddict added 3 commits June 12, 2019 04:34

Moved the cs_main LOCK

b431d64

Updated the getstakereport to cache the first stake timestamp

a585112

Optimized the nWalletFirstStake loop

3f80fc6

mxaddict force-pushed the patch-12 branch from f39d4e5 to 3f80fc6 Compare June 11, 2019 20:34

proletesseract approved these changes Jun 12, 2019

View reviewed changes

Missed the pointer

18cc110

aguycalled approved these changes Jun 12, 2019

View reviewed changes

mxaddict merged commit 82a2ba8 into navcoin:master Jun 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added new attributes to the `getstakinginfo` and `getstakereport` commands #485

Added new attributes to the `getstakinginfo` and `getstakereport` commands #485

mxaddict commented May 16, 2019

aguycalled May 16, 2019

mxaddict May 16, 2019

aguycalled May 16, 2019

proletesseract Jun 10, 2019

mxaddict commented May 16, 2019 via email

aguycalled commented May 16, 2019 •

edited

mxaddict commented May 16, 2019 •

edited by aguycalled

aguycalled commented May 16, 2019

aguycalled May 19, 2019

mxaddict May 19, 2019

mxaddict May 19, 2019

aguycalled May 19, 2019

mxaddict May 19, 2019

aguycalled May 19, 2019 •

edited

mxaddict May 19, 2019

aguycalled May 19, 2019

mxaddict May 19, 2019

mxaddict May 19, 2019

mxaddict commented May 28, 2019

proletesseract commented Jun 10, 2019

proletesseract commented Jun 10, 2019

proletesseract Jun 10, 2019 •

edited

proletesseract Jun 10, 2019

mxaddict commented Jun 10, 2019

proletesseract commented Jun 12, 2019

mxaddict commented Jun 12, 2019

Added new attributes to the getstakinginfo and getstakereport commands #485

Added new attributes to the getstakinginfo and getstakereport commands #485

Conversation

mxaddict commented May 16, 2019

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

mxaddict commented May 16, 2019 via email

aguycalled commented May 16, 2019 • edited

mxaddict commented May 16, 2019 • edited by aguycalled

aguycalled commented May 16, 2019

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

aguycalled May 19, 2019 • edited

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

mxaddict commented May 28, 2019

proletesseract commented Jun 10, 2019

proletesseract commented Jun 10, 2019

proletesseract Jun 10, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxaddict commented Jun 10, 2019

proletesseract commented Jun 12, 2019

mxaddict commented Jun 12, 2019

Added new attributes to the `getstakinginfo` and `getstakereport` commands #485

Added new attributes to the `getstakinginfo` and `getstakereport` commands #485

aguycalled commented May 16, 2019 •

edited

mxaddict commented May 16, 2019 •

edited by aguycalled

aguycalled May 19, 2019 •

edited

proletesseract Jun 10, 2019 •

edited