Adversary being able to produce new blocks after an invalid block

.github/workflows/check-PRs.yml

@@ -92,7 +92,7 @@ jobs:
 run: |
 ./setup-nightfall
 ./geth-standalone -s
-sleep 300
+sleep 300
 ./start-nightfall -l -d -a &> adversary-test.log &disown
 
 - name: Wait for images to be ready

cli/lib/nf3.mjs

@@ -1028,8 +1028,10 @@ class Nf3 {
 const { type, txDataToSign } = msg;
 if (type === 'commit' || type === 'challenge') {
 return new Promise((resolve, reject) => {
+logger.debug('-> Push transaction to challengerQueue');
 challengerQueue.push(async () => {
 try {
+logger.debug('-> Submit transaction from challengerQueue');
 const receipt = await this.submitTransaction(
 txDataToSign,
 this.challengesContractAddress,
@@ -1046,6 +1048,29 @@ class Nf3 {
 };
 }
 
+// eslint-disable-next-line class-methods-use-this
+pauseQueueChallenger() {
+return new Promise(resolve => {
+if (challengerQueue.autostart) {
+// put an event at the head of the queue which will cleanly pause it.
+challengerQueue.unshift(async () => {
+challengerQueue.autostart = false;
+challengerQueue.stop();
+logger.info(`queue challengerQueue has been paused`);
+resolve();
+});
+} else {
+resolve();
+}
+});
+}
+
+// eslint-disable-next-line class-methods-use-this
+unpauseQueueChallenger() {
+challengerQueue.autostart = true;
+challengerQueue.unshift(async () => logger.info(`queue challengerQueue has been unpaused`));
+}
+
 /**
 Returns an emitter, whose 'data' event fires whenever a challengeable block is
 detected, passing out the transaction needed to raise the challenge. This

docker-compose.adversary.yml

@@ -33,5 +33,6 @@ services:
 HASH_TYPE: mimc
 LOG_LEVEL: debug
 IS_CHALLENGER: 'false'
+NONSTOP_QUEUE_AFTER_INVALID_BLOCK: 'true'
 TRANSACTIONS_PER_BLOCK: ${TRANSACTIONS_PER_BLOCK:-2}
 command: ['npm', 'run', 'dev']

docker-compose.yml

@@ -33,7 +33,7 @@ services:
 OPTIMIST_HOST: optimist1
 OPTIMIST_PORT: 80
 USE_STUBS: 'false' # make sure this flag is the same as in deployer service
-command: [ 'npm', 'run', 'dev' ]
+command: ['npm', 'run', 'dev']
 
 client2:
 image: ghcr.io/eyblockchain/nightfall3-client:latest
@@ -66,7 +66,7 @@ services:
 OPTIMIST_HOST: optimist2
 OPTIMIST_PORT: 80
 USE_STUBS: 'false' # make sure this flag is the same as in deployer service
-command: [ 'npm', 'run', 'dev' ]
+command: ['npm', 'run', 'dev']
 
 worker:
 # image: 3800decac71d
@@ -141,7 +141,7 @@ services:
 LOG_LEVEL: debug
 IS_CHALLENGER: 'true'
 TRANSACTIONS_PER_BLOCK: ${TRANSACTIONS_PER_BLOCK:-2}
-command: [ 'npm', 'run', 'dev' ]
+command: ['npm', 'run', 'dev']
 
 optimist2:
 image: ghcr.io/eyblockchain/nightfall3-optimist:latest
@@ -165,7 +165,7 @@ services:
 LOG_LEVEL: error
 IS_CHALLENGER: 'true'
 TRANSACTIONS_PER_BLOCK: ${TRANSACTIONS_PER_BLOCK:-2}
-command: [ 'npm', 'run', 'dev' ]
+command: ['npm', 'run', 'dev']
 
 hosted-utils-api-server:
 build:
@@ -197,9 +197,7 @@ volumes:
 mongodb1: null
 mongodb2: null
 proving_files: null
-build:
-
-null
+build: null
 networks:
 nightfall_network:
 driver: bridge

nightfall-client/src/event-handlers/rollback.mjs

@@ -27,7 +27,7 @@ async function rollbackEventHandler(data) {
 // If we clear the commitments in blockNumberL2, we may spend them again while they are in an optimist mempool.
 const commitments = await getCommitmentsFromBlockNumberL2(Number(blockNumberL2) + 1);
 // Deposit transactions should not be dropped because they are always valid even post-rollback.
-const nonDeposit = commitments.filter(t => t.transactionType !== '0').map(c => c._id);
+const nonDeposit = commitments.filter(c => c.isDeposited === false).map(c => c._id);
 logger.debug(`nonDeposit: ${JSON.stringify(nonDeposit)}`);
 // Any commitments that have been nullified and are now no longer spent because
 // of the rollback should be made available to be spent again.

test/adversary.test.mjs

@@ -108,20 +108,23 @@ describe('Testing with an adversary', () => {
 });
 ercAddress = await nf3User.getContractAddress('ERC20Mock');
 
+// Because rollbacks removes the only registered proposer,
+// the proposer is registered again after each removal
+intervalId = setInterval(() => {
+registerProposerOnNoProposer(nf3AdversarialProposer);
+}, 5000);
+
 // Challenger registration
 await nf3Challenger.registerChallenger();
 // Chalenger listening for incoming events
 nf3Challenger.startChallenger();
+console.log('Pausing challenger queue...');
+// we pause the challenger queue and don't process challenger until unpauseQueueChallenger
+nf3Challenger.pauseQueueChallenger();
 });
 
-describe('User creates deposit and transfer transctions', () => {
-it('User should have the correct balance after a series of rollbacks', async () => {
-// Because rollbacks removes the only registered proposer,
-// the proposer is registered again after each remova
-intervalId = setInterval(() => {
-registerProposerOnNoProposer(nf3AdversarialProposer);
-}, 5000);
-
+describe('User creates deposit and transfer transactions', () => {
+it('User should have the correct balance after without challenge', async () => {
 // we are creating a block of deposits with high values such that there is
 // enough balance for a lot of transfers with low value.
 for (let j = 0; j < TRANSACTIONS_PER_BLOCK; j++) {
@@ -178,6 +181,20 @@ describe('Testing with an adversary', () => {
 console.log(`Completed endBalance`, endBalance);
 expect(expectedBalance).to.be.equal(endBalance - startBalance);
 });
+
+it('User should have the correct balance after challenge and a series of rollbacks', async () => {
+console.log('Unpausing challenger queue...');
+// Challenger unpause
+await nf3Challenger.unpauseQueueChallenger();
+// waiting sometime to ensure that all the good transactions from bad
+// blocks were proposed in other good blocks
+console.log('Waiting for rollbacks...');
+await new Promise(resolve => setTimeout(resolve, 30 * TX_WAIT));
+const endBalance = await retrieveL2Balance(nf3User);
+console.log(`Completed startBalance`, startBalance);
+console.log(`Completed endBalance`, endBalance);
+expect(expectedBalance).to.be.equal(endBalance - startBalance);
+});
 });
 
 after(async () => {

test/adversary/adversary-code/block.mjs

@@ -7,6 +7,13 @@ const error = [
 'ValidBlock',
 'IncorrectTreeRoot', // Needs two prior blocks
 'ValidBlock',
+'ValidBlock',
+/* 'ValidBlock',
+'ValidBlock',
+'ValidBlock',
+'ValidBlock',
+'ValidBlock',
+'ValidBlock',
 'IncorrectLeafCount', // Needs one prior block
 'ValidBlock',
 'DuplicateTransaction', // needs atleast one transaction in a prior block
@@ -16,7 +23,7 @@ const error = [
 'HistoricRootError',
 'ValidBlock',
 'IncorrectProof',
-'ValidBlock',
+'ValidBlock', */
 ];
 
 // eslint-disable-next-line no-unused-vars

test/adversary/adversary-code/database.mjs

@@ -11,6 +11,13 @@ const error = [
 'ValidTransaction',
 'IncorrectTreeRoot',
 'ValidTransaction',
+'ValidTransaction',
+/* 'ValidTransaction',
+'ValidTransaction',
+'ValidTransaction',
+'ValidTransaction',
+'ValidTransaction',
+'ValidTransaction',
 'IncorrectLeafCount',
 'ValidTransaction',
 'DuplicateTransaction',
@@ -20,7 +27,7 @@ const error = [
 'HistoricRootError',
 'ValidTransaction',
 'IncorrectProof',
-'ValidTransaction',
+'ValidTransaction', */
 ];
 
 const duplicateNullifier = async number => {

test/adversary/transpile-adversary.mjs

@@ -38,6 +38,23 @@ const transpileBlockAssembler = (_pathToSrc, _pathToInject) => {
 fs.writeFileSync(_pathToSrc, srcFile);
 };
 
+// Transpile Block Proposed Code that manage block proposed events
+const transpileBlockProposed = _pathToSrc => {
+let srcFile = fs.readFileSync(_pathToSrc, 'utf-8');
+
+// We need to take into account variable NONSTOP_QUEUE_AFTER_INVALID_BLOCK
+const regexReplaceComponents =
+/(await enqueueEvent\(\(\) => logger.info\('Stop Until Rollback'\), 2\);)/g;
+const reComponent = `logger.info(\`NONSTOP_QUEUE_AFTER_INVALID_BLOCK: \${process.env.NONSTOP_QUEUE_AFTER_INVALID_BLOCK}\`);
+if (!process.env.NONSTOP_QUEUE_AFTER_INVALID_BLOCK)
+await enqueueEvent(() => logger.info('Stop Until Rollback'), 2);`;
+srcFile = `/* THIS FILE CONTAINS CODE THAT HAS BEEN AUTOGENERATED DO NOT MODIFY MANUALLY */\n${srcFile.replace(
+regexReplaceComponents,
+reComponent,
+)}`;
+fs.writeFileSync(_pathToSrc, srcFile);
+};
+
 // Transpile Block Code that does block assembly
 const transpileBlockBuilder = (_pathToSrc, _pathToInject) => {
 let srcFile = fs.readFileSync(_pathToSrc, 'utf-8');
@@ -134,6 +151,9 @@ copyDir('./nightfall-optimist/', './test/adversary/nightfall-adversary/').then((
 './test/adversary/nightfall-adversary/src/classes/block.mjs',
 './test/adversary/adversary-code/block.mjs',
 );
+transpileBlockProposed(
+'./test/adversary/nightfall-adversary/src/event-handlers/block-proposed.mjs',
+);
 transpileTransactionLookup(
 './test/adversary/nightfall-adversary/src/services/database.mjs',
 './test/adversary/adversary-code/database.mjs',

wallet/src/nightfall-browser/event-handlers/rollback.js

@@ -29,7 +29,7 @@ async function rollbackEventHandler(data) {
 // If we clear the commitments in blockNumberL2, we may spend them again while they are in an optimist mempool.
 const commitments = await getCommitmentsFromBlockNumberL2(Number(blockNumberL2) + 1);
 // Deposit transactions should not be dropped because they are always valid even post-rollback.
-const nonDeposit = commitments.filter(t => t.transactionType !== '0').map(c => c._id);
+const nonDeposit = commitments.filter(c => c.isDeposited === false).map(c => c._id);
 logger.debug(`nonDeposit: ${JSON.stringify(nonDeposit)}`);
 // Any commitments that have been nullified and are now no longer spent because
 // of the rollback should be made available to be spent again.