Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Fixed fees calculations in getForgedByAccount endpoint #384

Merged
merged 14 commits into from
Jan 24, 2017
Merged

Fixed fees calculations in getForgedByAccount endpoint #384

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a58288f
Fixed fees calculations in getForgedByAccount endpoint
4miners Jan 9, 2017
47ecf23
Make sql query more readable
4miners Jan 10, 2017
ddffa97
Add check for delegate exists in aggregateBlocksReward
4miners Jan 10, 2017
0817002
Changing expectations for getForgedByAccount
4miners Jan 10, 2017
65e3cdb
Direct adjust of active delegates, changing expectations for getForge…
4miners Jan 10, 2017
539b4ba
Changing expectations for getForgedByAccount
4miners Jan 10, 2017
0e6064d
Changing expectations for getForgedByAccount
4miners Jan 10, 2017
9a38a92
Changing expectations for getForgedByAccount
4miners Jan 10, 2017
c6022f3
Search based on timestamp instead of height
4miners Jan 11, 2017
24cac3d
Make sql query more readable, changed error message
4miners Jan 24, 2017
0f8cc6f
Changing expectations for getForgedByAccount
4miners Jan 24, 2017
0dca3d2
Improved performance of summedRound SQL query
4miners Jan 24, 2017
858ea7d
Merge branch 'development' into lisk-sql-1
karmacoma Jan 24, 2017
f08ee65
Merge branch 'development' into lisk-sql-1
karmacoma Jan 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions modules/blocks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1398,21 +1398,24 @@ Blocks.prototype.cleanup = function (cb) {
};

Blocks.prototype.aggregateBlocksReward = function (filter, cb) {
var params = {}, where = [];
var params = {};

where.push('"b_generatorPublicKey"::bytea = ${generatorPublicKey}');
params.generatorPublicKey = filter.generatorPublicKey;
params.delegates = constants.activeDelegates;

if (filter.start !== undefined) {
params.start = filter.start - constants.epochTime.getTime () / 1000;
}

if (filter.end !== undefined) {
params.end = filter.end - constants.epochTime.getTime () / 1000;
}

where.push('"b_timestamp" >= ${start}');
params.start = filter.start - constants.epochTime.getTime () / 1000;

where.push('"b_timestamp" <= ${end}');
params.end = filter.end - constants.epochTime.getTime () / 1000;

library.db.query(sql.aggregateBlocksReward({
where: where,
}), params).then(function (rows) {
library.db.query(sql.aggregateBlocksReward(params), params).then(function (rows) {
var data = rows[0];
if (data.delegate === null) {
return setImmediate(cb, 'Account not exists or is not a delegate');
Copy link
Contributor

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.

}
data = { fees: data.fees || '0', rewards: data.rewards || '0', count: data.count || '0' };
return setImmediate(cb, null, data);
}).catch(function (err) {
Expand Down
5 changes: 4 additions & 1 deletion modules/delegates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why changed condition here?

Copy link
Contributor Author

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);
}
var forged = bignum(reward.fees).plus(bignum(reward.rewards)).toString();
return setImmediate(cb, null, {fees: reward.fees, rewards: reward.rewards, forged: forged, count: reward.count});
});
Expand Down
48 changes: 45 additions & 3 deletions sql/blocks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,51 @@ var BlocksSql = {

aggregateBlocksReward: function (params) {
return [
'SELECT SUM("b_totalFee") AS fees, SUM("b_reward") AS rewards, COUNT(1) AS count',
'FROM blocks_list',
(params.where.length ? 'WHERE ' + params.where.join(' AND ') : '')
'WITH',
'delegate as (SELECT',
'1 FROM mem_accounts m WHERE m."isDelegate" = 1 AND m."publicKey" = DECODE (${generatorPublicKey}, \'hex\') LIMIT 1),',
'borders as (SELECT',
'(SELECT (CAST(b.height / ${delegates} AS INTEGER) + (CASE WHEN b.height % ${delegates} > 0 THEN 1 ELSE 0 END)) FROM blocks b ORDER BY b.height DESC LIMIT 1) as current,',
'(SELECT (CAST(b.height / ${delegates} AS INTEGER) + (CASE WHEN b.height % ${delegates} > 0 THEN 1 ELSE 0 END)) FROM blocks b',
(params.start !== undefined ? ' WHERE b.timestamp >= ${start}' : ''),
'ORDER BY b.height ASC LIMIT 1) as min,',
'(SELECT (CAST(b.height / ${delegates} AS INTEGER) + (CASE WHEN b.height % ${delegates} > 0 THEN 1 ELSE 0 END)) FROM blocks b',
(params.end !== undefined ? ' WHERE b.timestamp <= ${end}' : ''),
'ORDER BY b.height DESC LIMIT 1) as max',
'),',
'r as (SELECT DISTINCT ',
'(CAST(b.height / ${delegates} AS INTEGER) + (CASE WHEN b.height % ${delegates} > 0 THEN 1 ELSE 0 END)) AS round',
'FROM blocks b WHERE b."generatorPublicKey" = DECODE (${generatorPublicKey}, \'hex\')),',
're as (SELECT r.round as round, ((r.round-1)*${delegates})+1 as min, r.round*${delegates} as max',
'FROM r WHERE r.round >= (SELECT min FROM borders) AND round <= (SELECT max FROM borders)),',
'sum_min as (SELECT',
'sum(CASE WHEN b."generatorPublicKey" = DECODE (${generatorPublicKey}, \'hex\') THEN b.reward ELSE 0 END) AS rewards,',
'sum(CASE WHEN b."generatorPublicKey" = DECODE (${generatorPublicKey}, \'hex\') THEN 1 ELSE 0 END) AS blocks',
'FROM blocks b WHERE b.height BETWEEN (SELECT min FROM re ORDER BY round ASC LIMIT 1) AND (SELECT max FROM re ORDER BY round ASC LIMIT 1)',
(params.start !== undefined ? 'AND b.timestamp >= ${start}' : ''),
'),',
'sum_max as (SELECT',
'sum(CASE WHEN b."generatorPublicKey" = DECODE (${generatorPublicKey}, \'hex\') THEN b.reward ELSE 0 END) AS rewards,',
'sum(CASE WHEN b."generatorPublicKey" = DECODE (${generatorPublicKey}, \'hex\') THEN 1 ELSE 0 END) AS blocks',
'FROM blocks b WHERE b.height BETWEEN (SELECT min FROM re ORDER BY round DESC LIMIT 1) AND (SELECT max FROM re ORDER BY round DESC LIMIT 1)',
(params.end !== undefined ? 'AND b.timestamp <= ${end}' : ''),
'),',
'rs as (SELECT re.*, SUM(b."totalFee") AS fees,',
'sum(CASE WHEN b."generatorPublicKey" = DECODE (${generatorPublicKey}, \'hex\') THEN b.reward ELSE 0 END) AS rewards,',
'sum(CASE WHEN b."generatorPublicKey" = DECODE (${generatorPublicKey}, \'hex\') THEN 1 ELSE 0 END) AS blocks',
'FROM re, blocks b WHERE b.height BETWEEN re.min AND re.max GROUP BY re.round, re.min, re.max),',
'rsc as (SELECT',
'(CASE WHEN round = borders.current THEN 0 ELSE fees END), round,',
'(CASE WHEN round = borders.min THEN (SELECT blocks FROM sum_min) ELSE (CASE WHEN round = borders.max THEN (SELECT blocks FROM sum_max) ELSE blocks END) END) as blocks,',
'(CASE WHEN round = borders.min THEN (SELECT rewards FROM sum_min) ELSE (CASE WHEN round = borders.max THEN (SELECT rewards FROM sum_max) ELSE rewards END) END) as rewards,',
'(SELECT 1 FROM blocks b WHERE b.height = rs.max AND b."generatorPublicKey" = DECODE (${generatorPublicKey}, \'hex\') LIMIT 1) AS last',
'FROM rs, borders)',
'SELECT',
'(SELECT * FROM delegate) as delegate,',
'sum (rsc.blocks) as count,',
Copy link
Contributor

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.

'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'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow

Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check that.

].filter(Boolean).join(' ');
},

Expand Down
38 changes: 36 additions & 2 deletions test/api/delegates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1017,8 +1017,8 @@ describe('GET /api/delegates/forging/getForgedByAccount', function () {
function buildParams () {
return [
'generatorPublicKey=' + validParams.generatorPublicKey,
validParams.start ? 'start=' + validParams.start : '',
validParams.end ? 'end=' + validParams.end : '',
validParams.start !== undefined ? 'start=' + validParams.start : '',
validParams.end !== undefined ? 'end=' + validParams.end : '',
].filter(Boolean).join('&');
}

Expand All @@ -1031,17 +1031,33 @@ describe('GET /api/delegates/forging/getForgedByAccount', function () {
});

it('using valid params should be ok', function (done) {
delete validParams.start;
delete validParams.end;

node.get('/api/delegates/forging/getForgedByAccount?' + buildParams(), function (err, res) {
node.expect(res.body).to.have.property('success').to.be.ok;
node.expect(res.body).to.have.property('fees').that.is.a('string');
node.expect(res.body).to.have.property('rewards').that.is.a('string');
node.expect(res.body).to.have.property('forged').that.is.a('string');
done();
});
});

it('using valid params with borders should be ok', function (done) {
node.get('/api/delegates/forging/getForgedByAccount?' + buildParams(), function (err, res) {
node.expect(res.body).to.have.property('success').to.be.ok;
node.expect(res.body).to.have.property('fees').that.is.a('string').and.eql('0');
node.expect(res.body).to.have.property('rewards').that.is.a('string').and.eql('0');
node.expect(res.body).to.have.property('forged').that.is.a('string').and.eql('0');
node.expect(res.body).to.have.property('count').that.is.a('string').and.eql('0');
done();
});
});

it('using unknown generatorPublicKey should fail', function (done) {
validParams.generatorPublicKey = node.randomAccount().publicKey;
delete validParams.start;
delete validParams.end;

node.get('/api/delegates/forging/getForgedByAccount?' + buildParams(), function (err, res) {
node.expect(res.body).to.have.property('success').to.be.not.ok;
Expand All @@ -1050,6 +1066,16 @@ describe('GET /api/delegates/forging/getForgedByAccount', function () {
});
});

it('using unknown generatorPublicKey with borders should fail', function (done) {
validParams.generatorPublicKey = node.randomAccount().publicKey;

node.get('/api/delegates/forging/getForgedByAccount?' + buildParams(), function (err, res) {
node.expect(res.body).to.have.property('success').to.be.not.ok;
node.expect(res.body).to.have.property('error').to.eql('Account not exists or is not a delegate');
done();
});
});

it('using invalid generatorPublicKey should fail', function (done) {
validParams.generatorPublicKey = 'invalidPublicKey';

Expand All @@ -1065,6 +1091,10 @@ describe('GET /api/delegates/forging/getForgedByAccount', function () {

node.get('/api/delegates/forging/getForgedByAccount?' + buildParams(), function (err, res) {
node.expect(res.body).to.have.property('success').to.be.ok;
node.expect(res.body).to.have.property('fees').that.is.a('string').and.eql('0');
node.expect(res.body).to.have.property('rewards').that.is.a('string').and.eql('0');
node.expect(res.body).to.have.property('forged').that.is.a('string').and.eql('0');
node.expect(res.body).to.have.property('count').that.is.a('string').and.eql('0');
done();
});
});
Expand All @@ -1074,6 +1104,10 @@ describe('GET /api/delegates/forging/getForgedByAccount', function () {

node.get('/api/delegates/forging/getForgedByAccount?' + buildParams(), function (err, res) {
node.expect(res.body).to.have.property('success').to.be.ok;
node.expect(res.body).to.have.property('fees').that.is.a('string');
node.expect(res.body).to.have.property('rewards').that.is.a('string');
node.expect(res.body).to.have.property('forged').that.is.a('string');
node.expect(res.body).to.have.property('count').that.is.a('string');
done();
});
});
Expand Down