diff --git a/contracts/AccountLocks.sol b/contracts/AccountLocks.sol deleted file mode 100644 index 5d438e6..0000000 --- a/contracts/AccountLocks.sol +++ /dev/null @@ -1,96 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.8; - -/// Implements account locking. The main goal of this design is to allow -/// unlocking of multiple accounts in O(1). To achieve this we keep a list of -/// locks per account. Every time an account is locked or unlocked, this list is -/// checked for inactive locks, which are subsequently removed. To ensure that -/// this operation does not become too expensive in gas costs, a maximum amount -/// of active locks per account is enforced. -contract AccountLocks { - type LockId is bytes32; - - uint256 public constant MAX_LOCKS_PER_ACCOUNT = 128; - - mapping(LockId => Lock) private locks; - mapping(address => Account) private accounts; - - /// Creates a lock that can be used to lock accounts. The id needs to be - /// unique and collision resistant. The expiry time is given in unix time. - function _createLock(LockId id, uint256 expiry) internal { - require(locks[id].owner == address(0), "Lock already exists"); - locks[id] = Lock(msg.sender, expiry, false); - } - - /// Attaches a lock to an account. Only when the lock is unlocked or expires - /// can the account be unlocked again. - /// Calling this function triggers a cleanup of inactive locks, making this - /// an O(N) operation, where N = MAX_LOCKS_PER_ACCOUNT. - function _lock(address account, LockId lockId) internal { - require(locks[lockId].owner != address(0), "Lock does not exist"); - LockId[] storage accountLocks = accounts[account].locks; - removeInactiveLocks(accountLocks); - require(accountLocks.length < MAX_LOCKS_PER_ACCOUNT, "Max locks reached"); - accountLocks.push(lockId); - } - - /// Unlocks a lock, thereby freeing any accounts that are attached to this - /// lock. This is an O(1) operation. Only the party that created the lock is - /// allowed to unlock it. - function _unlock(LockId lockId) internal { - Lock storage lock = locks[lockId]; - require(lock.owner != address(0), "Lock does not exist"); - require(lock.owner == msg.sender, "Only lock creator can unlock"); - lock.unlocked = true; - } - - /// Extends the locks expiry time. Lock must not have already expired. - /// NOTE: We do not need to check that msg.sender is the lock.owner because - /// this function is internal, and is only called after all checks have been - /// performed in Marketplace.fillSlot. - function _extendLockExpiryTo(LockId lockId, uint256 expiry) internal { - Lock storage lock = locks[lockId]; - require(lock.owner != address(0), "Lock does not exist"); - require(lock.expiry >= block.timestamp, "Lock already expired"); - lock.expiry = expiry; - } - - /// Unlocks an account. This will fail if there are any active locks attached - /// to this account. - /// Calling this function triggers a cleanup of inactive locks, making this - /// an O(N) operation, where N = MAX_LOCKS_PER_ACCOUNT. - function _unlockAccount() internal { - LockId[] storage accountLocks = accounts[msg.sender].locks; - removeInactiveLocks(accountLocks); - require(accountLocks.length == 0, "Account locked"); - } - - function removeInactiveLocks(LockId[] storage lockIds) private { - uint256 index = 0; - while (true) { - if (index >= lockIds.length) { - return; - } - if (isInactive(locks[lockIds[index]])) { - lockIds[index] = lockIds[lockIds.length - 1]; - lockIds.pop(); - } else { - index++; - } - } - } - - function isInactive(Lock storage lock) private view returns (bool) { - return lock.unlocked || lock.expiry <= block.timestamp; - } - - struct Lock { - address owner; - uint256 expiry; - bool unlocked; - } - - struct Account { - LockId[] locks; - } -} diff --git a/contracts/Collateral.sol b/contracts/Collateral.sol index 5799e3b..1719be2 100644 --- a/contracts/Collateral.sol +++ b/contracts/Collateral.sol @@ -2,9 +2,8 @@ pragma solidity ^0.8.0; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; -import "./AccountLocks.sol"; -abstract contract Collateral is AccountLocks { +abstract contract Collateral { IERC20 public immutable token; CollateralFunds private funds; @@ -39,11 +38,7 @@ abstract contract Collateral is AccountLocks { add(msg.sender, amount); } - // TODO: remove AccountLocks - function isWithdrawAllowed() internal virtual returns (bool) { - _unlockAccount(); - return true; - } + function isWithdrawAllowed() internal virtual returns (bool); function withdraw() public collateralInvariant { require(isWithdrawAllowed(), "Account locked"); diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index 3c97c2f..a50fa7a 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -68,8 +68,6 @@ contract Marketplace is Collateral, Proofs { requestsPerClient[request.client].add(RequestId.unwrap(id)); - _createLock(_toLockId(id), request.expiry); - uint256 amount = price(request); funds.received += amount; funds.balance += amount; @@ -91,8 +89,6 @@ contract Marketplace is Collateral, Proofs { require(slot.host == address(0), "Slot already filled"); require(balanceOf(msg.sender) >= collateral, "Insufficient collateral"); - LockId lockId = _toLockId(requestId); - _lock(msg.sender, lockId); ProofId proofId = _toProofId(slotId); _expectProofs(proofId, _toEndId(requestId), request.ask.proofProbability); @@ -109,7 +105,6 @@ contract Marketplace is Collateral, Proofs { if (context.slotsFilled == request.ask.slots) { context.state = RequestState.Started; context.startedAt = block.timestamp; - _extendLockExpiryTo(lockId, context.endsAt); emit RequestFulfilled(requestId); } } @@ -414,10 +409,6 @@ contract Marketplace is Collateral, Proofs { return SlotId.wrap(keccak256(abi.encode(requestId, slotIndex))); } - function _toLockId(RequestId requestId) internal pure returns (LockId) { - return LockId.wrap(RequestId.unwrap(requestId)); - } - function _toProofId(SlotId slotId) internal pure returns (ProofId) { return ProofId.wrap(SlotId.unwrap(slotId)); } diff --git a/contracts/TestAccountLocks.sol b/contracts/TestAccountLocks.sol deleted file mode 100644 index ef76c3c..0000000 --- a/contracts/TestAccountLocks.sol +++ /dev/null @@ -1,27 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; - -import "./AccountLocks.sol"; - -// exposes internal functions for testing -contract TestAccountLocks is AccountLocks { - function createLock(LockId id, uint256 expiry) public { - _createLock(id, expiry); - } - - function lock(address account, LockId id) public { - _lock(account, id); - } - - function unlock(LockId id) public { - _unlock(id); - } - - function unlockAccount() public { - _unlockAccount(); - } - - function extendLockExpiryTo(LockId lockId, uint256 expiry) public { - _extendLockExpiryTo(lockId, expiry); - } -} diff --git a/contracts/TestCollateral.sol b/contracts/TestCollateral.sol index 25bb366..e5c09b8 100644 --- a/contracts/TestCollateral.sol +++ b/contracts/TestCollateral.sol @@ -12,15 +12,7 @@ contract TestCollateral is Collateral { _slash(account, percentage); } - function createLock(LockId id, uint256 expiry) public { - _createLock(id, expiry); - } - - function lock(address account, LockId id) public { - _lock(account, id); - } - - function unlock(LockId id) public { - _unlock(id); + function isWithdrawAllowed() internal pure override returns (bool) { + return true; } } diff --git a/test/AccountLocks.test.js b/test/AccountLocks.test.js deleted file mode 100644 index 96f4432..0000000 --- a/test/AccountLocks.test.js +++ /dev/null @@ -1,214 +0,0 @@ -const { ethers } = require("hardhat") -const { expect } = require("chai") -const { hexlify, randomBytes, toHexString } = ethers.utils -const { - advanceTimeTo, - snapshot, - revert, - advanceTime, - currentTime, -} = require("./evm") -const { exampleLock } = require("./examples") -const { hours } = require("./time") - -describe("Account Locks", function () { - let locks - - beforeEach(async function () { - let AccountLocks = await ethers.getContractFactory("TestAccountLocks") - locks = await AccountLocks.deploy() - }) - - describe("creating a lock", function () { - it("allows creation of a lock with an expiry time", async function () { - let { id, expiry } = await exampleLock() - await locks.createLock(id, expiry) - }) - - it("fails to create a lock with an existing id", async function () { - let { id, expiry } = await exampleLock() - await locks.createLock(id, expiry) - await expect(locks.createLock(id, expiry + 1)).to.be.revertedWith( - "Lock already exists" - ) - }) - }) - - describe("locking an account", function () { - let lock - - beforeEach(async function () { - lock = await exampleLock() - await locks.createLock(lock.id, lock.expiry) - }) - - it("locks an account", async function () { - let [account] = await ethers.getSigners() - await locks.lock(account.address, lock.id) - }) - - it("fails to lock account when lock does not exist", async function () { - let [account] = await ethers.getSigners() - let nonexistent = (await exampleLock()).id - await expect(locks.lock(account.address, nonexistent)).to.be.revertedWith( - "Lock does not exist" - ) - }) - }) - - describe("unlocking a lock", function () { - let lock - - beforeEach(async function () { - lock = await exampleLock() - await locks.createLock(lock.id, lock.expiry) - }) - - it("unlocks a lock", async function () { - await locks.unlock(lock.id) - }) - - it("fails to unlock a lock that does not exist", async function () { - let nonexistent = (await exampleLock()).id - await expect(locks.unlock(nonexistent)).to.be.revertedWith( - "Lock does not exist" - ) - }) - - it("fails to unlock by someone other than the creator", async function () { - let [_, other] = await ethers.getSigners() - await expect(locks.connect(other).unlock(lock.id)).to.be.revertedWith( - "Only lock creator can unlock" - ) - }) - }) - - describe("unlocking an account", function () { - it("unlocks an account that has not been locked", async function () { - await locks.unlockAccount() - }) - - it("unlocks an account whose locks have been unlocked", async function () { - let [account] = await ethers.getSigners() - let lock = await exampleLock() - await locks.createLock(lock.id, lock.expiry) - await locks.lock(account.address, lock.id) - await locks.unlock(lock.id) - await locks.unlockAccount() - }) - - it("unlocks an account whose locks have expired", async function () { - let [account] = await ethers.getSigners() - let lock = { ...(await exampleLock()), expiry: currentTime() } - await locks.createLock(lock.id, lock.expiry) - await locks.lock(account.address, lock.id) - await locks.unlockAccount() - }) - - it("unlocks multiple accounts tied to the same lock", async function () { - let [account0, account1] = await ethers.getSigners() - let lock = await exampleLock() - await locks.createLock(lock.id, lock.expiry) - await locks.lock(account0.address, lock.id) - await locks.lock(account1.address, lock.id) - await locks.unlock(lock.id) - await locks.connect(account0).unlockAccount() - await locks.connect(account1).unlockAccount() - }) - - it("fails to unlock when some locks are still locked", async function () { - let [account] = await ethers.getSigners() - let [lock1, lock2] = [await exampleLock(), await exampleLock()] - await locks.createLock(lock1.id, lock1.expiry) - await locks.createLock(lock2.id, lock2.expiry) - await locks.lock(account.address, lock1.id) - await locks.lock(account.address, lock2.id) - await locks.unlock(lock1.id) - await expect(locks.unlockAccount()).to.be.revertedWith("Account locked") - }) - }) - - describe("limits", function () { - let maxlocks - let account - - beforeEach(async function () { - maxlocks = await locks.MAX_LOCKS_PER_ACCOUNT() - ;[account] = await ethers.getSigners() - }) - - async function addLock() { - let { id, expiry } = await exampleLock() - await locks.createLock(id, expiry) - await locks.lock(account.address, id) - return id - } - - it("supports a limited amount of locks per account", async function () { - for (let i = 0; i < maxlocks; i++) { - await addLock() - } - await expect(addLock()).to.be.revertedWith("Max locks reached") - }) - - it("doesn't count unlocked locks towards the limit", async function () { - for (let i = 0; i < maxlocks; i++) { - let id = await addLock() - await locks.unlock(id) - } - await expect(addLock()).not.to.be.reverted - }) - - it("handles maximum amount of locks within gas limit", async function () { - let ids = [] - for (let i = 0; i < maxlocks; i++) { - ids.push(await addLock()) - } - for (let id of ids) { - await locks.unlock(id) - } - await locks.unlockAccount() - }) - }) - - describe("extend lock expiry", function () { - let expiry - let newExpiry - let id - - beforeEach(async function () { - await snapshot() - - let lock = await exampleLock() - id = lock.id - expiry = lock.expiry - await locks.createLock(id, expiry) - newExpiry = (await currentTime()) + hours(1) - - let [account] = await ethers.getSigners() - await locks.lock(account.address, id) - }) - - afterEach(async function () { - await revert() - }) - - it("fails when lock id doesn't exist", async function () { - let other = await exampleLock() - await expect( - locks.extendLockExpiryTo(other.id, newExpiry) - ).to.be.revertedWith("Lock does not exist") - }) - - it("fails when lock is already expired", async function () { - await advanceTimeTo(expiry) - await expect(locks.extendLockExpiryTo(id, newExpiry)).to.be.revertedWith( - "Lock already expired" - ) - }) - - it("successfully updates lock expiry", async function () { - await expect(locks.extendLockExpiryTo(id, newExpiry)).not.to.be.reverted - }) - }) -})