Skip to content
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

/api/delegates/{address}/forging_statistics returns different results compared to v0.9.16 #2367

Closed
TheGoldenEye opened this Issue Aug 31, 2018 · 8 comments

Comments

Projects
3 participants
@TheGoldenEye
Copy link
Contributor

TheGoldenEye commented Aug 31, 2018

Expected behavior

forging statistics should not change with the software version

Actual behavior

v0.9.16:
.../api/delegates/forging/getForgedByAccount/?generatorPublicKey=f91766de68f3a8859a3634c3a0fdde38ebd82dd91fc37b67ac6cf010800a3e6e
result:

{
success: true,
fees: "286369702998",
rewards: "24484000000000",
forged: "24770369702998"
}

v1.0.1:
.../api/delegates/15056673209583244182L/forging_statistics
result:

{
meta: {
fromTimestamp: 1464109200000,
toTimestamp: 1535752640874
},
data: {
fees: "287611980385",
rewards: "24570000000000",
forged: "24857611980385",
count: "54049"
},
links: { }
}

Or only for year 2017:

v0.9.16
.../api/delegates/forging/getForgedByAccount/?generatorPublicKey=f91766de68f3a8859a3634c3a0fdde38ebd82dd91fc37b67ac6cf010800a3e6e&start=1483228800&end=1514764800
result:

{
success: true,
fees: "158721583262",
rewards: "15008200000000",
forged: "15166921583262",
count: "30771"
}

v1.0.1:
.../api/delegates/15056673209583244182L/forging_statistics?fromTimestamp=1483228800000&toTimestamp=1514764800000
result:

{
meta: {
fromTimestamp: 1483228800000,
toTimestamp: 1514764800000
},
data: {
fees: "158724157519",
rewards: "15008700000000",
forged: "15167424157519",
count: "30771"
},
links: { }
}

Which version(s) does this affect? (Environment, OS, etc...)

v1.0.1

@TheGoldenEye

This comment has been minimized.

Copy link
Contributor Author

TheGoldenEye commented Aug 31, 2018

Additional:
if I query time slices of each one day and sum them up for 2017, I receive a sum of 151698 LSK, which is much more than the total value of 151674 in 2017.

@TheGoldenEye

This comment has been minimized.

Copy link
Contributor Author

TheGoldenEye commented Aug 31, 2018

BTW, the new API expects no unix timestamps as documented
instead it uses the number of milliseconds since midnight Jan 1, 1970 (not the number of seconds)

@TheGoldenEye

This comment has been minimized.

@diego-G diego-G added this to Icebox in Lisk Pipelines Sep 4, 2018

@TheGoldenEye

This comment has been minimized.

Copy link
Contributor Author

TheGoldenEye commented Sep 6, 2018

You had time to check it?

@4miners

This comment has been minimized.

Copy link
Member

4miners commented Sep 7, 2018

@TheGoldenEye Thank you, I'll investigate.

@TheGoldenEye

This comment has been minimized.

Copy link
Contributor Author

TheGoldenEye commented Sep 22, 2018

I've found three reasons, because the forging statistic is different between v1.0.0 and v0.9.16:

  1. The statistic now includes the additional LSKs resulting from a bug in the lisk software (Round 27040 was applied twice) ==> OK

  2. The query now returns the forging rewards at the time of the round (was in 0.9.x the time of the forged block). There is thus a slight time lag, but the total result is correct ==> OK

  3. If I query time slices of each one day and sum them up for 2017, the result is significantly larger than the total result for 2017. That's a real bug and I've prepared a PR for this. The exact explanation you will find there too.

@TheGoldenEye

This comment has been minimized.

Copy link
Contributor Author

TheGoldenEye commented Oct 1, 2018

Maybe it's possible to close this issue with core v1.1?

@MaciejBaj MaciejBaj removed the discussion label Oct 3, 2018

@MaciejBaj

This comment has been minimized.

Copy link
Member

MaciejBaj commented Oct 3, 2018

@TheGoldenEye yes, we've improved the performance of forging_statistics endpoint and included into v1.1 - https://github.com/LiskHQ/lisk/pull/2360/files.
Your 2. point is for sure addressed by #2360:

The query now returns the forging rewards at the time of the round (was in 0.9.x the time of the forged block). There is thus a slight time lag, but the total result is correct ==> OK

Point 3. is a very nice catch:

The reason is a rounding error when converting the UNIX timestamp to Lisk timestamp:
timeStamp: 1486511999999
Milliseconds since lisk start: 1486511999999-epochtime = 22402799999
Lisk Timestamp: 22402799999 / 1000 = 22402799.999
The Lisk timestamp should be '22402799' and not '22402800' which is the (rounded) result of the toFixed() method !!!

We can include your fix into 1.1.0 version and make it fixing forging_statistics endpoint completely.

Thanks!

@MaciejBaj MaciejBaj added this to New Issues in Version 1.1.0 via automation Oct 3, 2018

@MaciejBaj MaciejBaj added this to the Version 1.1.0 milestone Oct 3, 2018

@MaciejBaj MaciejBaj closed this Oct 4, 2018

Version 1.1.0 automation moved this from New Issues to Closed Issues Oct 4, 2018

MaciejBaj added a commit that referenced this issue Oct 4, 2018

Merge pull request #2417 from TheGoldenEye/1.0.0_patch_aggregateBlock…
…sReward

Calculate the correct time interval in aggregateBlocksReward - Closes #2367

@MaciejBaj MaciejBaj removed this from Icebox in Lisk Pipelines Oct 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.