-
Notifications
You must be signed in to change notification settings - Fork 457
Fixed fees calculations in getForgedByAccount endpoint #384
Conversation
@@ -761,8 +761,11 @@ shared.getForgedByAccount = function (req, cb) { | |||
return setImmediate(cb, err[0].message); | |||
} | |||
|
|||
if (req.body.start && req.body.end) { | |||
if (req.body.start !== undefined || req.body.end !== undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why changed condition here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should allow start/end
to be 0
and search by only one border, previous cond not allow that.
modules.blocks.aggregateBlocksReward({generatorPublicKey: req.body.generatorPublicKey, start: req.body.start, end: req.body.end}, function (err, reward) { | ||
if (err) { | ||
return setImmediate(cb, err || 'Error calculating block rewards'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you will never be able to see the 'Error calculating block rewards' here, because 'err' satisfies the condition line above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, will fix that.
'sum (rsc.blocks) as count,', | ||
'sum(floor(rsc.fees/${delegates})*rsc.blocks + (CASE WHEN rsc.last = 1 THEN (rsc.fees-floor(rsc.fees/${delegates})*${delegates}) ELSE 0 END)) as fees,', | ||
'sum(rsc.rewards) as rewards', | ||
'FROM rsc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@4miners: Would it be possible to move this query to a postgres view?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check that.
'sum (rsc.blocks) as count,', | ||
'sum(floor(rsc.fees/${delegates})*rsc.blocks + (CASE WHEN rsc.last = 1 THEN (rsc.fees-floor(rsc.fees/${delegates})*${delegates}) ELSE 0 END)) as fees,', | ||
'sum(rsc.rewards) as rewards', | ||
'FROM rsc' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@4miners: Would it be possible to move this query to a postgres view?
'FROM rs, borders)', | ||
'SELECT', | ||
'(SELECT * FROM delegate) as delegate,', | ||
'sum (rsc.blocks) as count,', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@4miners Please capitalize postgres syntax such as SUM
, AS
. I think more clear.
var data = rows[0]; | ||
if (data.delegate === null) { | ||
return setImmediate(cb, 'Account not exists or is not a delegate'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@4miners Account not found or is not a delegate
.
@karmacoma Thank you for review, fixed. For postgres view I opened another issue - #403. |
@@ -19,7 +19,7 @@ var RoundsSql = { | |||
|
|||
updateBlockId: 'UPDATE mem_accounts SET "blockId" = ${newId} WHERE "blockId" = ${oldId};', | |||
|
|||
summedRound: 'SELECT SUM(b."totalFee")::bigint AS "fees", ARRAY_AGG(b."reward") AS "rewards", ARRAY_AGG(ENCODE(b."generatorPublicKey", \'hex\')) AS "delegates" FROM blocks b WHERE (SELECT (CAST(b."height" / ${activeDelegates} AS INTEGER) + (CASE WHEN b."height" % ${activeDelegates} > 0 THEN 1 ELSE 0 END))) = ${round}' | |||
summedRound: 'WITH round_blocks as (SELECT ((${round}-1)*${activeDelegates})+1 as min, ${round}*${activeDelegates} as max) SELECT SUM(b."totalFee")::bigint AS "fees", ARRAY_AGG(b."reward") AS "rewards", ARRAY_AGG(ENCODE(b."generatorPublicKey", \'hex\')) AS "delegates" FROM blocks b WHERE b.height BETWEEN (SELECT min FROM round_blocks) AND (SELECT max FROM round_blocks)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From 1200-3500 msec to 0.2-0.3 msec.
Before:
EXPLAIN ANALYZE SELECT SUM(b."totalFee")::bigint AS "fees", ARRAY_AGG(b."reward") AS "rewards", ARRAY_AGG(ENCODE(b."generatorPublicKey", 'hex')) AS "delegates" FROM blocks b WHERE (SELECT (CAST(b."height" / 101 AS INTEGER) + (CASE WHEN b."height" % 101 > 0 THEN 1 ELSE 0 END))) = 17400;
After:
EXPLAIN ANALYZE WITH round_blocks as (SELECT ((17400-1)*101)+1 as min, 17400*101 as max) SELECT SUM(b."totalFee")::bigint AS "fees", ARRAY_AGG(b."reward") AS "rewards", ARRAY_AGG(ENCODE(b."generatorPublicKey", 'hex')) AS "delegates" FROM blocks b WHERE b.height BETWEEN (SELECT min FROM round_blocks) AND (SELECT max FROM round_blocks);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@4miners Nice 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @4miners 👍
That query is little complicated and heavy, but should return very accurate results. Executing time for calculations of average active delegate: ~300ms to ~1sec.
Closes #396