From 9d277d1c8477e2898382b9122dd459b0cd57e3f7 Mon Sep 17 00:00:00 2001 From: Konstantin Shuplenkov Date: Tue, 6 Dec 2022 20:01:03 +0300 Subject: [PATCH 1/4] feat(drive): skip process proposal if prepare proposal is already called --- .../handlers/prepareProposalHandlerFactory.js | 9 ++++ .../handlers/processProposalHandlerFactory.js | 23 +++++++++ .../blockExecution/BlockExecutionContext.js | 32 ++++++++++++- .../BlockExecutionContextRepository.js | 1 + .../getBlockExecutionContextObjectFixture.js | 32 +++++++++++++ .../test/mock/BlockExecutionContextMock.js | 2 + .../BlockExecutionContextRepository.spec.js | 5 ++ .../prepareProposalHandlerFactory.spec.js | 15 ++++-- .../processProposalHandlerFactory.spec.js | 31 ++++++++++++ .../BlockExecutionContext.spec.js | 47 +++++++++++++++++++ 10 files changed, 193 insertions(+), 4 deletions(-) diff --git a/packages/js-drive/lib/abci/handlers/prepareProposalHandlerFactory.js b/packages/js-drive/lib/abci/handlers/prepareProposalHandlerFactory.js index 3a693ede0d0..ca5d29e8b4b 100644 --- a/packages/js-drive/lib/abci/handlers/prepareProposalHandlerFactory.js +++ b/packages/js-drive/lib/abci/handlers/prepareProposalHandlerFactory.js @@ -145,6 +145,15 @@ function prepareProposalHandlerFactory( + ` (valid txs = ${validTxCount}, invalid txs = ${invalidTxCount})`, ); + proposalBlockExecutionContext.setPrepareProposalResult({ + appHash, + txResults, + consensusParamUpdates, + validatorSetUpdate, + coreChainLockUpdate, + txRecords, + }); + return new ResponsePrepareProposal({ appHash, txResults, diff --git a/packages/js-drive/lib/abci/handlers/processProposalHandlerFactory.js b/packages/js-drive/lib/abci/handlers/processProposalHandlerFactory.js index 2ac48aafc39..ab9a4f68aaf 100644 --- a/packages/js-drive/lib/abci/handlers/processProposalHandlerFactory.js +++ b/packages/js-drive/lib/abci/handlers/processProposalHandlerFactory.js @@ -60,6 +60,29 @@ function processProposalHandlerFactory( consensusLogger.debug('ProcessProposal ABCI method requested'); consensusLogger.trace({ abciRequest: request }); + if (proposalBlockExecutionContext.getHeight() + && proposalBlockExecutionContext.getHeight().toNumber() === height.toNumber() + && proposalBlockExecutionContext.getRound() === round) { + consensusLogger.debug('Returning cached result'); + + const prepareProposalResult = proposalBlockExecutionContext.getPrepareProposalResult(); + + const { + appHash, + txResults, + consensusParamUpdates, + validatorSetUpdate, + } = prepareProposalResult; + + return new ResponseProcessProposal({ + status: proposalStatus.ACCEPT, + appHash, + txResults, + consensusParamUpdates, + validatorSetUpdate, + }); + } + if (coreChainLockUpdate) { const chainLockIsValid = await verifyChainLock(coreChainLockUpdate); diff --git a/packages/js-drive/lib/blockExecution/BlockExecutionContext.js b/packages/js-drive/lib/blockExecution/BlockExecutionContext.js index eec2053dd5b..b1c21afa7cd 100644 --- a/packages/js-drive/lib/blockExecution/BlockExecutionContext.js +++ b/packages/js-drive/lib/blockExecution/BlockExecutionContext.js @@ -219,6 +219,28 @@ class BlockExecutionContext { return this.round; } + /** + * Set PrepareProposal Result + * + * @param {Object} prepareProposalResult + * + * @returns {BlockExecutionContext} + */ + setPrepareProposalResult(prepareProposalResult) { + this.prepareProposalResult = prepareProposalResult; + + return this; + } + + /** + * Get PrepareProposal Result + * + * @return {Object} + */ + getPrepareProposalResult() { + return this.prepareProposalResult; + } + /** * Reset state */ @@ -234,6 +256,7 @@ class BlockExecutionContext { this.round = null; this.epochInfo = null; this.timeMs = null; + this.prepareProposalResult = null; } /** @@ -257,11 +280,12 @@ class BlockExecutionContext { this.height = blockExecutionContext.height; this.coreChainLockedHeight = blockExecutionContext.coreChainLockedHeight; this.version = blockExecutionContext.version; - this.consensusLogger = blockExecutionContext.consensusLogger; + this.consensusLogger = blockExecutionContext.consensusLogger || null; this.withdrawalTransactionsMap = blockExecutionContext.withdrawalTransactionsMap; this.round = blockExecutionContext.round; this.epochInfo = blockExecutionContext.epochInfo; this.timeMs = blockExecutionContext.timeMs; + this.prepareProposalResult = blockExecutionContext.prepareProposalResult || null; } /** @@ -281,11 +305,13 @@ class BlockExecutionContext { this.version = Consensus.fromObject(object.version); this.withdrawalTransactionsMap = object.withdrawalTransactionsMap; this.round = object.round; + this.prepareProposalResult = object.prepareProposalResult; } /** * @param {Object} options * @param {boolean} [options.skipConsensusLogger=false] + * @param {boolean} [options.skipPrepareProposalResult=false] * @return {{ * dataContracts: Object[], * height: number, @@ -322,6 +348,10 @@ class BlockExecutionContext { object.consensusLogger = this.consensusLogger; } + if (!options.skipPrepareProposalResult) { + object.prepareProposalResult = this.prepareProposalResult; + } + return object; } } diff --git a/packages/js-drive/lib/blockExecution/BlockExecutionContextRepository.js b/packages/js-drive/lib/blockExecution/BlockExecutionContextRepository.js index 553b697c787..bcd6d45c585 100644 --- a/packages/js-drive/lib/blockExecution/BlockExecutionContextRepository.js +++ b/packages/js-drive/lib/blockExecution/BlockExecutionContextRepository.js @@ -24,6 +24,7 @@ class BlockExecutionContextRepository { BlockExecutionContextRepository.EXTERNAL_STORE_KEY_NAME, await cbor.encodeAsync(blockExecutionContext.toObject({ skipConsensusLogger: true, + skipPrepareProposalResult: true, })), options, ); diff --git a/packages/js-drive/lib/test/fixtures/getBlockExecutionContextObjectFixture.js b/packages/js-drive/lib/test/fixtures/getBlockExecutionContextObjectFixture.js index 16339b949a9..833a6684ced 100644 --- a/packages/js-drive/lib/test/fixtures/getBlockExecutionContextObjectFixture.js +++ b/packages/js-drive/lib/test/fixtures/getBlockExecutionContextObjectFixture.js @@ -2,6 +2,10 @@ const { tendermint: { abci: { CommitInfo, + ValidatorSetUpdate, + }, + types: { + ConsensusParams, }, }, } = require('@dashevo/abci/types'); @@ -60,6 +64,34 @@ function getBlockExecutionContextObjectFixture(dataContract = getDataContractFix [hash(txTwoBytes).toString('hex')]: txTwoBytes, }, round: 42, + prepareProposalResult: { + appHash: Buffer.alloc(32, 3), + txResults: new Array(3).fill({ code: 0 }), + consensusParamUpdates: new ConsensusParams({ + block: { + maxBytes: 1, + maxGas: 2, + }, + evidence: { + maxAgeDuration: null, + maxAgeNumBlocks: 1, + maxBytes: 2, + }, + version: { + appVersion: 1, + }, + }), + validatorSetUpdate: new ValidatorSetUpdate(), + coreChainLockUpdate: { + coreBlockHeight: 42, + coreBlockHash: '1528e523f4c20fa84ba70dd96372d34e00ce260f357d53ad1a8bc892ebf20e2d', + signature: '1897ce8f54d2070f44ca5c29983b68b391e8137c25e44f67416e579f3e3bdfef7b4fd22db7818399147e52907998857b0fbc8edfdc40a64f2c7df0e88544d31d12ca8c15e73d50dda25ca23f754ed3f789ed4bcb392161995f464017c10df404', + }, + txRecords: [{ + tx: Buffer.alloc(5, 0), + action: 1, + }], + }, }; } diff --git a/packages/js-drive/lib/test/mock/BlockExecutionContextMock.js b/packages/js-drive/lib/test/mock/BlockExecutionContextMock.js index 0b7590ef2b5..dd2f95d5bee 100644 --- a/packages/js-drive/lib/test/mock/BlockExecutionContextMock.js +++ b/packages/js-drive/lib/test/mock/BlockExecutionContextMock.js @@ -55,6 +55,8 @@ class BlockExecutionContextMock { this.getTimeMs = sinon.stub(); this.setRound = sinon.stub(); this.getRound = sinon.stub(); + this.getPrepareProposalResult = sinon.stub(); + this.setPrepareProposalResult = sinon.stub(); } } diff --git a/packages/js-drive/test/integration/blockExecution/BlockExecutionContextRepository.spec.js b/packages/js-drive/test/integration/blockExecution/BlockExecutionContextRepository.spec.js index 240bce1275a..6895d6e4359 100644 --- a/packages/js-drive/test/integration/blockExecution/BlockExecutionContextRepository.spec.js +++ b/packages/js-drive/test/integration/blockExecution/BlockExecutionContextRepository.spec.js @@ -58,6 +58,7 @@ describe('BlockExecutionContextRepository', () => { expect(rawBlockExecutionContext).to.deep.equal(blockExecutionContext.toObject({ skipConsensusLogger: true, + skipPrepareProposalResult: true, })); }); @@ -76,8 +77,10 @@ describe('BlockExecutionContextRepository', () => { expect(fetchedBlockExecutionContext.toObject({ skipConsensusLogger: true, + skipPrepareProposalResult: true, })).to.deep.equal(blockExecutionContext.toObject({ skipConsensusLogger: true, + skipPrepareProposalResult: true, })); }); @@ -100,8 +103,10 @@ describe('BlockExecutionContextRepository', () => { expect(fetchedBlockExecutionContext.toObject({ skipConsensusLogger: true, + skipPrepareProposalResult: true, })).to.deep.equal(blockExecutionContext.toObject({ skipConsensusLogger: true, + skipPrepareProposalResult: true, })); }); }); diff --git a/packages/js-drive/test/unit/abci/handlers/prepareProposalHandlerFactory.spec.js b/packages/js-drive/test/unit/abci/handlers/prepareProposalHandlerFactory.spec.js index ad3a340bfe9..1cf1a028707 100644 --- a/packages/js-drive/test/unit/abci/handlers/prepareProposalHandlerFactory.spec.js +++ b/packages/js-drive/test/unit/abci/handlers/prepareProposalHandlerFactory.spec.js @@ -30,7 +30,7 @@ describe('prepareProposalHandlerFactory', () => { let validatorSetUpdate; let coreChainLockUpdate; let endBlockResult; - let proposalBlockExecutionMock; + let proposalBlockExecutionContextMock; let round; beforeEach(function beforeEach() { @@ -58,7 +58,7 @@ describe('prepareProposalHandlerFactory', () => { }); validatorSetUpdate = new ValidatorSetUpdate(); - proposalBlockExecutionMock = new BlockExecutionContextMock(this.sinon); + proposalBlockExecutionContextMock = new BlockExecutionContextMock(this.sinon); loggerMock = new LoggerMock(this.sinon); @@ -83,7 +83,7 @@ describe('prepareProposalHandlerFactory', () => { prepareProposalHandler = prepareProposalHandlerFactory( deliverTxMock, loggerMock, - proposalBlockExecutionMock, + proposalBlockExecutionContextMock, beginBlockMock, endBlockMock, updateCoreChainLockMock, @@ -166,6 +166,15 @@ describe('prepareProposalHandlerFactory', () => { }, loggerMock, ); + + expect(proposalBlockExecutionContextMock.setPrepareProposalResult).to.be.calledOnceWithExactly({ + appHash, + txResults: new Array(3).fill({ code: 0 }), + consensusParamUpdates, + validatorSetUpdate, + coreChainLockUpdate, + txRecords, + }); }); it('should cut txs that are not fit into the size limit', async () => { diff --git a/packages/js-drive/test/unit/abci/handlers/processProposalHandlerFactory.spec.js b/packages/js-drive/test/unit/abci/handlers/processProposalHandlerFactory.spec.js index f15e4c4cc80..af16f50ed20 100644 --- a/packages/js-drive/test/unit/abci/handlers/processProposalHandlerFactory.spec.js +++ b/packages/js-drive/test/unit/abci/handlers/processProposalHandlerFactory.spec.js @@ -157,4 +157,35 @@ describe('processProposalHandlerFactory', () => { expect(result).to.be.an.instanceOf(ResponseProcessProposal); expect(result.status).to.equal(2); }); + + it('should return prepareProposalResult from execution context', async () => { + proposalBlockExecutionContextMock.getHeight.returns(request.height); + proposalBlockExecutionContextMock.getRound.returns(request.round); + + proposalBlockExecutionContextMock.getPrepareProposalResult.returns({ + appHash, + txResults: new Array(3).fill({ code: 0 }), + consensusParamUpdates, + validatorSetUpdate, + }); + + const result = await processProposalHandler(request); + + expect(proposalBlockExecutionContextMock.getPrepareProposalResult).to.be.calledOnce(); + + expect(result).to.be.an.instanceOf(ResponseProcessProposal); + expect(result.status).to.equal(1); + expect(result.appHash).to.equal(appHash); + expect(result.txResults).to.be.deep.equal(new Array(3).fill({ code: 0 })); + expect(result.consensusParamUpdates).to.be.equal(consensusParamUpdates); + expect(result.validatorSetUpdate).to.be.equal(validatorSetUpdate); + + expect(beginBlockMock).to.not.be.called(); + + expect(deliverTxMock).to.not.be.called(); + + expect(verifyChainLockMock).to.not.be.called(); + + expect(endBlockMock).to.not.be.called(); + }); }); diff --git a/packages/js-drive/test/unit/blockExecution/BlockExecutionContext.spec.js b/packages/js-drive/test/unit/blockExecution/BlockExecutionContext.spec.js index 52e5e4f3ff1..90b09bbb355 100644 --- a/packages/js-drive/test/unit/blockExecution/BlockExecutionContext.spec.js +++ b/packages/js-drive/test/unit/blockExecution/BlockExecutionContext.spec.js @@ -26,6 +26,7 @@ describe('BlockExecutionContext', () => { let version; let epochInfo; let timeMs; + let prepareProposalResult; beforeEach(() => { blockExecutionContext = new BlockExecutionContext(); @@ -42,6 +43,7 @@ describe('BlockExecutionContext', () => { version = Consensus.fromObject(plainObject.version); epochInfo = plainObject.epochInfo; timeMs = plainObject.timeMs; + prepareProposalResult = plainObject.prepareProposalResult; }); describe('#addDataContract', () => { @@ -224,6 +226,30 @@ describe('BlockExecutionContext', () => { }); }); + describe('#setPrepareProposalResult', () => { + it('should set PrepareProposal result', async () => { + const result = blockExecutionContext.setPrepareProposalResult( + plainObject.prepareProposalResult, + ); + + expect(result).to.equal(blockExecutionContext); + + expect(blockExecutionContext.prepareProposalResult).to.deep.equal( + plainObject.prepareProposalResult, + ); + }); + }); + + describe('#getPrepareProposalResult', () => { + it('should get PrepareProposal result', async () => { + blockExecutionContext.prepareProposalResult = plainObject.prepareProposalResult; + + expect(blockExecutionContext.getPrepareProposalResult()).to.deep.equal( + plainObject.prepareProposalResult, + ); + }); + }); + describe('#setTimeMs', () => { it('should set time', async () => { blockExecutionContext.setTimeMs(timeMs); @@ -307,6 +333,7 @@ describe('BlockExecutionContext', () => { blockExecutionContext.timeMs = timeMs; blockExecutionContext.withdrawalTransactionsMap = plainObject.withdrawalTransactionsMap; blockExecutionContext.round = plainObject.round; + blockExecutionContext.prepareProposalResult = plainObject.prepareProposalResult; expect(blockExecutionContext.toObject()).to.deep.equal(plainObject); }); @@ -322,6 +349,7 @@ describe('BlockExecutionContext', () => { blockExecutionContext.round = plainObject.round; blockExecutionContext.epochInfo = epochInfo; blockExecutionContext.timeMs = timeMs; + blockExecutionContext.prepareProposalResult = prepareProposalResult; const result = blockExecutionContext.toObject({ skipConsensusLogger: true }); @@ -329,6 +357,25 @@ describe('BlockExecutionContext', () => { expect(result).to.deep.equal(plainObject); }); + + it('should skipPrepareProposalResult if the option passed', () => { + blockExecutionContext.dataContracts = [dataContract]; + blockExecutionContext.lastCommitInfo = lastCommitInfo; + blockExecutionContext.height = height; + blockExecutionContext.coreChainLockedHeight = coreChainLockedHeight; + blockExecutionContext.version = version; + blockExecutionContext.consensusLogger = logger; + blockExecutionContext.withdrawalTransactionsMap = plainObject.withdrawalTransactionsMap; + blockExecutionContext.round = plainObject.round; + blockExecutionContext.epochInfo = epochInfo; + blockExecutionContext.timeMs = timeMs; + + const result = blockExecutionContext.toObject({ skipPrepareProposalResult: true }); + + delete plainObject.prepareProposalResult; + + expect(result).to.deep.equal(plainObject); + }); }); describe('#fromObject', () => { From bb9b5a213c09531028bded29ad3a4053a988b15e Mon Sep 17 00:00:00 2001 From: Konstantin Shuplenkov Date: Tue, 6 Dec 2022 20:53:00 +0300 Subject: [PATCH 2/4] WIP --- .../lib/abci/handlers/prepareProposalHandlerFactory.js | 2 -- .../fixtures/getBlockExecutionContextObjectFixture.js | 9 --------- 2 files changed, 11 deletions(-) diff --git a/packages/js-drive/lib/abci/handlers/prepareProposalHandlerFactory.js b/packages/js-drive/lib/abci/handlers/prepareProposalHandlerFactory.js index 1322a9b80e1..b90a89101a6 100644 --- a/packages/js-drive/lib/abci/handlers/prepareProposalHandlerFactory.js +++ b/packages/js-drive/lib/abci/handlers/prepareProposalHandlerFactory.js @@ -148,8 +148,6 @@ function prepareProposalHandlerFactory( txResults, consensusParamUpdates, validatorSetUpdate, - coreChainLockUpdate, - txRecords, }); return new ResponsePrepareProposal({ diff --git a/packages/js-drive/lib/test/fixtures/getBlockExecutionContextObjectFixture.js b/packages/js-drive/lib/test/fixtures/getBlockExecutionContextObjectFixture.js index 833a6684ced..c9abca30cd6 100644 --- a/packages/js-drive/lib/test/fixtures/getBlockExecutionContextObjectFixture.js +++ b/packages/js-drive/lib/test/fixtures/getBlockExecutionContextObjectFixture.js @@ -82,15 +82,6 @@ function getBlockExecutionContextObjectFixture(dataContract = getDataContractFix }, }), validatorSetUpdate: new ValidatorSetUpdate(), - coreChainLockUpdate: { - coreBlockHeight: 42, - coreBlockHash: '1528e523f4c20fa84ba70dd96372d34e00ce260f357d53ad1a8bc892ebf20e2d', - signature: '1897ce8f54d2070f44ca5c29983b68b391e8137c25e44f67416e579f3e3bdfef7b4fd22db7818399147e52907998857b0fbc8edfdc40a64f2c7df0e88544d31d12ca8c15e73d50dda25ca23f754ed3f789ed4bcb392161995f464017c10df404', - }, - txRecords: [{ - tx: Buffer.alloc(5, 0), - action: 1, - }], }, }; } From caffa321a748746fe65fa796e63aaee644d701c2 Mon Sep 17 00:00:00 2001 From: Konstantin Shuplenkov Date: Wed, 7 Dec 2022 11:51:30 +0300 Subject: [PATCH 3/4] WIP --- .../unit/abci/handlers/prepareProposalHandlerFactory.spec.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/js-drive/test/unit/abci/handlers/prepareProposalHandlerFactory.spec.js b/packages/js-drive/test/unit/abci/handlers/prepareProposalHandlerFactory.spec.js index fb6b3d4ff2c..6631ea60c05 100644 --- a/packages/js-drive/test/unit/abci/handlers/prepareProposalHandlerFactory.spec.js +++ b/packages/js-drive/test/unit/abci/handlers/prepareProposalHandlerFactory.spec.js @@ -179,8 +179,6 @@ describe('prepareProposalHandlerFactory', () => { txResults: new Array(3).fill({ code: 0 }), consensusParamUpdates, validatorSetUpdate, - coreChainLockUpdate, - txRecords, }); }); From fcbfcdf901508417dacfa7a9794e038c41bc2915 Mon Sep 17 00:00:00 2001 From: Konstantin Shuplenkov Date: Wed, 7 Dec 2022 17:14:56 +0300 Subject: [PATCH 4/4] WIP --- .../lib/abci/handlers/processProposalHandlerFactory.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/js-drive/lib/abci/handlers/processProposalHandlerFactory.js b/packages/js-drive/lib/abci/handlers/processProposalHandlerFactory.js index 344c32b63b5..a65523b2477 100644 --- a/packages/js-drive/lib/abci/handlers/processProposalHandlerFactory.js +++ b/packages/js-drive/lib/abci/handlers/processProposalHandlerFactory.js @@ -62,13 +62,13 @@ function processProposalHandlerFactory( consensusLogger.debug('ProcessProposal ABCI method requested'); consensusLogger.trace({ abciRequest: request }); - if (proposalBlockExecutionContext.getHeight() + const prepareProposalResult = proposalBlockExecutionContext.getPrepareProposalResult(); + + if (prepareProposalResult && proposalBlockExecutionContext.getHeight().toNumber() === height.toNumber() && proposalBlockExecutionContext.getRound() === round) { consensusLogger.debug('Returning cached result'); - const prepareProposalResult = proposalBlockExecutionContext.getPrepareProposalResult(); - const { appHash, txResults,