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

Make usage of delegate list BFT compliant - Closes #3681 #4397

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions framework/src/controller/schema/application_config_schema.js
Expand Up @@ -87,6 +87,7 @@ module.exports = {
'EPOCH_TIME',
'BLOCK_TIME',
'MAX_TRANSACTIONS_PER_BLOCK',
'DELEGATE_LIST_ROUND_OFFSET',
'REWARDS',
],
properties: {
Expand All @@ -111,6 +112,12 @@ module.exports = {
maximum: 150,
description: 'Maximum number of transactions allowed per block',
},
DELEGATE_LIST_ROUND_OFFSET: {
type: 'number',
minimum: 0,
description:
'Number of rounds before in which the list of delegates will be used for the current round - i.e. The set of active delegates that will be chosen to forge during round `r` will be taken from the list generated in the end of round `r - DELEGATE_LIST_ROUND_OFFSET`',
},
REWARDS: {
id: 'rewards',
type: 'object',
Expand Down Expand Up @@ -186,6 +193,7 @@ module.exports = {
EPOCH_TIME: new Date(Date.UTC(2016, 4, 24, 17, 0, 0, 0)).toISOString(),
BLOCK_TIME: 10,
MAX_TRANSACTIONS_PER_BLOCK: 25,
DELEGATE_LIST_ROUND_OFFSET: 2,
REWARDS: {
MILESTONES: [
'500000000', // Initial Reward
Expand Down
22 changes: 18 additions & 4 deletions framework/src/modules/chain/block_processor_v0.js
Expand Up @@ -110,6 +110,7 @@ const validateSchema = ({ block }) => {
class BlockProcessorV0 extends BaseBlockProcessor {
constructor({ blocksModule, dposModule, logger, constants, exceptions }) {
super();
const delegateListRoundOffset = 0;
this.blocksModule = blocksModule;
this.dposModule = dposModule;
this.logger = logger;
Expand All @@ -132,7 +133,8 @@ class BlockProcessorV0 extends BaseBlockProcessor {
blockBytes,
}), // validate common block header
data => this.blocksModule.verifyInMemory(data),
({ block }) => this.dposModule.verifyBlockForger(block),
({ block }) =>
this.dposModule.verifyBlockForger(block, { delegateListRoundOffset }),
]);

this.validateDetached.pipe([
Expand All @@ -155,11 +157,23 @@ class BlockProcessorV0 extends BaseBlockProcessor {

this.verify.pipe([data => this.blocksModule.verify(data)]);

this.apply.pipe([data => this.blocksModule.apply(data)]);
this.apply.pipe([
data => this.blocksModule.apply(data),
({ block, tx }) =>
this.dposModule.apply(block, { tx, delegateListRoundOffset }),
]);

this.applyGenesis.pipe([data => this.blocksModule.applyGenesis(data)]);
this.applyGenesis.pipe([
data => this.blocksModule.applyGenesis(data),
({ block, tx }) =>
this.dposModule.apply(block, { tx, delegateListRoundOffset }),
]);

this.undo.pipe([data => this.blocksModule.undo(data)]);
this.undo.pipe([
data => this.blocksModule.undo(data),
({ block, tx }) =>
this.dposModule.undo(block, { tx, delegateListRoundOffset }),
]);

this.create.pipe([data => this._create(data)]);
}
Expand Down
11 changes: 7 additions & 4 deletions framework/src/modules/chain/block_processor_v2.js
Expand Up @@ -174,7 +174,6 @@ class BlockProcessorV2 extends BaseBlockProcessor {
exceptions,
}) {
super();
const verifyBlockForgerOffset = 2;
this.blocksModule = blocksModule;
this.bftModule = bftModule;
this.dposModule = dposModule;
Expand Down Expand Up @@ -204,8 +203,7 @@ class BlockProcessorV2 extends BaseBlockProcessor {
blockBytes,
}), // validate common block header
data => this.blocksModule.verifyInMemory(data),
({ block }) =>
this.dposModule.verifyBlockForger(block, verifyBlockForgerOffset),
({ block }) => this.dposModule.verifyBlockForger(block),
({ block }) => this.bftModule.validateBlock(block),
]);

Expand All @@ -232,13 +230,18 @@ class BlockProcessorV2 extends BaseBlockProcessor {
this.apply.pipe([
data => this.blocksModule.verify(data),
data => this.blocksModule.apply(data),
({ block, tx }) => this.dposModule.apply(block, { tx }),
({ block, tx }) => this.bftModule.addNewBlock(block, tx),
]);

this.applyGenesis.pipe([data => this.blocksModule.applyGenesis(data)]);
this.applyGenesis.pipe([
data => this.blocksModule.applyGenesis(data),
({ block, tx }) => this.dposModule.apply(block, { tx }),
]);

this.undo.pipe([
data => this.blocksModule.undo(data),
({ block, tx }) => this.dposModule.undo(block, { tx }),
({ block }) => this.bftModule.deleteBlocks([block]),
]);

Expand Down
27 changes: 9 additions & 18 deletions framework/src/modules/chain/blocks/blocks.js
Expand Up @@ -36,8 +36,7 @@ const {
deleteLastBlock,
deleteFromBlockId,
undoConfirmedStep,
saveBlockStep,
undoBlockStep,
saveBlock,
} = require('./chain');
const {
calculateSupply,
Expand Down Expand Up @@ -69,7 +68,6 @@ class Blocks extends EventEmitter {
slots,
exceptions,
// Modules
dposModule,
interfaceAdapters,
// constants
blockReceiptTimeout, // set default
Expand Down Expand Up @@ -100,7 +98,6 @@ class Blocks extends EventEmitter {

this.logger = logger;
this.storage = storage;
this.dposModule = dposModule;
this.exceptions = exceptions;
this.genesisBlock = genesisBlock;
this.interfaceAdapters = interfaceAdapters;
Expand Down Expand Up @@ -327,6 +324,8 @@ class Blocks extends EventEmitter {
this.exceptions,
tx,
);

this._lastBlock = block;
}

async applyGenesis({ block, tx }) {
Expand All @@ -337,6 +336,12 @@ class Blocks extends EventEmitter {
this.exceptions,
tx,
);

this._lastBlock = block;
}

async save({ blockJSON, tx }) {
await saveBlock(this.storage, blockJSON, tx);
}

async undo({ block, tx }) {
Expand All @@ -347,20 +352,6 @@ class Blocks extends EventEmitter {
this.exceptions,
tx,
);

await undoBlockStep(this.dposModule, block, tx);
}

async save({ block, blockJSON, tx, skipSave }) {
await saveBlockStep(
this.storage,
this.dposModule,
block,
blockJSON,
skipSave,
tx,
);
this._lastBlock = block;
}

async remove({ block, blockJSON, tx }, saveTempBlock = false) {
Expand Down
36 changes: 0 additions & 36 deletions framework/src/modules/chain/blocks/chain.js
Expand Up @@ -184,32 +184,6 @@ const applyConfirmedGenesisStep = async (
return block;
};

/**
* Calls saveBlock for the block and performs round tick
*
* @private
* @param {Object} storage - Storage component with write methods
* @param {Object} dposModule - Dpos module class
* @param {Object} block - Block object
* @param {boolean} skipSave - Flag to save block into database
* @param {function} tx - Database transaction for atomic operations
* @returns {Promise<reject|resolve>}
*/
const saveBlockStep = async (
storage,
dposModule,
block,
blockJSON,
skipSave,
tx,
) => {
if (!skipSave) {
await saveBlock(storage, blockJSON, tx);
}

await dposModule.apply(block, tx);
lsilvs marked this conversation as resolved.
Show resolved Hide resolved
};

/**
* Reverts confirmed transactions due to block deletion
* @param {Object} block - secondLastBlock
Expand Down Expand Up @@ -245,21 +219,11 @@ const undoConfirmedStep = async (storage, slots, block, exceptions, tx) => {
await stateStore.account.finalize();
};

/**
* Revert given block
* @param {Object} block - block to be undone
* @param {Object} tx - database transaction
*/
const undoBlockStep = async (dposModule, block, tx) =>
dposModule.undo(block, tx);

module.exports = {
saveBlock,
undoBlockStep,
saveBlockBatch,
deleteLastBlock,
deleteFromBlockId,
saveBlockStep,
applyConfirmedStep,
applyConfirmedGenesisStep,
undoConfirmedStep,
Expand Down
5 changes: 3 additions & 2 deletions framework/src/modules/chain/chain.js
Expand Up @@ -416,6 +416,8 @@ module.exports = class Chain {
logger: this.logger,
slots: this.slots,
activeDelegates: this.options.constants.ACTIVE_DELEGATES,
delegateListRoundOffset: this.options.constants
.DELEGATE_LIST_ROUND_OFFSET,
exceptions: this.options.exceptions,
});

Expand All @@ -430,7 +432,6 @@ module.exports = class Chain {
genesisBlock: this.options.genesisBlock,
slots: this.slots,
exceptions: this.options.exceptions,
dposModule: this.dpos,
interfaceAdapters: this.interfaceAdapters,
blockReceiptTimeout: this.options.constants.BLOCK_RECEIPT_TIMEOUT,
loadPerIteration: 1000,
Expand Down Expand Up @@ -468,7 +469,7 @@ module.exports = class Chain {
logger: this.logger,
slots: this.slots,
blocks: this.blocks,
dpos: {},
dposModule: this.dpos,
activeDelegates: this.options.constants.ACTIVE_DELEGATES,
});

Expand Down
25 changes: 17 additions & 8 deletions framework/src/modules/chain/dpos/delegates_info.js
Expand Up @@ -67,7 +67,7 @@ class DelegatesInfo {
this.exceptions = exceptions;
}

async apply(block, tx) {
async apply(block, { tx, delegateListRoundOffset }) {
const undo = false;

/**
Expand All @@ -80,30 +80,33 @@ class DelegatesInfo {
return false;
}

return this._update(block, undo, tx);
return this._update(block, { undo, tx, delegateListRoundOffset });
}

async undo(block, tx) {
async undo(block, { tx, delegateListRoundOffset }) {
const undo = true;

// Never undo genesis block
if (_isGenesisBlock(block)) {
throw new Error('Cannot undo genesis block');
}
return this._update(block, undo, tx);
return this._update(block, { undo, tx, delegateListRoundOffset });
}

/**
* @param {Block} block
*/
async _update(block, undo, tx) {
async _update(block, { undo, tx, delegateListRoundOffset }) {
await this._updateProducedBlocks(block, undo, tx);

// Perform updates that only happens in the end of the round
if (this._isLastBlockOfTheRound(block)) {
const round = this.slots.calcRound(block.height);

const roundSummary = await this._summarizeRound(block, tx);
const roundSummary = await this._summarizeRound(block, {
tx,
delegateListRoundOffset,
});

await Promise.all([
this._updateMissedBlocks(roundSummary, undo, tx),
Expand Down Expand Up @@ -239,7 +242,7 @@ class DelegatesInfo {
* @returns {Object} { earnings: { fee, reward } }
* @returns {Object} { delegateAccount: AccountEntity }
*/
async _summarizeRound(block, tx) {
async _summarizeRound(block, { tx, delegateListRoundOffset }) {
const round = this.slots.calcRound(block.height);
this.logger.debug('Calculating rewards and fees for round: ', round);

Expand Down Expand Up @@ -293,6 +296,7 @@ class DelegatesInfo {

return {
round,
delegateListRoundOffset,
totalFee,
uniqForgersInfo,
};
Expand All @@ -302,9 +306,14 @@ class DelegatesInfo {
}
}

async _getMissedBlocksDelegatePublicKeys({ round, uniqForgersInfo }) {
async _getMissedBlocksDelegatePublicKeys({
round,
delegateListRoundOffset,
uniqForgersInfo,
}) {
const expectedForgingPublicKeys = await this.delegatesList.getForgerPublicKeysForRound(
round,
delegateListRoundOffset,
);

return expectedForgingPublicKeys.filter(
Expand Down
24 changes: 14 additions & 10 deletions framework/src/modules/chain/dpos/delegates_list.js
Expand Up @@ -45,12 +45,16 @@ class DelegatesList {
}

/**
* Get shuffled list of active delegate public keys for a specific round -> forger public keys
* Get shuffled list of active delegate public keys (forger public keys) for a specific round.
* The list of delegates used is the one computed at the end of the round `r - delegateListRoundOffset`
* @param {number} round
* @param {number} delegateListRoundOffset
*/
async getForgerPublicKeysForRound(round) {
async getForgerPublicKeysForRound(round, delegateListRoundOffset) {
// Delegate list is generated from round 1 hence `roundWithOffset` can't be less than 1
const roundWithOffset = Math.max(round - delegateListRoundOffset, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think adding a check against negative offset values make sense? For instance, if round: 6 and I provide offset: -2 I can get the list for round 8. Just saying..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point but I would prefer doing this in another ticket to avoid more changes and tests broken in this PR 😬

const delegatePublicKeys = await this.storage.entities.RoundDelegates.getActiveDelegatesForRound(
round,
roundWithOffset,
);

if (!delegatePublicKeys.length) {
Expand Down Expand Up @@ -124,18 +128,18 @@ class DelegatesList {
* Validates if block was forged by correct delegate
*
* @param {Object} block
* @param {Number} roundOffset - use delegate list generated at the end of `roundOffset` before the current round
* @param {Number} delegateListRoundOffset - use delegate list generated at the end of `delegateListRoundOffset` before the current round
* @return {Boolean} - `true`
* @throw {Error} Failed to verify slot
*/
async verifyBlockForger(block, roundOffset) {
async verifyBlockForger(block, delegateListRoundOffset) {
const currentSlot = this.slots.getSlotNumber(block.timestamp);
const currentRound = this.slots.calcRound(block.height);

// Delegate list is generated from round 1 hence `roundToVerify` can't be less than 1
const roundToVerify = Math.max(currentRound - roundOffset, 1);

const delegateList = await this.getForgerPublicKeysForRound(roundToVerify);
const delegateList = await this.getForgerPublicKeysForRound(
currentRound,
delegateListRoundOffset,
);

if (!delegateList.length) {
throw new Error(
Expand All @@ -160,7 +164,7 @@ class DelegatesList {
* Should be tackled by https://github.com/LiskHQ/lisk-sdk/issues/4194
*/
const { ignoreDelegateListCacheForRounds = [] } = this.exceptions;
if (ignoreDelegateListCacheForRounds.includes(roundToVerify)) {
if (ignoreDelegateListCacheForRounds.includes(currentRound)) {
return true;
}

Expand Down