diff --git a/src/zkbob/sequencer/ZkBobSequencer.sol b/src/zkbob/sequencer/ZkBobSequencer.sol index 8e1e094..d6facc1 100644 --- a/src/zkbob/sequencer/ZkBobSequencer.sol +++ b/src/zkbob/sequencer/ZkBobSequencer.sol @@ -5,29 +5,23 @@ pragma solidity 0.8.15; import {PriorityQueue, PriorityOperation} from "./PriorityQueue.sol"; import {ZkBobPool} from "../ZkBobPool.sol"; import {SequencerABIDecoder} from "./SequencerABIDecoder.sol"; -import {ZkBobDirectDepositQueue} from "../ZkBobDirectDepositQueue.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; - import {IERC20Permit} from "../../interfaces/IERC20Permit.sol"; contract ZkBobSequencer is SequencerABIDecoder { using PriorityQueue for PriorityQueue.Queue; using SafeERC20 for IERC20; - uint256 constant TRANSFER_DELTA_SIZE = 28; bytes4 internal constant MESSAGE_PREFIX_COMMON_V1 = 0x00000000; uint16 public constant DEPOSIT = uint16(0); uint16 public constant PERMIT_DEPOSIT = uint16(3); - // TODO: make it configurable - uint256 public constant EXPIRATION_TIME = 1 hours; - uint256 public constant PROXY_GRACE_PERIOD = 10 minutes; // Queue of operations PriorityQueue.Queue priorityQueue; // Pool contract, this contract is operator of the pool - ZkBobPool _pool; + ZkBobPool pool; // Last time when queue was updated uint256 lastQueueUpdateTimestamp; @@ -35,13 +29,20 @@ contract ZkBobSequencer is SequencerABIDecoder { mapping(uint256 => bool) public pendingNullifiers; mapping(uint256 => bool) public pendingDirectDeposits; + uint64 public expirationTime; + + uint64 public gracePeriod; + event Commited(); // TODO: Fill the data + event DirectDepositCommited(); event Proved(); event Rejected(); event Skipped(); - constructor(address pool) { - _pool = ZkBobPool(pool); + constructor(address _pool, uint64 _expirationTime, uint64 _gracePeriod) { + pool = ZkBobPool(_pool); + expirationTime = _expirationTime; + gracePeriod = _gracePeriod; } // Possible problems here: @@ -58,15 +59,15 @@ contract ZkBobSequencer is SequencerABIDecoder { ) = _parseCommitCalldata(); require(pendingNullifiers[nullifier] == false, "ZkBobSequencer: nullifier is already pending"); - require(_pool.nullifiers(nullifier) == 0, "ZkBobSequencer: nullifier is spent"); + require(pool.nullifiers(nullifier) == 0, "ZkBobSequencer: nullifier is spent"); require(msg.sender == _parseProver(memo), "ZkBobSequencer: not authorized"); - require(uint96(index) <= _pool.pool_index(), "ZkBobSequencer: index is too high"); - require(_pool.transfer_verifier().verifyProof(transfer_pub(index, nullifier, outCommit, transferDelta, memo), transferProof), "ZkBobSequencer: invalid proof"); + require(uint96(index) <= pool.pool_index(), "ZkBobSequencer: index is too high"); + require(pool.transfer_verifier().verifyProof(transfer_pub(index, nullifier, outCommit, transferDelta, memo), transferProof), "ZkBobSequencer: invalid proof"); require(_parseMessagePrefix(memo, txType) == MESSAGE_PREFIX_COMMON_V1, "ZkBobPool: bad message prefix"); - claimDepositProxyFee(txType, memo, nullifier); + _claimDepositProxyFee(txType, memo, nullifier); - bytes32 hash = commitHash(nullifier, outCommit, transferDelta, transferProof, memo); + bytes32 hash = _commitHash(nullifier, outCommit, transferDelta, transferProof, memo); PriorityOperation memory op = PriorityOperation(hash, nullifier, new uint256[](0), block.timestamp); priorityQueue.pushBack(op); pendingNullifiers[nullifier] = true; @@ -75,12 +76,10 @@ contract ZkBobSequencer is SequencerABIDecoder { } // Possible problems here: - // 1. If the PROXY_GRACE_PERIOD is ended then anyone can prove the operation and race condition is possible - // We can prevent it by implementing some mechanism to pick a prover from the previous ones - // 2. Malicious user can commit a bad operation (low fees, problems with compliance, etc.) so there is no prover that will be ready to prove it + // 1. Malicious user can commit a bad operation (low fees, problems with compliance, etc.) so there is no prover that will be ready to prove it // In this case the prioirity queue will be locked until the expiration time function prove() external { - PriorityOperation memory op = popFirstUnexpiredOperation(); + PriorityOperation memory op = _popFirstUnexpiredOperation(); uint256 nullifier = _transfer_nullifier(); uint48 index = _transfer_index(); @@ -88,13 +87,13 @@ contract ZkBobSequencer is SequencerABIDecoder { uint256[8] calldata transferProof = _transfer_proof(); require( - op.commitHash == commitHash(nullifier, _transfer_out_commit(), _transfer_delta(), transferProof, memo), + op.commitHash == _commitHash(nullifier, _transfer_out_commit(), _transfer_delta(), transferProof, memo), "ZkBobSequencer: invalid commit hash" ); address prover = _parseProver(memo); - uint256 timestamp = max(op.timestamp, lastQueueUpdateTimestamp); - if (block.timestamp <= timestamp + PROXY_GRACE_PERIOD) { + uint256 timestamp = _max(op.timestamp, lastQueueUpdateTimestamp); + if (block.timestamp <= timestamp + gracePeriod) { require(msg.sender == prover, "ZkBobSequencer: not authorized"); } @@ -103,8 +102,8 @@ contract ZkBobSequencer is SequencerABIDecoder { // We check proofs twice with the current implementation. // It should be possible to avoid it but we need to modify pool contract. require( - _pool.tree_verifier().verifyProof( - _tree_pub(_pool.roots(index)), + pool.tree_verifier().verifyProof( + _tree_pub(pool.roots(index)), treeProof ), "ZkBobSequencer: invalid proof" @@ -113,7 +112,7 @@ contract ZkBobSequencer is SequencerABIDecoder { lastQueueUpdateTimestamp = block.timestamp; delete pendingNullifiers[nullifier]; - bool success = propagateToPool(ZkBobPool.transact.selector); + bool success = _propagateToPool(ZkBobPool.transact.selector); // We remove the commitment from the queue regardless of the result of the call to pool contract // If we check that the prover is not malicious then the tx is not valid because of the limits or @@ -134,14 +133,14 @@ contract ZkBobSequencer is SequencerABIDecoder { { // TODO: access control to prevent race condition - uint256 hashsum = _pool.direct_deposit_queue().validateBatch(_indices, _out_commit); + uint256 hashsum = pool.direct_deposit_queue().validateBatch(_indices, _out_commit); for (uint256 i = 0; i < _indices.length; i++) { require(pendingDirectDeposits[_indices[i]] == false, "ZkBobSequencer: direct deposit is already in the queue"); } // verify that _out_commit corresponds to zero output account + 16 chosen notes + 111 empty notes require( - _pool.batch_deposit_verifier().verifyProof([hashsum], _batch_deposit_proof), "ZkBobSequencer: bad batch deposit proof" + pool.batch_deposit_verifier().verifyProof([hashsum], _batch_deposit_proof), "ZkBobSequencer: bad batch deposit proof" ); // Save pending indices @@ -150,9 +149,11 @@ contract ZkBobSequencer is SequencerABIDecoder { } // Save operation in priority queue - bytes32 hash = commitDirectDepositHash(_indices, _out_commit, _batch_deposit_proof); + bytes32 hash = _commitDirectDepositHash(_indices, _out_commit, _batch_deposit_proof); PriorityOperation memory op = PriorityOperation(hash, uint256(0), _indices, block.timestamp); priorityQueue.pushBack(op); + + emit DirectDepositCommited(); } function proveDirectDeposit( @@ -162,25 +163,25 @@ contract ZkBobSequencer is SequencerABIDecoder { uint256[8] memory _batch_deposit_proof, uint256[8] memory _tree_proof ) external { - PriorityOperation memory op = popFirstUnexpiredOperation(); + PriorityOperation memory op = _popFirstUnexpiredOperation(); require( - op.commitHash == commitDirectDepositHash(_indices, _out_commit, _batch_deposit_proof), + op.commitHash == _commitDirectDepositHash(_indices, _out_commit, _batch_deposit_proof), "ZkBobSequencer: invalid commit hash" ); // TODO: Access control to prevent race condition // We need to check that the proof is valid to prevent the case when the prover is malicious - uint256[3] memory tree_pub = [_pool.roots(_pool.pool_index()), _root_after, _out_commit]; - require(_pool.tree_verifier().verifyProof(tree_pub, _tree_proof), "ZkBobSequencer: bad tree proof"); + uint256[3] memory tree_pub = [pool.roots(pool.pool_index()), _root_after, _out_commit]; + require(pool.tree_verifier().verifyProof(tree_pub, _tree_proof), "ZkBobSequencer: bad tree proof"); lastQueueUpdateTimestamp = block.timestamp; for (uint256 i = 0; i < op.directDeposits.length; i++) { delete pendingDirectDeposits[op.directDeposits[i]]; } - bool success = propagateToPool(ZkBobPool.appendDirectDeposits.selector); + bool success = _propagateToPool(ZkBobPool.appendDirectDeposits.selector); if (success) { emit Proved(); @@ -189,14 +190,14 @@ contract ZkBobSequencer is SequencerABIDecoder { } } - function claimDepositProxyFee(uint16 _txType, bytes calldata _memo, uint256 _nullifier) internal { + function _claimDepositProxyFee(uint16 _txType, bytes calldata _memo, uint256 _nullifier) internal { int256 fee = int64(_parseProxyFee(_memo)); if (_txType == DEPOSIT) { - _pool.claimFee(_commitDepositSpender(), fee); + pool.claimFee(_commitDepositSpender(), fee); } else if (_txType == PERMIT_DEPOSIT) { (uint64 expiry, address _user) = _parsePermitData(_memo); (uint8 v, bytes32 r, bytes32 s) = _permittable_signature_proxy_fee(); - _pool.claimFeeUsingPermit( + pool.claimFeeUsingPermit( _user, _nullifier, fee, @@ -209,30 +210,31 @@ contract ZkBobSequencer is SequencerABIDecoder { } function _root() override internal view returns (uint256){ - return _pool.roots(_pool.pool_index()); + return pool.roots(pool.pool_index()); } function _pool_id() override internal view returns (uint256){ - return _pool.pool_id(); + return pool.pool_id(); } - function popFirstUnexpiredOperation() internal returns (PriorityOperation memory) { + function _popFirstUnexpiredOperation() internal returns (PriorityOperation memory) { PriorityOperation memory op = priorityQueue.popFront(); - uint256 timestamp = max(op.timestamp, lastQueueUpdateTimestamp); - while (timestamp + EXPIRATION_TIME < block.timestamp) { + uint256 timestamp = _max(op.timestamp, lastQueueUpdateTimestamp); + while (timestamp + expirationTime < block.timestamp) { delete pendingNullifiers[op.nullifier]; for (uint256 i = 0; i < op.directDeposits.length; i++) { delete pendingDirectDeposits[op.directDeposits[i]]; } // TODO: is it correct? - lastQueueUpdateTimestamp = op.timestamp + EXPIRATION_TIME; + lastQueueUpdateTimestamp = op.timestamp + expirationTime; + emit Skipped(); op = priorityQueue.popFront(); - timestamp = max(op.timestamp, lastQueueUpdateTimestamp); + timestamp = _max(op.timestamp, lastQueueUpdateTimestamp); } return op; } - function commitHash( + function _commitHash( uint256 nullifier, uint256 out_commit, uint256 transfer_delta, @@ -244,7 +246,7 @@ contract ZkBobSequencer is SequencerABIDecoder { return keccak256(abi.encodePacked(nullifier, out_commit, transfer_delta, transfer_proof, memo)); } - function commitDirectDepositHash( + function _commitDirectDepositHash( uint256[] calldata _indices, uint256 _out_commit, uint256[8] memory _batch_deposit_proof @@ -260,18 +262,18 @@ contract ZkBobSequencer is SequencerABIDecoder { uint256 transferDelta, bytes calldata memo ) internal view returns (uint256[5] memory r) { - r[0] = _pool.roots(index); + r[0] = pool.roots(index); r[1] = nullifier; r[2] = outCommit; - r[3] = transferDelta + (_pool.pool_id() << (TRANSFER_DELTA_SIZE * 8)); + r[3] = transferDelta + (pool.pool_id() << (transfer_delta_size * 8)); r[4] = uint256(keccak256(memo)) % R; } - function propagateToPool(bytes4 selector) internal returns (bool success) { - (success, ) = address(_pool).call(abi.encodePacked(selector, msg.data[4:])); + function _propagateToPool(bytes4 selector) internal returns (bool success) { + (success, ) = address(pool).call(abi.encodePacked(selector, msg.data[4:])); } - function max(uint256 a, uint256 b) internal pure returns (uint256 maxValue) { + function _max(uint256 a, uint256 b) internal pure returns (uint256 maxValue) { maxValue = a; if (b > a) { maxValue = b; @@ -283,14 +285,12 @@ contract ZkBobSequencer is SequencerABIDecoder { uint256 head = priorityQueue.getFirstUnprocessedPriorityTx(); uint256 tail = priorityQueue.getTotalPriorityTxs(); - bool found = false; for (uint256 i = head; i <= tail; i++) { - if (op.timestamp + EXPIRATION_TIME >= block.timestamp) { + if (op.timestamp + expirationTime >= block.timestamp) { op = priorityQueue.get(i); - found = true; break; } } - require(found, "ZkBobSequencer: no pending operations"); + require(op.commitHash != bytes32(0), "ZkBobSequencer: no pending operations"); } } \ No newline at end of file diff --git a/test/zkbob/ZkBobSequencer.t.sol b/test/zkbob/ZkBobSequencer.t.sol index 58de223..7137a85 100644 --- a/test/zkbob/ZkBobSequencer.t.sol +++ b/test/zkbob/ZkBobSequencer.t.sol @@ -156,7 +156,7 @@ abstract contract AbstractZkBobPoolSequencerTest is AbstractForkTest { 0 ); pool.setAccounting(accounting); - sequencer = new ZkBobSequencer(address(pool)); + sequencer = new ZkBobSequencer(address(pool), 1 hours, 10 minutes); operatorManager = new MutableOperatorManager( address(sequencer), address(sequencer), @@ -218,7 +218,7 @@ abstract contract AbstractZkBobPoolSequencerTest is AbstractForkTest { assertTrue(success); assertEq(pool.accumulatedFee(prover1), prover1FeeBefore + proxyFee); - vm.warp(block.timestamp + sequencer.PROXY_GRACE_PERIOD() + 1); + vm.warp(block.timestamp + sequencer.gracePeriod() + 1); hoax(prover2, prover2); (success, ) = address(sequencer).call(abi.encodePacked(ZkBobSequencer.prove.selector, proveData)); @@ -247,7 +247,7 @@ abstract contract AbstractZkBobPoolSequencerTest is AbstractForkTest { (success, ) = address(sequencer).call(abi.encodePacked(ZkBobSequencer.commit.selector, secondCommitData)); assertTrue(success); - vm.warp(block.timestamp + sequencer.EXPIRATION_TIME() + 1); + vm.warp(block.timestamp + sequencer.expirationTime() + 1); hoax(prover1, prover1); (success, ) = address(sequencer).call(abi.encodePacked(ZkBobSequencer.prove.selector, firstProveData));