fix(drive): non-deterministic run of mn identities sync (#910)

packages/js-drive/lib/abci/handlers/proposal/beginBlockFactory.js

@@ -108,7 +108,7 @@ function beginBlockFactory(
     proposalBlockExecutionContext.setLastCommitInfo(lastCommitInfo);
 
     // Update SML
-    const isSimplifiedMasternodeListUpdated = await updateSimplifiedMasternodeList(
+    await updateSimplifiedMasternodeList(
       coreChainLockedHeight,
       {
         logger: contextLogger,
@@ -144,8 +144,7 @@ function beginBlockFactory(
       validatorSetQuorumHash: quorumHash,
       coreChainLockedHeight,
       lastSyncedCoreHeight,
-      // TODO: Since we don't have HPMNs now and every masternode can be a validator,
-      //  we pass the whole list
+      // TODO: We should pass only HPMNs
       totalHpmns: simplifiedMasternodeList.getStore()
         .getCurrentSML()
         .getValidMasternodesList()
@@ -191,19 +190,18 @@ function beginBlockFactory(
     }
 
     // Synchronize masternode identities
+    const blockInfo = BlockInfo.createFromBlockExecutionContext(proposalBlockExecutionContext);
 
-    if (isSimplifiedMasternodeListUpdated) {
-      const blockInfo = BlockInfo.createFromBlockExecutionContext(proposalBlockExecutionContext);
-
-      const synchronizeMasternodeIdentitiesResult = await synchronizeMasternodeIdentities(
-        coreChainLockedHeight,
-        blockInfo,
-      );
+    const synchronizeMasternodeIdentitiesResult = await synchronizeMasternodeIdentities(
+      coreChainLockedHeight,
+      blockInfo,
+    );
 
-      const {
-        createdEntities, updatedEntities, removedEntities, fromHeight, toHeight,
-      } = synchronizeMasternodeIdentitiesResult;
+    const {
+      createdEntities, updatedEntities, removedEntities, fromHeight, toHeight,
+    } = synchronizeMasternodeIdentitiesResult;
 
+    if (fromHeight !== toHeight) {
       contextLogger.info(
         `Masternode identities are synced for heights from ${fromHeight} to ${toHeight}: ${createdEntities.length} created, ${updatedEntities.length} updated, ${removedEntities.length} removed`,
       );

packages/js-drive/lib/identity/masternode/synchronizeMasternodeIdentitiesFactory.js

@@ -58,8 +58,6 @@ function synchronizeMasternodeIdentitiesFactory(
   lastSyncedCoreHeightRepository,
   fetchSimplifiedMNList,
 ) {
-  let lastSyncedCoreHeight = 0;
-
   /**
    * @typedef synchronizeMasternodeIdentities
    * @param {number} coreHeight
@@ -73,18 +71,23 @@ function synchronizeMasternodeIdentitiesFactory(
    * }>}
    */
   async function synchronizeMasternodeIdentities(coreHeight, blockInfo) {
+    // TODO: Must be moved outside of this function
+    const lastSyncedHeightResult = await lastSyncedCoreHeightRepository.fetch({
+      useTransaction: true,
+    });
+
+    const lastSyncedCoreHeight = lastSyncedHeightResult.getValue() || 0;
+
     let result = {
       createdEntities: [],
       updatedEntities: [],
       removedEntities: [],
+      fromHeight: lastSyncedCoreHeight,
+      toHeight: coreHeight,
     };
 
-    if (!lastSyncedCoreHeight) {
-      const lastSyncedHeightResult = await lastSyncedCoreHeightRepository.fetch({
-        useTransaction: true,
-      });
-
-      lastSyncedCoreHeight = lastSyncedHeightResult.getValue() || 0;
+    if (lastSyncedCoreHeight === coreHeight) {
+      return result;
     }
 
     let newMasternodes;
@@ -242,9 +245,7 @@ function synchronizeMasternodeIdentitiesFactory(
       result = mergeEntities(result, affectedEntities);
     }
 
-    result.fromHeight = lastSyncedCoreHeight;
-    result.toHeight = coreHeight;
-
+    // TODO: Must be moved outside of this function
     await lastSyncedCoreHeightRepository.store(coreHeight, {
       useTransaction: true,
     });

packages/js-drive/test/integration/identity/masternode/synchronizeMasternodeIdentitiesFactory.spec.js

@@ -520,6 +520,8 @@ describe.skip('synchronizeMasternodeIdentitiesFactory', function main() {
     }
   });
 
+  it('should do nothing if last synced height is equal to the current height');
+
   it('should create identities for all masternodes on the first sync', async () => {
     const result = await synchronizeMasternodeIdentities(coreHeight, blockInfo);
 

packages/js-drive/test/unit/abci/handlers/proposal/beginBlockFactory.spec.js

@@ -188,7 +188,10 @@ describe('beginBlockFactory', () => {
       coreChainLockedHeight, { logger: loggerMock },
     );
 
-    expect(synchronizeMasternodeIdentitiesMock).to.not.been.called();
+    expect(synchronizeMasternodeIdentitiesMock).to.be.calledWithExactly(
+      coreChainLockedHeight,
+      blockInfo,
+    );
 
     expect(executionTimerMock.clearTimer).to.be.calledTwice();
     expect(executionTimerMock.clearTimer.getCall(1)).to.be.calledWithExactly('roundExecution');