From 88989971beefaf081a75cf5869e1151f601647c0 Mon Sep 17 00:00:00 2001 From: Jacob Caban-Tomski Date: Tue, 17 Aug 2021 00:35:41 -0600 Subject: [PATCH] WIP Allow disputing a transfer from/to a non-existent state. Add stateHash field to StateMerkleProof to allow proof to dispute empty to/from indexes. Sync some enums from Types.sol to match up in TS types in client. Add more extensive test cases for disputeTransitionTransfer for triggering rollbacks. Typo signautre -> signature. --- contracts/libs/Transition.sol | 24 ++- contracts/libs/Types.sol | 11 +- contracts/rollup/Rollup.sol | 18 +- test/slow/rollup.transfer.test.ts | 331 ++++++++++++++++++++++-------- ts/client/features/transfer.ts | 2 +- ts/interfaces.ts | 7 +- ts/tx.ts | 2 +- 7 files changed, 294 insertions(+), 101 deletions(-) diff --git a/contracts/libs/Transition.sol b/contracts/libs/Transition.sol index 7e0bd099..361c2a82 100644 --- a/contracts/libs/Transition.sol +++ b/contracts/libs/Transition.sol @@ -124,15 +124,25 @@ library Transition { uint256 fee, Types.StateMerkleProof memory proof ) internal pure returns (bytes32 newRoot, Types.Result) { + // disputer must first justify the inclusion of the stateHash require( MerkleTree.verify( stateRoot, - keccak256(proof.state.encode()), + proof.stateHash, senderStateIndex, proof.witness ), "Transition: Sender does not exist" ); + // check if sender is empty + if (proof.stateHash == Types.ZERO_BYTES32) + return (bytes32(0), Types.Result.BadFromIndex); + // sender is non-empty, the disputer now has to justify the preimage of the state + require( + proof.stateHash == keccak256(proof.state.encode()), + "stateHash mismatch" + ); + (Types.UserState memory newSender, Types.Result result) = validateAndApplySender(tokenID, amount, fee, proof.state); if (result != Types.Result.Ok) return (bytes32(0), result); @@ -151,15 +161,25 @@ library Transition { uint256 amount, Types.StateMerkleProof memory proof ) internal pure returns (bytes32 newRoot, Types.Result) { + // disputer must first justify the inclusion of the stateHash require( MerkleTree.verify( stateRoot, - keccak256(proof.state.encode()), + proof.stateHash, receiverStateIndex, proof.witness ), "Transition: receiver does not exist" ); + // check if receiver is empty + if (proof.stateHash == Types.ZERO_BYTES32) + return (bytes32(0), Types.Result.BadFromIndex); + // receiver is non-empty, the disputer now has to justify the preimage of the state + require( + proof.stateHash == keccak256(proof.state.encode()), + "stateHash mismatch" + ); + (Types.UserState memory newReceiver, Types.Result result) = validateAndApplyReceiver(tokenID, amount, proof.state); if (result != Types.Result.Ok) return (bytes32(0), result); diff --git a/contracts/libs/Types.sol b/contracts/libs/Types.sol index 3abe7f37..2b898f72 100644 --- a/contracts/libs/Types.sol +++ b/contracts/libs/Types.sol @@ -7,6 +7,9 @@ pragma solidity ^0.6.12; library Types { // prettier-ignore uint256 public constant ADDRESS_MASK = 0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff; + bytes32 public constant ZERO_BYTES32 = + 0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563; + struct SignatureProof { Types.UserState[] states; bytes32[][] stateWitnesses; @@ -225,6 +228,7 @@ library Types { struct StateMerkleProof { UserState state; + bytes32 stateHash; bytes32[] witness; } @@ -239,6 +243,9 @@ library Types { bytes32[] witness; } + /** + * @notice Results of a validation check on a transaction or commit + */ enum Result { Ok, InvalidTokenAmount, @@ -250,6 +257,8 @@ library Types { BadWithdrawRoot, BadCompression, TooManyTx, - BadPrecompileCall + BadPrecompileCall, + BadFromIndex, + BadToIndex } } diff --git a/contracts/rollup/Rollup.sol b/contracts/rollup/Rollup.sol index e57d8277..2b952bdb 100644 --- a/contracts/rollup/Rollup.sol +++ b/contracts/rollup/Rollup.sol @@ -28,8 +28,10 @@ contract Rollup is BatchManager, EIP712, IEIP712 { string public constant DOMAIN_NAME = "Hubble"; string public constant DOMAIN_VERSION = "1"; - bytes32 public constant ZERO_BYTES32 = - 0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563; + /** + * @dev If this is not being externally consumed, it can be removed. + */ + bytes32 public immutable ZERO_BYTES32; // External contracts BLSAccountRegistry public immutable accountRegistry; @@ -69,6 +71,8 @@ contract Rollup is BatchManager, EIP712, IEIP712 { ) EIP712(DOMAIN_NAME, DOMAIN_VERSION) { + ZERO_BYTES32 = Types.ZERO_BYTES32; + accountRegistry = _accountRegistry; transfer = _transfer; massMigration = _massMigration; @@ -80,11 +84,11 @@ contract Rollup is BatchManager, EIP712, IEIP712 { ); bytes32 genesisCommitment = - keccak256(abi.encode(genesisStateRoot, ZERO_BYTES32)); + keccak256(abi.encode(genesisStateRoot, Types.ZERO_BYTES32)); // Same effect as `MerkleTree.merklize` bytes32 commitmentRoot = - keccak256(abi.encode(genesisCommitment, ZERO_BYTES32)); + keccak256(abi.encode(genesisCommitment, Types.ZERO_BYTES32)); batches[nextBatchID] = Types.Batch({ commitmentRoot: commitmentRoot, meta: Types.encodeMeta( @@ -355,13 +359,15 @@ contract Rollup is BatchManager, EIP712, IEIP712 { vacant.witness ); bytes32 depositCommitment = - keccak256(abi.encode(newRoot, ZERO_BYTES32)); + keccak256(abi.encode(newRoot, Types.ZERO_BYTES32)); // Same effect as `MerkleTree.merklize` - bytes32 root = keccak256(abi.encode(depositCommitment, ZERO_BYTES32)); + bytes32 root = + keccak256(abi.encode(depositCommitment, Types.ZERO_BYTES32)); // AccountRoot doesn't matter for deposit, add dummy value submitBatch(root, 1, bytes32(0), Types.Usage.Deposit); } + // TODO Add note about proofs beng dif in this one than C2T and MM? function disputeTransitionTransfer( uint256 batchID, Types.CommitmentInclusionProof memory previous, diff --git a/test/slow/rollup.transfer.test.ts b/test/slow/rollup.transfer.test.ts index 392f2dd8..495dcc55 100644 --- a/test/slow/rollup.transfer.test.ts +++ b/test/slow/rollup.transfer.test.ts @@ -1,19 +1,22 @@ +import { flatten } from "lodash"; 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 { getAggregateSig, serialize, TxTransfer } 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 { USDT } from "../../ts/decimal"; +import { float16, USDT } from "../../ts/decimal"; import { hexToUint8Array } from "../../ts/utils"; -import { Group, txTransferFactory } from "../../ts/factory"; +import { Group, txTransferFactory, User } from "../../ts/factory"; import { deployKeyless } from "../../ts/deployment/deploy"; import { handleNewBatch } from "../../ts/client/batchHandler"; +import { BigNumber } from "ethers"; +import { Result } from "../../ts/interfaces"; chai.use(chaiAsPromised); @@ -21,18 +24,29 @@ const DOMAIN = hexToUint8Array( "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" ); +class BadCompressionTxTransfer extends TxTransfer { + constructor() { + super(0, 0, BigNumber.from(0), BigNumber.from(0), 0); + } + + public encode(): string { + return "0x1337"; + } +} + describe("Rollup Transfer", async function() { const tokenID = 1; + const initialBalance = USDT.fromHumanValue("55.6").l2Value; let contracts: allContracts; let stateTree: StateTree; let registry: AccountRegistry; let users: Group; let genesisRoot: string; + let feeReceiver: number; before(async function() { await mcl.init(); }); - beforeEach(async function() { const [signer] = await ethers.getSigners(); @@ -45,11 +59,13 @@ describe("Rollup Transfer", async function() { stateTree = new StateTree(TESTING_PARAMS.MAX_DEPTH); - const initialBalance = USDT.fromHumanValue("55.6").l2Value; users .connect(stateTree) .createStates({ initialBalance, tokenID, zeroNonce: true }); + const user0 = users.getUser(0); + feeReceiver = user0.stateID; + genesisRoot = stateTree.root; await deployKeyless(signer, false); @@ -66,91 +82,228 @@ describe("Rollup Transfer", async function() { } }); - it("fails if batchID is incorrect", async function() { - const feeReceiver = users.getUser(0).stateID; - const { txs, signature } = txTransferFactory( - users, - TESTING_PARAMS.MAX_TXS_PER_COMMIT - ); - - const commit = TransferCommitment.new( - stateTree.root, - registry.root(), - signature, - feeReceiver, - serialize(txs) - ); - const batch = commit.toBatch(); - - const invalidBatchID = 666; - await assert.isRejected( - batch.submit( - contracts.rollup, - invalidBatchID, + describe("submit", () => { + it("fails if batchID is incorrect", async function() { + const { txs, signature } = txTransferFactory( + users, + TESTING_PARAMS.MAX_TXS_PER_COMMIT + ); + + const commit = TransferCommitment.new( + stateTree.root, + registry.root(), + signature, + feeReceiver, + serialize(txs) + ); + const batch = commit.toBatch(); + + const invalidBatchID = 666; + await assert.isRejected( + batch.submit( + contracts.rollup, + invalidBatchID, + TESTING_PARAMS.STAKE_AMOUNT + ), + /.*batchID does not match nextBatchID.*/ + ); + }); + + it("submit a batch and dispute", async function() { + const { rollup } = contracts; + const { txs, signature } = txTransferFactory( + users, + TESTING_PARAMS.MAX_TXS_PER_COMMIT + ); + + const { proofs } = stateTree.processTransferCommit( + txs, + feeReceiver + ); + + const commitment = TransferCommitment.new( + stateTree.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 - ), - /.*batchID does not match nextBatchID.*/ - ); + ); + const _txSubmitReceipt = await _txSubmit.wait(); + console.log( + "submitBatch execution cost", + _txSubmitReceipt.gasUsed.toNumber() + ); + + const [event] = await rollup.queryFilter( + rollup.filters.NewBatch(), + _txSubmit.blockHash + ); + const parsedBatch = await handleNewBatch(event, rollup); + const batchID = event.args?.batchID; + + assert.equal( + parsedBatch.commitmentRoot, + targetBatch.commitmentRoot, + "mismatch commitment tree root" + ); + const previousMP = getGenesisProof(genesisRoot); + const commitmentMP = targetBatch.proof(0); + + const _tx = await rollup.disputeTransitionTransfer( + batchID, + previousMP, + commitmentMP, + proofs + ); + const receipt = await _tx.wait(); + console.log( + "disputeBatch execution cost", + receipt.gasUsed.toNumber() + ); + assert.equal( + (await rollup.invalidBatchMarker()).toNumber(), + 0, + "Good state transition should not rollback" + ); + }).timeout(120000); }); - it("submit a batch and dispute", async function() { - const feeReceiver = users.getUser(0).stateID; - const { rollup } = contracts; - const { txs, signature } = txTransferFactory( - users, - TESTING_PARAMS.MAX_TXS_PER_COMMIT - ); - - const { proofs } = stateTree.processTransferCommit(txs, feeReceiver); - - const commitment = TransferCommitment.new( - stateTree.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 - ); - const _txSubmitReceipt = await _txSubmit.wait(); - console.log( - "submitBatch execution cost", - _txSubmitReceipt.gasUsed.toNumber() - ); - - const [event] = await rollup.queryFilter( - rollup.filters.NewBatch(null, null, null), - _txSubmit.blockHash - ); - const parsedBatch = await handleNewBatch(event, rollup); - const batchID = event.args?.batchID; - - assert.equal( - parsedBatch.commitmentRoot, - targetBatch.commitmentRoot, - "mismatch commitment tree root" - ); - const previousMP = getGenesisProof(genesisRoot); - const commitmentMP = targetBatch.proof(0); - - const _tx = await rollup.disputeTransitionTransfer( - batchID, - previousMP, - commitmentMP, - proofs - ); - const receipt = await _tx.wait(); - console.log("disputeBatch execution cost", receipt.gasUsed.toNumber()); - assert.equal( - (await rollup.invalidBatchMarker()).toNumber(), - 0, - "Good state transition should not rollback" - ); - }).timeout(120000); + describe("disputeTransitionTransfer", () => { + const txFactory = (overrides: Partial, signer?: User) => { + const user0 = users.getUser(0); + const amount = overrides.amount ?? float16.round(BigNumber.from(1)); + const tx = new TxTransfer( + overrides.fromIndex ?? user0.stateID, + overrides.toIndex ?? user0.stateID, + amount, + overrides.fee ?? BigNumber.from(0), + overrides.nonce ?? 0 + ); + const txSigner = signer ?? user0; + tx.signature = txSigner.sign(tx); + return tx; + }; + + [ + { + description: "0 amount", + txsFactory: () => [ + txFactory({ amount: float16.round(BigNumber.from(0)) }) + ], + expectedResult: Result.InvalidTokenAmount + }, + { + description: "not enough token balance", + txsFactory: () => [ + txFactory({ amount: float16.round(initialBalance.mul(2)) }) + ], + expectedResult: Result.NotEnoughTokenBalance + }, + { + description: "bad compression (encoding)", + txsFactory: () => { + const tx = new BadCompressionTxTransfer(); + tx.signature = users.getUser(0).sign(tx); + return [tx]; + }, + expectedResult: Result.BadCompression + }, + { + description: `too many txs in commit (> ${TESTING_PARAMS.MAX_TXS_PER_COMMIT})`, + txsFactory: () => { + const lenTooManyTxs = TESTING_PARAMS.MAX_TXS_PER_COMMIT + 1; + const txs = []; + while (txs.length < lenTooManyTxs) { + txs.push(txFactory({})); + } + return txs; + }, + expectedResult: Result.TooManyTx + }, + { + description: "non-existant fromIndex", + txsFactory: () => [txFactory({ fromIndex: users.size + 1 })], + expectedResult: Result.BadFromIndex + }, + { + description: "non-existant toIndex", + txsFactory: () => [txFactory({ toIndex: users.size + 1 })], + expectedResult: Result.BadToIndex + } + ].forEach(({ description, txsFactory, expectedResult }) => { + it(`dispute and rollback a bad batch with l2 tx(s) with ${description}`, async function() { + const { rollup } = contracts; + + const txs = txsFactory(); + const signature = getAggregateSig(txs); + + // Replace with Array.flat once >= es2019 is supported + const proofs = flatten( + txs.map(tx => [ + stateTree.getState(tx.fromIndex), + stateTree.getState(tx.toIndex) + ]) + ); + + const commitment = TransferCommitment.new( + stateTree.root, + registry.root(), + signature, + feeReceiver, + serialize(txs) + ); + + const targetBatch = commitment.toBatch(); + const transferBatchID = 1; + const submitTx = await targetBatch.submit( + rollup, + transferBatchID, + TESTING_PARAMS.STAKE_AMOUNT + ); + + const [event] = await rollup.queryFilter( + rollup.filters.NewBatch(), + submitTx.blockHash + ); + const parsedBatch = await handleNewBatch(event, rollup); + const batchID = event.args?.batchID; + + assert.equal( + parsedBatch.commitmentRoot, + targetBatch.commitmentRoot, + "mismatch commitment tree root" + ); + const previousMP = getGenesisProof(genesisRoot); + const commitmentMP = targetBatch.proof(0); + + const disputeL1Txn = await rollup.disputeTransitionTransfer( + batchID, + previousMP, + commitmentMP, + proofs + ); + + const invalidBatchMarker = await rollup.invalidBatchMarker(); + assert.equal(invalidBatchMarker.toNumber(), transferBatchID); + + const [rollbackEvent] = await rollup.queryFilter( + rollup.filters.RollbackTriggered(), + disputeL1Txn.blockHash + ); + assert.equal( + rollbackEvent.args?.batchID.toNumber(), + transferBatchID + ); + assert.equal(rollbackEvent.args?.result, expectedResult); + }); + }); + }); }); diff --git a/ts/client/features/transfer.ts b/ts/client/features/transfer.ts index 1258090d..50471407 100644 --- a/ts/client/features/transfer.ts +++ b/ts/client/features/transfer.ts @@ -219,7 +219,7 @@ type TransactionBundle = { export function getAggregateSig(txs: OffchainTx[]): solG1 { const signatures = []; for (const tx of txs) { - if (!tx.signature) throw new Error(`tx has no signautre ${tx}`); + if (!tx.signature) throw new Error(`tx has no signature ${tx}`); signatures.push(tx.signature); } return aggregate(signatures).sol; diff --git a/ts/interfaces.ts b/ts/interfaces.ts index 066eef05..3512fbb0 100644 --- a/ts/interfaces.ts +++ b/ts/interfaces.ts @@ -1,3 +1,4 @@ +// This should stay in sync with contracts/libs/Types.sol:Usage export enum Usage { Genesis, Transfer, @@ -6,6 +7,7 @@ export enum Usage { Deposit } +// This should stay in sync with contracts/libs/Types.sol:Result export enum Result { Ok, InvalidTokenAmount, @@ -16,7 +18,10 @@ export enum Result { MismatchedAmount, BadWithdrawRoot, BadCompression, - TooManyTx + TooManyTx, + BadPrecompileCall, + BadFromIndex, + BadToIndex } export type Wei = string; diff --git a/ts/tx.ts b/ts/tx.ts index 61985778..67d0b795 100644 --- a/ts/tx.ts +++ b/ts/tx.ts @@ -57,7 +57,7 @@ export function serialize(txs: Tx[]): string { export function getAggregateSig(txs: SignableTx[]): solG1 { const signatures = []; for (const tx of txs) { - if (!tx.signature) throw new Error(`tx has no signautre ${tx}`); + if (!tx.signature) throw new Error(`tx has no signature ${tx}`); signatures.push(tx.signature); } return aggregate(signatures).sol;