Merge pull request #672 from b-tarczynski/contracts/inconsistent-depo…

…sit-subtree-ids

Inconsistent deposit subtree IDs after rollback

contracts/DepositManager.sol

@@ -69,17 +69,21 @@ contract SubtreeQueue {
 
     event DepositSubTreeReady(uint256 subtreeID, bytes32 subtreeRoot);
 
-    function enqueue(bytes32 subtreeRoot) internal returns (uint256 subtreeID) {
+    function unshift(bytes32 subtreeRoot) internal {
+        uint256 subtreeID = front - 1;
+        require(subtreeID > 0, "Deposit Core: No subtrees to unshift");
+        front = subtreeID;
+        queue[subtreeID] = subtreeRoot;
+    }
+
+    function push(bytes32 subtreeRoot) internal returns (uint256 subtreeID) {
         subtreeID = back + 1;
         back = subtreeID;
         queue[subtreeID] = subtreeRoot;
         emit DepositSubTreeReady(subtreeID, subtreeRoot);
     }
 
-    function dequeue()
-        internal
-        returns (uint256 subtreeID, bytes32 subtreeRoot)
-    {
+    function shift() internal returns (uint256 subtreeID, bytes32 subtreeRoot) {
         subtreeID = front;
         require(back >= subtreeID, "Deposit Core: Queue should be non-empty");
         subtreeRoot = queue[subtreeID];
@@ -132,7 +136,7 @@ contract DepositCore is SubtreeQueue {
 
         // Subtree is ready, send to SubtreeQueue
         if (numDeposits == paramMaxSubtreeSize) {
-            subtreeID = enqueue(babyTrees[0]);
+            subtreeID = push(babyTrees[0]);
             // reset
             babyTreesLength = 0;
             depositCount = 0;
@@ -234,10 +238,10 @@ contract DepositManager is
         onlyRollup
         returns (uint256 subtreeID, bytes32 subtreeRoot)
     {
-        return dequeue();
+        return shift();
     }
 
     function reenqueue(bytes32 subtreeRoot) external override onlyRollup {
-        enqueue(subtreeRoot);
+        unshift(subtreeRoot);
     }
 }

contracts/test/TestRollup.sol

@@ -19,9 +19,7 @@ contract MockDepositManager is IDepositManager {
         return (0, bytes32(0));
     }
 
-    function reenqueue(bytes32 subtreeRoot) external override {
-        emit DepositSubTreeReady(0, subtreeRoot);
-    }
+    function reenqueue(bytes32 subtreeRoot) external override {}
 }
 
 contract TestRollup is BatchManager {

test/fast/rollback.test.ts

@@ -123,28 +123,4 @@ describe("Rollback", function() {
         assert.equal(Number(await rollup.invalidBatchMarker()), 0);
         assert.equal(await getTipBatchID(), goodBatchID);
     });
-    it("Test rollback with deposits", async function() {
-        const badBatchID = await getTipBatchID();
-        const [subtree1, subtree2, subtree3] = [
-            randHex(32),
-            randHex(32),
-            randHex(32)
-        ];
-        await rollup.submitDeposits(subtree1, { value: param.STAKE_AMOUNT });
-        await rollup.submitDummyBatch({ value: param.STAKE_AMOUNT });
-        await rollup.submitDeposits(subtree2, { value: param.STAKE_AMOUNT });
-        await rollup.submitDummyBatch({ value: param.STAKE_AMOUNT });
-        await rollup.submitDeposits(subtree3, { value: param.STAKE_AMOUNT });
-        const tx = await rollup.testRollback(badBatchID, { gasLimit: 1000000 });
-        const events = await depositManager.queryFilter(
-            depositManager.filters.DepositSubTreeReady(),
-            tx.blockHash
-        );
-        assert.equal(events.length, 3);
-        const [event1, event2, event3] = events;
-        // Since we are rolling "back", the events are emitted in reverse order
-        assert.equal(event1.args?.subtreeRoot, subtree3);
-        assert.equal(event2.args?.subtreeRoot, subtree2);
-        assert.equal(event3.args?.subtreeRoot, subtree1);
-    });
 });

test/slow/rollup.deposit.test.ts

@@ -0,0 +1,195 @@
+import { deployAll } from "../../ts/deploy";
+import { TESTING_PARAMS } from "../../ts/constants";
+import { ethers } from "hardhat";
+import { StateTree } from "../../ts/stateTree";
+import { AccountRegistry } from "../../ts/accountTree";
+import { serialize } from "../../ts/tx";
+import * as mcl from "../../ts/mcl";
+import { allContracts } from "../../ts/allContractsInterfaces";
+import chai, { assert } from "chai";
+import chaiAsPromised from "chai-as-promised";
+import { getGenesisProof, TransferCommitment } from "../../ts/commitments";
+import { ERC20ValueFactory, USDT } from "../../ts/decimal";
+import { hexToUint8Array } from "../../ts/utils";
+import { Group, txTransferFactory } from "../../ts/factory";
+import { deployKeyless } from "../../ts/deployment/deploy";
+import { handleNewBatch } from "../../ts/client/features/deposit";
+import { Batch } from "../../ts/client/features/interface";
+import { BigNumberish } from "ethers";
+import { State } from "../../ts/state";
+
+chai.use(chaiAsPromised);
+
+const DOMAIN = hexToUint8Array(
+    "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
+);
+
+describe("Rollup Deposit", async function() {
+    const tokenID = 0;
+    let contracts: allContracts;
+    let stateTree: StateTree;
+    let registry: AccountRegistry;
+    let users: Group;
+    let genesisRoot: string;
+    let erc20: ERC20ValueFactory;
+
+    before(async function() {
+        await mcl.init();
+    });
+
+    beforeEach(async function() {
+        const [signer] = await ethers.getSigners();
+
+        users = Group.new({
+            n: 32,
+            initialStateID: 0,
+            initialPubkeyID: 0,
+            domain: DOMAIN
+        });
+
+        stateTree = new StateTree(TESTING_PARAMS.MAX_DEPTH);
+
+        const initialBalance = USDT.fromHumanValue("55.6").l2Value;
+        users
+            .connect(stateTree)
+            .createStates({ initialBalance, tokenID, zeroNonce: true });
+
+        genesisRoot = stateTree.root;
+
+        await deployKeyless(signer, false);
+        contracts = await deployAll(signer, {
+            ...TESTING_PARAMS,
+            BLOCKS_TO_FINALISE: 10,
+            GENESIS_STATE_ROOT: genesisRoot
+        });
+
+        registry = await AccountRegistry.new(contracts.blsAccountRegistry);
+
+        for (const user of users.userIterator()) {
+            const pubkeyID = await registry.register(user.pubkey);
+            assert.equal(pubkeyID, user.pubkeyID);
+        }
+
+        const { exampleToken, depositManager } = contracts;
+        erc20 = new ERC20ValueFactory(await exampleToken.decimals());
+        await exampleToken.approve(
+            depositManager.address,
+            erc20.fromHumanValue("1000000").l1Value
+        );
+    });
+
+    it("reenqueue deposit subtree on rollback", async function() {
+        const feeReceiver = users.getUser(0).stateID;
+        const { rollup, depositManager } = contracts;
+        const { txs, signature } = txTransferFactory(
+            users,
+            TESTING_PARAMS.MAX_TXS_PER_COMMIT
+        );
+
+        const postBatchStateTree = new StateTree(TESTING_PARAMS.MAX_DEPTH);
+        const commitment = TransferCommitment.new(
+            postBatchStateTree.root,
+            registry.root(),
+            signature,
+            feeReceiver,
+            serialize(txs)
+        );
+
+        const targetBatch = commitment.toBatch();
+        const transferBatchID = 1;
+        const _txSubmit = await targetBatch.submit(
+            rollup,
+            transferBatchID,
+            TESTING_PARAMS.STAKE_AMOUNT
+        );
+        await _txSubmit.wait();
+
+        const subtreeRoots = await submitTwoDepositBatches(
+            targetBatch,
+            2,
+            postBatchStateTree,
+            0
+        );
+
+        const { proofs } = stateTree.processTransferCommit(txs, feeReceiver);
+        const _tx = await rollup.disputeTransitionTransfer(
+            transferBatchID,
+            getGenesisProof(genesisRoot),
+            targetBatch.proof(0),
+            proofs,
+            { gasLimit: 10000000 }
+        );
+        const receipt = await _tx.wait();
+        console.log("disputeBatch execution cost", receipt.gasUsed.toNumber());
+
+        for (let i = 0; i < subtreeRoots.length; i++) {
+            const root = await depositManager.queue(i + 1);
+            assert.equal(root, subtreeRoots[i]);
+        }
+    }).timeout(120000);
+
+    async function submitTwoDepositBatches(
+        previousBatch: Batch,
+        batchID: number,
+        stateTree: StateTree,
+        startStateID: number
+    ): Promise<string[]> {
+        const submitResult1 = await submitDepositBatch(
+            previousBatch,
+            batchID,
+            stateTree,
+            startStateID
+        );
+
+        const submitResult2 = await submitDepositBatch(
+            submitResult1.batch,
+            batchID + 1,
+            stateTree,
+            startStateID + 4
+        );
+
+        return [submitResult1.subtreeRoot, submitResult2.subtreeRoot];
+    }
+
+    async function submitDepositBatch(
+        previousBatch: Batch,
+        batchID: BigNumberish,
+        stateTree: StateTree,
+        startStateID: number
+    ): Promise<{
+        subtreeRoot: string;
+        batch: Batch;
+    }> {
+        const { depositManager, rollup } = contracts;
+
+        const amount = erc20.fromHumanValue("10");
+
+        const vacancyProof = stateTree.getVacancyProof(
+            startStateID,
+            TESTING_PARAMS.MAX_DEPOSIT_SUBTREE_DEPTH
+        );
+
+        const nDeposits = 2 ** TESTING_PARAMS.MAX_DEPOSIT_SUBTREE_DEPTH;
+        for (let i = 0; i < nDeposits; i++) {
+            await depositManager.depositFor(i, amount.l1Value, tokenID);
+            const state = State.new(i, tokenID, amount.l2Value, 0);
+            stateTree.createState(startStateID + i, state);
+        }
+        const _txSubmit = await rollup.submitDeposits(
+            batchID,
+            previousBatch.proofCompressed(0),
+            vacancyProof,
+            { value: TESTING_PARAMS.STAKE_AMOUNT }
+        );
+        await _txSubmit.wait();
+
+        const [event] = await rollup.queryFilter(
+            rollup.filters.NewBatch(null, null, null),
+            _txSubmit.blockHash
+        );
+        return {
+            subtreeRoot: await rollup.deposits(2),
+            batch: await handleNewBatch(event, rollup)
+        };
+    }
+});