diff --git a/docs/architecture-summary.md b/docs/architecture-summary.md index 5dc1b96..519a63f 100644 --- a/docs/architecture-summary.md +++ b/docs/architecture-summary.md @@ -126,7 +126,7 @@ not violate them. settle in flight. - `CelerWallet` has exactly one **operator** (a `CelerLedger` instance). Operatorship transfer is the migration pivot; only the current operator (or all owners - cooperatively, via `proposeNewOperator`) can transfer it. + cooperatively, via `voteForOperator`) can transfer it. --- diff --git a/docs/contracts.md b/docs/contracts.md index 8cfcf13..a69e926 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -44,7 +44,7 @@ can `pause` / `unpause` and (when paused) `drainToken` to recover stuck funds. ### Roles -- **Owners** — the channel peers; receive withdrawals and vote on operator proposals. +- **Owners** — the channel peers; receive withdrawals and vote on operator candidates. - **Operator** — exactly one per wallet; the `CelerLedger` instance authorized to move funds. Operatorship is the migration pivot point. - **Contract owner** (Ownable) — can pause / unpause and drain when paused. @@ -53,21 +53,21 @@ can `pause` / `unpause` and (when paused) `drainToken` to recover stuck funds. | Function | Caller | Purpose | |---|---|---| -| [`create`](../src/CelerWallet.sol#L66) | anyone (typically a `CelerLedger`) | Create a new wallet for a peer-pair, returning its `walletId`. | -| [`depositNative`](../src/CelerWallet.sol#L89) | anyone (payable) | Deposit native (e.g., ETH) into a wallet. | -| [`depositERC20`](../src/CelerWallet.sol#L101) | anyone | Deposit ERC-20 tokens (requires prior `approve`). | -| [`withdraw`](../src/CelerWallet.sol#L119) | operator only | Withdraw funds to a receiver. | -| [`transferToWallet`](../src/CelerWallet.sol#L141) | operator only | Move funds between two wallets sharing the same operator (channel rebalancing). | -| [`transferOperatorship`](../src/CelerWallet.sol#L164) | current operator | Transfer operatorship to a new operator (the migration path). | -| [`proposeNewOperator`](../src/CelerWallet.sol#L179) | wallet owner | Propose a new operator; takes effect when *all* owners propose the same address (manual fallback for stuck migrations). | -| [`drainToken`](../src/CelerWallet.sol#L207) | contract owner, when paused | Emergency token recovery. | +| [`create`](../src/CelerWallet.sol) | anyone (typically a `CelerLedger`) | Create a new wallet for a peer-pair, returning its `walletId`. | +| [`depositNative`](../src/CelerWallet.sol) | anyone (payable) | Deposit native (e.g., ETH) into a wallet. | +| [`depositERC20`](../src/CelerWallet.sol) | anyone | Deposit ERC-20 tokens (requires prior `approve`). | +| [`withdraw`](../src/CelerWallet.sol) | operator only | Withdraw funds to a receiver. | +| [`transferBetweenWallets`](../src/CelerWallet.sol) | operator only | Move funds between two wallets sharing the same operator (channel rebalancing). | +| [`transferOperatorship`](../src/CelerWallet.sol) | current operator | Transfer operatorship to a new operator (the migration path). | +| [`voteForOperator`](../src/CelerWallet.sol) | wallet owner | Vote for a new operator candidate; the change takes effect when *all* owners have voted for the same candidate (manual fallback for stuck migrations). | +| [`drainToken`](../src/CelerWallet.sol) | contract owner, when paused | Emergency token recovery. | | `pause` / `unpause` | contract owner | Pause guard for deposits / withdrawals / operator changes. | -| `getWalletOwners` / `getOperator` / `getBalance` / `getProposedNewOperator` / `getProposalVote` | view | Wallet introspection. | +| `walletOwners` / `walletOperator` / `balanceOf` / `pendingOperator` / `hasVoted` | view | Wallet introspection. | ### Events -`CreateWallet`, `DepositToWallet`, `WithdrawFromWallet`, `TransferToWallet`, -`ChangeOperator`, `ProposeNewOperator`, `DrainToken`. See +`WalletCreated`, `Deposited`, `Withdrawn`, `TransferredBetweenWallets`, +`OperatorChanged`, `OperatorVoted`, `TokenDrained`. See [`ICelerWallet.sol`](../src/interfaces/ICelerWallet.sol). ### Storage @@ -76,11 +76,11 @@ can `pause` / `unpause` and (when paused) `drainToken` to recover stuck funds. struct Wallet { address[] owners; address operator; - mapping(address => uint256) balances; // tokenAddr (0 = ETH) → balance - address proposedNewOperator; - mapping(address => bool) proposalVotes; + mapping(address => uint256) balances; // tokenAddr (0 = native) → balance + address pendingOperator; + mapping(address => bool) votes; } -uint256 public walletNum; +uint256 public walletCount; mapping(bytes32 => Wallet) private wallets; ``` @@ -131,35 +131,35 @@ Balance limits are **enabled by default** post-deployment. Configure them via | Function | Purpose | |---|---| -| [`openChannel`](../src/CelerLedger.sol#L66) | Open a fully-funded channel from a co-signed `PaymentChannelInitializer` (single tx). | -| [`deposit`](../src/CelerLedger.sol#L78) | Deposit native (msg.value) and/or pull from pre-approved wrapped-native or ERC-20 into a channel. | -| [`depositInBatch`](../src/CelerLedger.sol#L91) | Batch deposit across multiple channels in one tx. Not payable — native entries fund only via pre-approved wrapped-native (no `msg.value` path). | -| [`snapshotStates`](../src/CelerLedger.sol#L114) | Persist a co-signed simplex state on-chain (lightweight checkpoint). | +| [`openChannel`](../src/CelerLedger.sol) | Open a fully-funded channel from a co-signed `PaymentChannelInitializer` (single tx). | +| [`deposit`](../src/CelerLedger.sol) | Deposit native (msg.value) and/or pull from pre-approved wrapped-native or ERC-20 into a channel. | +| [`depositInBatch`](../src/CelerLedger.sol) | Batch deposit across multiple channels in one tx. Not payable — native entries fund only via pre-approved wrapped-native (no `msg.value` path). | +| [`snapshotStates`](../src/CelerLedger.sol) | Persist a co-signed simplex state on-chain (lightweight checkpoint). | ### External functions — withdrawals | Function | Purpose | |---|---| -| [`cooperativeWithdraw`](../src/CelerLedger.sol#L153) | Single-tx withdrawal with a co-signed `CooperativeWithdrawInfo`. | -| [`intendWithdraw`](../src/CelerLedger.sol#L126) | Start a unilateral withdrawal challenge window. | -| [`vetoWithdraw`](../src/CelerLedger.sol#L145) | Counterparty cancels an in-flight unilateral withdrawal. | -| [`confirmWithdraw`](../src/CelerLedger.sol#L135) | Finalize a unilateral withdrawal after the window closes. | +| [`cooperativeWithdraw`](../src/CelerLedger.sol) | Single-tx withdrawal with a co-signed `CooperativeWithdrawInfo`. | +| [`intendWithdraw`](../src/CelerLedger.sol) | Start a unilateral withdrawal challenge window. | +| [`vetoWithdraw`](../src/CelerLedger.sol) | Counterparty cancels an in-flight unilateral withdrawal. | +| [`confirmWithdraw`](../src/CelerLedger.sol) | Finalize a unilateral withdrawal after the window closes. | ### External functions — settlement | Function | Purpose | |---|---| -| [`cooperativeSettle`](../src/CelerLedger.sol#L192) | Single-tx close with a co-signed `CooperativeSettleInfo`. | -| [`intendSettle`](../src/CelerLedger.sol#L165) | Start unilateral settlement using the latest co-signed simplex states. | -| [`clearPays`](../src/CelerLedger.sol#L175) | Settle additional pending pays via a `PayIdList` after `intendSettle`. | -| [`confirmSettle`](../src/CelerLedger.sol#L184) | Finalize unilateral settlement after the challenge window. | +| [`cooperativeSettle`](../src/CelerLedger.sol) | Single-tx close with a co-signed `CooperativeSettleInfo`. | +| [`intendSettle`](../src/CelerLedger.sol) | Start unilateral settlement using the latest co-signed simplex states. | +| [`clearPays`](../src/CelerLedger.sol) | Settle additional pending pays via a `PayIdList` after `intendSettle`. | +| [`confirmSettle`](../src/CelerLedger.sol) | Finalize unilateral settlement after the challenge window. | ### External functions — migration (decentralized versioning) | Function | Purpose | |---|---| -| [`migrateChannelFrom`](../src/CelerLedger.sol#L210) | Called on the **new** ledger; orchestrates the migration end-to-end. | -| [`migrateChannelTo`](../src/CelerLedger.sol#L201) | Called on the **old** ledger by the new one; transfers operatorship and exposes state. | +| [`migrateChannelFrom`](../src/CelerLedger.sol) | Called on the **new** ledger; orchestrates the migration end-to-end. | +| [`migrateChannelTo`](../src/CelerLedger.sol) | Called on the **old** ledger by the new one; transfers operatorship and exposes state. | ### External functions — admin @@ -221,8 +221,8 @@ constructor(address _registryAddr, address _virtResolverAddr) | Function | Purpose | |---|---| -| [`resolvePaymentByConditions`](../src/PayResolver.sol#L44) | Evaluate every condition (hash-locks via preimages, deployed/virtual contracts via `isFinalized`+`getOutcome`), apply the `transfer_func`, write to `PayRegistry`. Reverts if any condition is not finalized. | -| [`resolvePaymentByVouchedResult`](../src/PayResolver.sol#L71) | Bypass condition evaluation by accepting a result co-signed by `pay.src` and `pay.dest`; capped at `pay.transferFunc.maxTransfer.receiver.amt`. | +| [`resolvePaymentByConditions`](../src/PayResolver.sol) | Evaluate every condition (hash-locks via preimages, deployed/virtual contracts via `isFinalized`+`getOutcome`), apply the `transfer_func`, write to `PayRegistry`. Reverts if any condition is not finalized. | +| [`resolvePaymentByVouchedResult`](../src/PayResolver.sol) | Bypass condition evaluation by accepting a result co-signed by `pay.src` and `pay.dest`; capped at `pay.transferFunc.maxTransfer.receiver.amt`. | ### Resolution rules @@ -263,13 +263,13 @@ No constructor (no state to initialize). | Function | Purpose | |---|---| -| [`calculatePayId`](../src/PayRegistry.sol#L25) | Pure helper: `keccak256(payHash, setter)`. | -| [`setPayAmount`](../src/PayRegistry.sol#L29) | Setter writes the resolved amount under its own namespace. | -| [`setPayDeadline`](../src/PayRegistry.sol#L37) | Setter writes the resolve deadline under its own namespace. | -| [`setPayInfo`](../src/PayRegistry.sol#L45) | Combined `setPayAmount` + `setPayDeadline`. | -| [`setPayAmounts`](../src/PayRegistry.sol#L54) / [`setPayDeadlines`](../src/PayRegistry.sol#L68) / [`setPayInfos`](../src/PayRegistry.sol#L82) | Batched variants. | -| [`getPayAmounts`](../src/PayRegistry.sol#L107) | Bulk read for settlement; gates each pay by its own `resolveDeadline` if resolved, or the per-channel `pay_clear_deadline` (`max(pay.resolveDeadline) + clearMargin`) if never resolved. | -| [`getPayInfo`](../src/PayRegistry.sol#L126) | Single-pay read. | +| [`calculatePayId`](../src/PayRegistry.sol) | Pure helper: `keccak256(payHash, setter)`. | +| [`setPayAmount`](../src/PayRegistry.sol) | Setter writes the resolved amount under its own namespace. | +| [`setPayDeadline`](../src/PayRegistry.sol) | Setter writes the resolve deadline under its own namespace. | +| [`setPayInfo`](../src/PayRegistry.sol) | Combined `setPayAmount` + `setPayDeadline`. | +| [`setPayAmounts`](../src/PayRegistry.sol) / [`setPayDeadlines`](../src/PayRegistry.sol) / [`setPayInfos`](../src/PayRegistry.sol) | Batched variants. | +| [`getPayAmounts`](../src/PayRegistry.sol) | Bulk read for settlement; gates each pay by its own `resolveDeadline` if resolved, or the per-channel `pay_clear_deadline` (`max(pay.resolveDeadline) + clearMargin`) if never resolved. | +| [`getPayInfo`](../src/PayRegistry.sol) | Single-pay read. | | `payInfoMap` (auto-getter) | Public mapping accessor. | ### Events @@ -297,8 +297,8 @@ virtual address (used in `Condition.virtual_contract_address`) is | Function | Purpose | |---|---| -| [`deploy`](../src/VirtContractResolver.sol#L20) | Deploy bytecode under the (code, nonce) virtual address; reverts if already deployed. | -| [`resolve`](../src/VirtContractResolver.sol#L40) | Look up the deployed address for a virtual address. | +| [`deploy`](../src/VirtContractResolver.sol) | Deploy bytecode under the (code, nonce) virtual address; reverts if already deployed. | +| [`resolve`](../src/VirtContractResolver.sol) | Look up the deployed address for a virtual address. | ### Events @@ -318,9 +318,9 @@ refresh. | Function | Purpose | |---|---| -| [`registerRouter`](../src/RouterRegistry.sol#L18) | Add `msg.sender` to the registry. Reverts if already present. | -| [`deregisterRouter`](../src/RouterRegistry.sol#L29) | Remove `msg.sender`. | -| [`refreshRouter`](../src/RouterRegistry.sol#L40) | Update the stored timestamp for `msg.sender`. | +| [`registerRouter`](../src/RouterRegistry.sol) | Add `msg.sender` to the registry. Reverts if already present. | +| [`deregisterRouter`](../src/RouterRegistry.sol) | Remove `msg.sender`. | +| [`refreshRouter`](../src/RouterRegistry.sol) | Update the stored timestamp for `msg.sender`. | ### Events diff --git a/src/CelerWallet.sol b/src/CelerWallet.sol index 372804a..607515c 100644 --- a/src/CelerWallet.sol +++ b/src/CelerWallet.sol @@ -20,10 +20,8 @@ import "@openzeppelin/contracts/access/Ownable.sol"; contract CelerWallet is ICelerWallet, Pausable, Ownable { using SafeERC20 for IERC20; - enum MathOperation { - Add, - Sub - } + /// @notice Hard cap on `owners.length` per wallet. + uint256 public constant MAX_OWNERS = 10; struct Wallet { // corresponding to peers in CelerLedger @@ -32,49 +30,53 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable { address operator; // address(0) for native mapping(address => uint256) balances; - address proposedNewOperator; - mapping(address => bool) proposalVotes; + // operator candidate currently being voted on; address(0) when no vote in flight + address pendingOperator; + // owner => has-voted for the current pendingOperator + mapping(address => bool) votes; } - uint256 public walletNum; + uint256 public walletCount; mapping(bytes32 => Wallet) private wallets; constructor() Ownable(msg.sender) {} - /** - * @dev Throws if called by any account other than the wallet's operator - * @param _walletId id of the wallet to be operated - */ + // ------------------------------------------------------------------------- + // Modifiers and internal checks + // ------------------------------------------------------------------------- + + /// @dev Throws if called by any account other than the wallet's operator. modifier onlyOperator(bytes32 _walletId) { - require(msg.sender == wallets[_walletId].operator, AgentPayErrors.NotOperator()); + _checkOperator(_walletId); _; } - /** - * @dev Throws if given address is not an owner of the wallet - * @param _walletId id of the wallet to be operated - * @param _addr address to be checked - */ + /// @dev Throws if `_addr` is not an owner of the wallet. modifier onlyWalletOwner(bytes32 _walletId, address _addr) { - require(_isWalletOwner(_walletId, _addr), AgentPayErrors.NotWalletOwner()); + _checkWalletOwner(_walletId, _addr); _; } - /** - * @notice Create a new wallet - * @dev `walletId = keccak256(chainid, walletAddr, creatorAddr, nonce)` where - * `creatorAddr` is `msg.sender` (the account that calls `create`, e.g. CelerLedger). - * @param _owners owners of the wallet - * @param _operator initial operator of the wallet - * @param _nonce nonce given by caller to generate the wallet id - * @return id of created wallet - */ - function create(address[] memory _owners, address _operator, bytes32 _nonce) - public + function _checkOperator(bytes32 _walletId) internal view { + require(msg.sender == wallets[_walletId].operator, AgentPayErrors.NotOperator()); + } + + function _checkWalletOwner(bytes32 _walletId, address _addr) internal view { + require(_isWalletOwner(_walletId, _addr), AgentPayErrors.NotWalletOwner()); + } + + // ------------------------------------------------------------------------- + // External / state-changing API + // ------------------------------------------------------------------------- + + /// @inheritdoc ICelerWallet + function create(address[] calldata _owners, address _operator, bytes32 _nonce) + external whenNotPaused returns (bytes32) { require(_operator != address(0), AgentPayErrors.ZeroAddress()); + require(_owners.length <= MAX_OWNERS, AgentPayErrors.TooManyOwners()); bytes32 walletId = keccak256(abi.encodePacked(block.chainid, address(this), msg.sender, _nonce)); Wallet storage w = wallets[walletId]; @@ -82,195 +84,150 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable { require(w.operator == address(0), AgentPayErrors.WalletIdOccupied()); w.owners = _owners; w.operator = _operator; - walletNum++; + walletCount++; - emit CreateWallet(walletId, _owners, _operator); + emit WalletCreated(walletId, _owners, _operator); return walletId; } - /** - * @notice Deposit native (e.g. ETH) to a wallet - * @param _walletId id of the wallet to deposit into - */ - function depositNative(bytes32 _walletId) public payable whenNotPaused { + /// @inheritdoc ICelerWallet + function depositNative(bytes32 _walletId) external payable whenNotPaused { uint256 amount = msg.value; - _updateBalance(_walletId, address(0), amount, MathOperation.Add); - emit DepositToWallet(_walletId, address(0), amount); + wallets[_walletId].balances[address(0)] += amount; + emit Deposited(_walletId, address(0), amount); } - /** - * @notice Deposit ERC20 tokens to a wallet - * @param _walletId id of the wallet to deposit into - * @param _tokenAddress address of token to deposit - * @param _amount deposit token amount - */ - function depositERC20(bytes32 _walletId, address _tokenAddress, uint256 _amount) public whenNotPaused { - _updateBalance(_walletId, _tokenAddress, _amount, MathOperation.Add); - emit DepositToWallet(_walletId, _tokenAddress, _amount); + /// @inheritdoc ICelerWallet + function depositERC20(bytes32 _walletId, address _tokenAddress, uint256 _amount) external whenNotPaused { + wallets[_walletId].balances[_tokenAddress] += _amount; + emit Deposited(_walletId, _tokenAddress, _amount); IERC20(_tokenAddress).safeTransferFrom(msg.sender, address(this), _amount); } /** - * @notice Withdraw funds to an address - * @dev Since this withdraw() function uses direct transfer to send native, if CelerLedger - * allows non externally-owned account (EOA) to be a peer of the channel namely an owner - * of the wallet, CelerLedger should implement a withdraw pattern for native to avoid - * maliciously fund locking. Withdraw pattern reference: - * @param _walletId id of the wallet to withdraw from - * @param _tokenAddress address of token to withdraw - * @param _receiver token receiver - * @param _amount withdrawal token amount + * @inheritdoc ICelerWallet + * @dev CEI: balance debit precedes the external transfer. Combined with + * Solidity 0.8's checked arithmetic, this bounds any reentrant + * withdraw chain at the wallet's actual balance — equivalent to N + * sequential withdraw calls. */ function withdraw(bytes32 _walletId, address _tokenAddress, address _receiver, uint256 _amount) - public + external whenNotPaused onlyOperator(_walletId) onlyWalletOwner(_walletId, _receiver) { - _updateBalance(_walletId, _tokenAddress, _amount, MathOperation.Sub); - emit WithdrawFromWallet(_walletId, _tokenAddress, _receiver, _amount); + // Solidity 0.8 checked subtraction reverts on underflow if the wallet + // doesn't hold enough of `_tokenAddress` — implicit balance gate. + wallets[_walletId].balances[_tokenAddress] -= _amount; + emit Withdrawn(_walletId, _tokenAddress, _receiver, _amount); _withdrawToken(_tokenAddress, _receiver, _amount); } - /** - * @notice Transfer funds from one wallet to another wallet with a same owner (as the receiver) - * @dev from wallet and to wallet must have one common owner as the receiver or beneficiary - * of this transfer - * @param _fromWalletId id of wallet to transfer funds from - * @param _toWalletId id of wallet to transfer funds to - * @param _tokenAddress address of token to transfer - * @param _receiver beneficiary who transfers her funds from one wallet to another wallet - * @param _amount transferred token amount - */ - function transferToWallet( + /// @inheritdoc ICelerWallet + function transferBetweenWallets( bytes32 _fromWalletId, bytes32 _toWalletId, address _tokenAddress, address _receiver, uint256 _amount ) - public + external whenNotPaused onlyOperator(_fromWalletId) onlyWalletOwner(_fromWalletId, _receiver) onlyWalletOwner(_toWalletId, _receiver) { - _updateBalance(_fromWalletId, _tokenAddress, _amount, MathOperation.Sub); - _updateBalance(_toWalletId, _tokenAddress, _amount, MathOperation.Add); - emit TransferToWallet(_fromWalletId, _toWalletId, _tokenAddress, _receiver, _amount); + wallets[_fromWalletId].balances[_tokenAddress] -= _amount; + wallets[_toWalletId].balances[_tokenAddress] += _amount; + emit TransferredBetweenWallets(_fromWalletId, _toWalletId, _tokenAddress, _receiver, _amount); } - /** - * @notice Current operator transfers the operatorship of a wallet to the new operator - * @param _walletId id of wallet to transfer the operatorship - * @param _newOperator the new operator - */ + /// @inheritdoc ICelerWallet function transferOperatorship(bytes32 _walletId, address _newOperator) - public + external whenNotPaused onlyOperator(_walletId) { _changeOperator(_walletId, _newOperator); } - /** - * @notice Wallet owners propose and assign a new operator of their wallet - * @dev it will assign a new operator if all owners propose the same new operator. - * This does not require unpaused. - * @param _walletId id of wallet which owners propose new operator of - * @param _newOperator the new operator proposal - */ - function proposeNewOperator(bytes32 _walletId, address _newOperator) public onlyWalletOwner(_walletId, msg.sender) { - require(_newOperator != address(0), AgentPayErrors.ZeroAddress()); + /// @inheritdoc ICelerWallet + function voteForOperator(bytes32 _walletId, address _candidate) external onlyWalletOwner(_walletId, msg.sender) { + require(_candidate != address(0), AgentPayErrors.ZeroAddress()); Wallet storage w = wallets[_walletId]; - if (_newOperator != w.proposedNewOperator) { + // Voting for a different candidate replaces the in-flight proposal + // and resets every owner's tally — consensus is per-candidate. + if (_candidate != w.pendingOperator) { _clearVotes(w); - w.proposedNewOperator = _newOperator; + w.pendingOperator = _candidate; } - w.proposalVotes[msg.sender] = true; - emit ProposeNewOperator(_walletId, _newOperator, msg.sender); + w.votes[msg.sender] = true; + emit OperatorVoted(_walletId, _candidate, msg.sender); + // _changeOperator clears the pendingOperator + vote tally on success. if (_checkAllVotes(w)) { - _changeOperator(_walletId, _newOperator); - _clearVotes(w); + _changeOperator(_walletId, _candidate); } } - /** - * @notice Pauser drains one type of tokens when paused - * @notice This is only for emergent situations. - * @param _tokenAddress address of token to drain - * @param _receiver token receiver - * @param _amount drained token amount - */ - function drainToken(address _tokenAddress, address _receiver, uint256 _amount) public whenPaused onlyOwner { - emit DrainToken(_tokenAddress, _receiver, _amount); + /// @inheritdoc ICelerWallet + function drainToken(address _tokenAddress, address _receiver, uint256 _amount) external whenPaused onlyOwner { + emit TokenDrained(_tokenAddress, _receiver, _amount); _withdrawToken(_tokenAddress, _receiver, _amount); } - /** - * @notice Get owners of a given wallet - * @param _walletId id of the queried wallet - * @return wallet's owners - */ - function getWalletOwners(bytes32 _walletId) external view returns (address[] memory) { + /// @notice Pause the wallet. Owner-only. Blocks deposits / withdrawals / operator changes. + function pause() external onlyOwner { + _pause(); + } + + /// @notice Resume normal operation after a pause. Owner-only. + function unpause() external onlyOwner { + _unpause(); + } + + // ------------------------------------------------------------------------- + // External views + // ------------------------------------------------------------------------- + + /// @inheritdoc ICelerWallet + function walletOwners(bytes32 _walletId) external view returns (address[] memory) { return wallets[_walletId].owners; } - /** - * @notice Get operator of a given wallet - * @param _walletId id of the queried wallet - * @return wallet's operator - */ - function getOperator(bytes32 _walletId) public view returns (address) { + /// @inheritdoc ICelerWallet + function walletOperator(bytes32 _walletId) external view returns (address) { return wallets[_walletId].operator; } - /** - * @notice Get balance of a given token in a given wallet - * @param _walletId id of the queried wallet - * @param _tokenAddress address of the queried token - * @return amount of the given token in the wallet - */ - function getBalance(bytes32 _walletId, address _tokenAddress) public view returns (uint256) { + /// @inheritdoc ICelerWallet + function balanceOf(bytes32 _walletId, address _tokenAddress) external view returns (uint256) { return wallets[_walletId].balances[_tokenAddress]; } - /** - * @notice Get proposedNewOperator of a given wallet - * @param _walletId id of the queried wallet - * @return wallet's proposedNewOperator - */ - function getProposedNewOperator(bytes32 _walletId) external view returns (address) { - return wallets[_walletId].proposedNewOperator; + /// @inheritdoc ICelerWallet + function pendingOperator(bytes32 _walletId) external view returns (address) { + return wallets[_walletId].pendingOperator; } - /** - * @notice Get the vote of an owner for the proposedNewOperator of a wallet - * @param _walletId id of the queried wallet - * @param _owner owner to be checked - * @return the owner's vote for the proposedNewOperator - */ - function getProposalVote(bytes32 _walletId, address _owner) - external - view - onlyWalletOwner(_walletId, _owner) - returns (bool) - { - return wallets[_walletId].proposalVotes[_owner]; + /// @inheritdoc ICelerWallet + function hasVoted(bytes32 _walletId, address _owner) external view returns (bool) { + return wallets[_walletId].votes[_owner]; } - /** - * @notice Internal function to withdraw out one type of token - * @param _tokenAddress address of token to withdraw - * @param _receiver token receiver - * @param _amount withdrawal token amount - */ + // ------------------------------------------------------------------------- + // Internals + // ------------------------------------------------------------------------- + + /// @notice Send `_amount` of `_tokenAddress` to `_receiver`. Native uses + /// raw `.call{value:}` (see {withdraw} NatSpec for reentrancy notes); + /// ERC-20 routes through OpenZeppelin `SafeERC20`. function _withdrawToken(address _tokenAddress, address _receiver, uint256 _amount) internal { if (_tokenAddress == address(0)) { (bool success,) = payable(_receiver).call{value: _amount}(""); @@ -280,85 +237,61 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable { } } - /** - * @notice Update balance record - * @param _walletId id of wallet to update - * @param _tokenAddress address of token to update - * @param _amount update amount - * @param _op update operation - */ - function _updateBalance(bytes32 _walletId, address _tokenAddress, uint256 _amount, MathOperation _op) internal { - Wallet storage w = wallets[_walletId]; - if (_op == MathOperation.Add) { - w.balances[_tokenAddress] = w.balances[_tokenAddress] + _amount; - } else if (_op == MathOperation.Sub) { - w.balances[_tokenAddress] = w.balances[_tokenAddress] - _amount; - } else { - assert(false); - } - } - - /** - * @notice Clear all votes of new operator proposals of the wallet - * @param _w the wallet - */ + /// @notice Clear all owners' votes on the current pendingOperator. function _clearVotes(Wallet storage _w) internal { - for (uint256 i = 0; i < _w.owners.length; i++) { - _w.proposalVotes[_w.owners[i]] = false; + uint256 n = _w.owners.length; + for (uint256 i = 0; i < n;) { + _w.votes[_w.owners[i]] = false; + unchecked { + ++i; + } } } - /** - * @notice Internal function of changing the operator of a wallet - * @param _walletId id of wallet to change its operator - * @param _newOperator the new operator - */ + /// @notice Internal operator change. Also clears any in-flight pending + /// candidate — the just-completed change supersedes any pending vote, and + /// leaving stale state confuses subsequent {voteForOperator} calls. function _changeOperator(bytes32 _walletId, address _newOperator) internal { require(_newOperator != address(0), AgentPayErrors.ZeroAddress()); Wallet storage w = wallets[_walletId]; address oldOperator = w.operator; w.operator = _newOperator; - emit ChangeOperator(_walletId, oldOperator, _newOperator); + + if (w.pendingOperator != address(0)) { + _clearVotes(w); + delete w.pendingOperator; + } + + emit OperatorChanged(_walletId, oldOperator, _newOperator); } - /** - * @notice Check if all owners have voted for the same new operator - * @param _w the wallet - * @return true if all owners have voted for a same operator; otherwise false - */ + /// @notice True iff every owner has voted for the current pendingOperator. function _checkAllVotes(Wallet storage _w) internal view returns (bool) { - for (uint256 i = 0; i < _w.owners.length; i++) { - if (_w.proposalVotes[_w.owners[i]] == false) { + uint256 n = _w.owners.length; + for (uint256 i = 0; i < n;) { + if (_w.votes[_w.owners[i]] == false) { return false; } + unchecked { + ++i; + } } return true; } - /** - * @notice Check if an address is an owner of a wallet - * @param _walletId id of wallet to check - * @param _addr address to check - * @return true if this address is an owner of the wallet; otherwise false - */ + /// @notice True iff `_addr` appears in the wallet's `owners` array. function _isWalletOwner(bytes32 _walletId, address _addr) internal view returns (bool) { Wallet storage w = wallets[_walletId]; - for (uint256 i = 0; i < w.owners.length; i++) { + uint256 n = w.owners.length; + for (uint256 i = 0; i < n;) { if (_addr == w.owners[i]) { return true; } + unchecked { + ++i; + } } return false; } - - /// @notice Pause the wallet. Owner-only. Blocks deposits / withdrawals / operator changes. - function pause() external onlyOwner { - _pause(); - } - - /// @notice Resume normal operation after a pause. Owner-only. - function unpause() external onlyOwner { - _unpause(); - } } diff --git a/src/interfaces/ICelerWallet.sol b/src/interfaces/ICelerWallet.sol index ec5a75c..a79e6e3 100644 --- a/src/interfaces/ICelerWallet.sol +++ b/src/interfaces/ICelerWallet.sol @@ -8,13 +8,14 @@ pragma solidity ^0.8.26; * channel peers, recipients of withdrawals) and a single *operator* (typically a * CelerLedger contract version) authorized to move funds. Operatorship is the pivot * point for cooperative migration to a new ledger version — see - * {transferOperatorship} and {proposeNewOperator}. + * {transferOperatorship} and {voteForOperator}. */ interface ICelerWallet { /** * @notice Create a new wallet. * @dev `walletId = keccak256(chainid, walletContract, msg.sender, _nonce)`. - * Reverts if the derived id is already in use or if `_operator == address(0)`. + * Reverts if the derived id is already in use, if `_operator == address(0)`, + * or if `_owners.length` exceeds the contract's `MAX_OWNERS` cap. * @param _owners Owners of the wallet (typically the two channel peers). * @param _operator Initial operator authorized to move funds. * @param _nonce Caller-supplied nonce, used in the wallet-id derivation. @@ -61,7 +62,7 @@ interface ICelerWallet { * @param _receiver Beneficiary owner present in both wallets. * @param _amount Amount to transfer. */ - function transferToWallet( + function transferBetweenWallets( bytes32 _fromWalletId, bytes32 _toWalletId, address _tokenAddress, @@ -72,22 +73,27 @@ interface ICelerWallet { /** * @notice Operator transfers operatorship of a wallet to a new operator. * @dev Migration pivot point: the new operator is typically a newer CelerLedger - * version cooperatively chosen by the peers. + * version cooperatively chosen by the peers. Also clears any in-flight + * {voteForOperator} candidate + tally — the direct transfer supersedes any + * pending vote. * @param _walletId Wallet whose operatorship moves. * @param _newOperator New operator address. */ function transferOperatorship(bytes32 _walletId, address _newOperator) external; /** - * @notice Manual fallback: wallet owners cooperatively assign a new operator. - * @dev Operatorship changes only when *all* owners have proposed the same address. - * Proposing a different address resets the vote tally. This path bypasses the - * current operator and is intended for use only when the operator contract is - * broken or compromised. - * @param _walletId Wallet whose operatorship is being proposed for change. - * @param _newOperator Proposed new operator. + * @notice Cast (or re-cast) `msg.sender`'s vote for `_candidate` to become the + * next operator of `_walletId`. Operatorship changes only when *every* owner + * has voted for the same candidate. + * @dev `msg.sender`'s vote is recorded in the same call — the proposer does + * not need to call again. Voting for a candidate different from the in-flight + * one resets every owner's tally (consensus is per-candidate). This path + * bypasses the current operator and is intended for use only when the + * operator contract is broken or compromised. + * @param _walletId Wallet whose operatorship is being voted on. + * @param _candidate Candidate operator the voter is endorsing. */ - function proposeNewOperator(bytes32 _walletId, address _newOperator) external; + function voteForOperator(bytes32 _walletId, address _candidate) external; /** * @notice Emergency token recovery (callable only when the contract is paused). @@ -98,33 +104,38 @@ interface ICelerWallet { function drainToken(address _tokenAddress, address _receiver, uint256 _amount) external; /// @notice Owners of `_walletId`. - function getWalletOwners(bytes32 _walletId) external view returns (address[] memory); + function walletOwners(bytes32 _walletId) external view returns (address[] memory); - /// @notice Operator of `_walletId`. - function getOperator(bytes32 _walletId) external view returns (address); + /// @notice Current operator of `_walletId`. Distinct from this contract's + /// Ownable owner — the wallet operator is per-wallet (typically a CelerLedger). + function walletOperator(bytes32 _walletId) external view returns (address); /// @notice Token balance of `_walletId` for `_tokenAddress` (`address(0)` for native). - function getBalance(bytes32 _walletId, address _tokenAddress) external view returns (uint256); + function balanceOf(bytes32 _walletId, address _tokenAddress) external view returns (uint256); - /// @notice Currently proposed new operator for `_walletId`, if any. - function getProposedNewOperator(bytes32 _walletId) external view returns (address); + /// @notice Operator candidate currently being voted on for `_walletId`, + /// or `address(0)` if no vote is in flight. + function pendingOperator(bytes32 _walletId) external view returns (address); - /// @notice Whether `_owner` has voted for the current proposed new operator. - function getProposalVote(bytes32 _walletId, address _owner) external view returns (bool); + /// @notice True iff `_owner` has voted for the current `pendingOperator(_walletId)`. + function hasVoted(bytes32 _walletId, address _owner) external view returns (bool); - /// @notice Emitted on wallet creation. - event CreateWallet(bytes32 indexed walletId, address[] indexed owners, address indexed operator); + /// @notice Emitted on wallet creation. `owners` is indexed — subscribers + /// receive a `keccak256(abi.encode(owners))` topic rather than the array + /// itself, so per-owner filtering via topic is not directly supported. + /// For AgentPay's channel use case, the parallel `CelerLedger.OpenChannel` + /// event emits the two peer addresses unindexed in its data field, which + /// is the cheaper place to read them from. + event WalletCreated(bytes32 indexed walletId, address[] indexed owners, address indexed operator); /// @notice Emitted on every successful deposit. - event DepositToWallet(bytes32 indexed walletId, address indexed tokenAddress, uint256 amount); + event Deposited(bytes32 indexed walletId, address indexed tokenAddress, uint256 amount); /// @notice Emitted on every successful withdrawal. - event WithdrawFromWallet( - bytes32 indexed walletId, address indexed tokenAddress, address indexed receiver, uint256 amount - ); + event Withdrawn(bytes32 indexed walletId, address indexed tokenAddress, address indexed receiver, uint256 amount); - /// @notice Emitted on inter-wallet transfers via {transferToWallet}. - event TransferToWallet( + /// @notice Emitted on inter-wallet transfers via {transferBetweenWallets}. + event TransferredBetweenWallets( bytes32 indexed fromWalletId, bytes32 indexed toWalletId, address indexed tokenAddress, @@ -133,11 +144,11 @@ interface ICelerWallet { ); /// @notice Emitted whenever a wallet's operator changes (either path). - event ChangeOperator(bytes32 indexed walletId, address indexed oldOperator, address indexed newOperator); + event OperatorChanged(bytes32 indexed walletId, address indexed oldOperator, address indexed newOperator); - /// @notice Emitted when an owner proposes / votes on a new operator. - event ProposeNewOperator(bytes32 indexed walletId, address indexed newOperator, address indexed proposer); + /// @notice Emitted when an owner casts a vote via {voteForOperator}. + event OperatorVoted(bytes32 indexed walletId, address indexed candidate, address indexed voter); /// @notice Emitted on emergency drains via {drainToken}. - event DrainToken(address indexed tokenAddress, address indexed receiver, uint256 amount); + event TokenDrained(address indexed tokenAddress, address indexed receiver, uint256 amount); } diff --git a/src/lib/AgentPayErrors.sol b/src/lib/AgentPayErrors.sol index afd2923..bd28666 100644 --- a/src/lib/AgentPayErrors.sol +++ b/src/lib/AgentPayErrors.sol @@ -111,6 +111,12 @@ library AgentPayErrors { /// @notice Native transfer via `payable.call{value:}` returned false. error NativeTransferFailed(); + /// @notice `create` was called with more owners than `MAX_OWNERS`. + /// Bounds the loop costs of `_isWalletOwner` / `_clearVotes` / + /// `_checkAllVotes` so a malicious caller can't DoS wallet ops by + /// inflating the owner list. + error TooManyOwners(); + // ------------------------------------------------------------------------- // LedgerOperation: openChannel // ------------------------------------------------------------------------- diff --git a/src/lib/ledgerlib/LedgerMigrate.sol b/src/lib/ledgerlib/LedgerMigrate.sol index 5fb19aa..04cfc0c 100644 --- a/src/lib/ledgerlib/LedgerMigrate.sol +++ b/src/lib/ledgerlib/LedgerMigrate.sol @@ -75,7 +75,9 @@ library LedgerMigrate { bytes32 channelId = ICelerLedger(fromLedgerAddrPayable).migrateChannelTo(_migrationRequest); LedgerStruct.Channel storage c = _self.channelMap[channelId]; require(c.status == LedgerStruct.ChannelStatus.Uninitialized, AgentPayErrors.ChannelAlreadyMigrated()); - require(_self.celerWallet.getOperator(channelId) == address(this), AgentPayErrors.OperatorshipNotTransferred()); + require( + _self.celerWallet.walletOperator(channelId) == address(this), AgentPayErrors.OperatorshipNotTransferred() + ); _self._updateChannelStatus(c, LedgerStruct.ChannelStatus.Operable); // Do not migrate WithdrawIntent, in other words, migration will implicitly veto diff --git a/src/lib/ledgerlib/LedgerOperation.sol b/src/lib/ledgerlib/LedgerOperation.sol index 9e96b76..288032b 100644 --- a/src/lib/ledgerlib/LedgerOperation.sol +++ b/src/lib/ledgerlib/LedgerOperation.sol @@ -696,7 +696,7 @@ library LedgerOperation { // move funds from one channel's wallet to another channel's wallet _self.celerWallet - .transferToWallet(_channelId, _recipientChannelId, c.token.tokenAddress, _receiver, _amount); + .transferBetweenWallets(_channelId, _recipientChannelId, c.token.tokenAddress, _receiver, _amount); } } diff --git a/test/CelerLedger.Migrate.t.sol b/test/CelerLedger.Migrate.t.sol index ec70243..2504594 100644 --- a/test/CelerLedger.Migrate.t.sol +++ b/test/CelerLedger.Migrate.t.sol @@ -142,7 +142,7 @@ contract CelerLedgerMigrateTest is LedgerTestBase { } function _assertOperatorOnChannel(bytes32 _channelId, address _expectedOperator) internal view { - assertEq(celerWallet.getOperator(_channelId), _expectedOperator); + assertEq(celerWallet.walletOperator(_channelId), _expectedOperator); } function _assertMigratedFromOldToNew(bytes32 _channelId) internal view { diff --git a/test/CelerWallet.t.sol b/test/CelerWallet.t.sol index 48dde7c..3e0b1c2 100644 --- a/test/CelerWallet.t.sol +++ b/test/CelerWallet.t.sol @@ -38,20 +38,18 @@ contract CelerWalletTest is Test { bytes32 internal walletId2; event Paused(address account); - event ChangeOperator(bytes32 indexed walletId, address indexed oldOperator, address indexed newOperator); - event ProposeNewOperator(bytes32 indexed walletId, address indexed newOperator, address indexed proposer); - event DepositToWallet(bytes32 indexed walletId, address indexed tokenAddress, uint256 amount); - event WithdrawFromWallet( - bytes32 indexed walletId, address indexed tokenAddress, address indexed receiver, uint256 amount - ); - event TransferToWallet( + event OperatorChanged(bytes32 indexed walletId, address indexed oldOperator, address indexed newOperator); + event OperatorVoted(bytes32 indexed walletId, address indexed candidate, address indexed voter); + event Deposited(bytes32 indexed walletId, address indexed tokenAddress, uint256 amount); + event Withdrawn(bytes32 indexed walletId, address indexed tokenAddress, address indexed receiver, uint256 amount); + event TransferredBetweenWallets( bytes32 indexed fromWalletId, bytes32 indexed toWalletId, address indexed tokenAddress, address receiver, uint256 amount ); - event DrainToken(address indexed tokenAddress, address indexed receiver, uint256 amount); + event TokenDrained(address indexed tokenAddress, address indexed receiver, uint256 amount); function setUp() public { owner = address(this); @@ -100,7 +98,7 @@ contract CelerWalletTest is Test { } function test_walletNum_incrementsOnEachCreate() public view { - assertEq(wallet.walletNum(), 2); + assertEq(wallet.walletCount(), 2); } function test_create_revertsForZeroOperator() public { @@ -120,8 +118,18 @@ contract CelerWalletTest is Test { wallet.create(owners, operator, bytes32(uint256(42))); } + function test_create_revertsForTooManyOwners() public { + // Bound at MAX_OWNERS = 10. Pass 11 to exercise the gate. + address[] memory owners = new address[](wallet.MAX_OWNERS() + 1); + for (uint256 i = 0; i < owners.length; i++) { + owners[i] = address(uint160(0x1000 + i)); + } + vm.expectRevert(AgentPayErrors.TooManyOwners.selector); + wallet.create(owners, operator, bytes32(uint256(7))); + } + function test_getWalletOwners_returnsBothOwners() public view { - address[] memory owners = wallet.getWalletOwners(walletId); + address[] memory owners = wallet.walletOwners(walletId); assertEq(owners.length, 2); assertEq(owners[0], owner0); assertEq(owners[1], owner1); @@ -134,11 +142,11 @@ contract CelerWalletTest is Test { function test_depositNative_emitsEvent_creditsBalance() public { vm.deal(stranger, 1 ether); vm.expectEmit(true, true, false, true, address(wallet)); - emit DepositToWallet(walletId, address(0), 25); + emit Deposited(walletId, address(0), 25); vm.prank(stranger); wallet.depositNative{value: 25}(walletId); - assertEq(wallet.getBalance(walletId, address(0)), 100 + 25); + assertEq(wallet.balanceOf(walletId, address(0)), 100 + 25); } function test_depositERC20_emitsEvent_pullsTokens() public { @@ -147,11 +155,11 @@ contract CelerWalletTest is Test { token.approve(address(wallet), 1000); vm.expectEmit(true, true, false, true, address(wallet)); - emit DepositToWallet(walletId, address(token), 50); + emit Deposited(walletId, address(token), 50); vm.prank(stranger); wallet.depositERC20(walletId, address(token), 50); - assertEq(wallet.getBalance(walletId, address(token)), 200 + 50); + assertEq(wallet.balanceOf(walletId, address(token)), 200 + 50); } // ========================================================================= @@ -160,11 +168,11 @@ contract CelerWalletTest is Test { function test_withdraw_succeeds_emitsEvent() public { vm.expectEmit(true, true, true, true, address(wallet)); - emit WithdrawFromWallet(walletId, address(token), owner0, 80); + emit Withdrawn(walletId, address(token), owner0, 80); vm.prank(operator); wallet.withdraw(walletId, address(token), owner0, 80); - assertEq(wallet.getBalance(walletId, address(token)), 200 - 80); + assertEq(wallet.balanceOf(walletId, address(token)), 200 - 80); assertEq(token.balanceOf(owner0), 100_000 - 200 + 80); } @@ -186,18 +194,18 @@ contract CelerWalletTest is Test { function test_transferToWallet_succeeds_emitsEvent() public { vm.expectEmit(true, true, true, true, address(wallet)); - emit TransferToWallet(walletId, walletId2, address(token), owner0, 50); + emit TransferredBetweenWallets(walletId, walletId2, address(token), owner0, 50); vm.prank(operator); - wallet.transferToWallet(walletId, walletId2, address(token), owner0, 50); + wallet.transferBetweenWallets(walletId, walletId2, address(token), owner0, 50); - assertEq(wallet.getBalance(walletId, address(token)), 200 - 50); - assertEq(wallet.getBalance(walletId2, address(token)), 50); + assertEq(wallet.balanceOf(walletId, address(token)), 200 - 50); + assertEq(wallet.balanceOf(walletId2, address(token)), 50); } function test_transferToWallet_revertsForReceiverNotInBoth() public { vm.expectRevert(AgentPayErrors.NotWalletOwner.selector); vm.prank(operator); - wallet.transferToWallet(walletId, walletId2, address(token), stranger, 50); + wallet.transferBetweenWallets(walletId, walletId2, address(token), stranger, 50); } // ========================================================================= @@ -206,11 +214,11 @@ contract CelerWalletTest is Test { function test_transferOperatorship_byOperator_succeeds_emitsEvent() public { vm.expectEmit(true, true, true, false, address(wallet)); - emit ChangeOperator(walletId, operator, newOperator); + emit OperatorChanged(walletId, operator, newOperator); vm.prank(operator); wallet.transferOperatorship(walletId, newOperator); - assertEq(wallet.getOperator(walletId), newOperator); + assertEq(wallet.walletOperator(walletId), newOperator); } function test_transferOperatorship_revertsForNonOperator() public { @@ -219,65 +227,85 @@ contract CelerWalletTest is Test { wallet.transferOperatorship(walletId, newOperator); } - function test_proposeNewOperator_unanimous_changesOperator() public { - // First owner proposes — operator does not change yet. + function test_voteForOperator_unanimous_changesOperator() public { + // First owner votes — operator does not change yet. vm.expectEmit(true, true, true, false, address(wallet)); - emit ProposeNewOperator(walletId, newOperator, owner0); + emit OperatorVoted(walletId, newOperator, owner0); vm.prank(owner0); - wallet.proposeNewOperator(walletId, newOperator); + wallet.voteForOperator(walletId, newOperator); - assertEq(wallet.getOperator(walletId), operator); + assertEq(wallet.walletOperator(walletId), operator); // Second owner agrees → operator changes; vote tally is then cleared. vm.expectEmit(true, true, true, false, address(wallet)); - emit ProposeNewOperator(walletId, newOperator, owner1); + emit OperatorVoted(walletId, newOperator, owner1); vm.expectEmit(true, true, true, false, address(wallet)); - emit ChangeOperator(walletId, operator, newOperator); + emit OperatorChanged(walletId, operator, newOperator); vm.prank(owner1); - wallet.proposeNewOperator(walletId, newOperator); + wallet.voteForOperator(walletId, newOperator); + + assertEq(wallet.walletOperator(walletId), newOperator); + assertEq(wallet.hasVoted(walletId, owner0), false); + assertEq(wallet.hasVoted(walletId, owner1), false); + // The pendingOperator slot is also cleared on success — leaving + // it stale would confuse the next `voteForOperator` call's reset + // condition (`_candidate != w.pendingOperator`). + assertEq(wallet.pendingOperator(walletId), address(0)); + } + + function test_transferOperatorship_clearsPendingProposal() public { + // owner0 has an in-flight proposal for `newOperator`. + vm.prank(owner0); + wallet.voteForOperator(walletId, newOperator); + assertEq(wallet.pendingOperator(walletId), newOperator); + assertEq(wallet.hasVoted(walletId, owner0), true); - assertEq(wallet.getOperator(walletId), newOperator); - assertEq(wallet.getProposalVote(walletId, owner0), false); - assertEq(wallet.getProposalVote(walletId, owner1), false); + // Current operator transfers operatorship directly — should also wipe + // any pending proposal + vote tally. + vm.prank(operator); + wallet.transferOperatorship(walletId, newOperator); + + assertEq(wallet.pendingOperator(walletId), address(0)); + assertEq(wallet.hasVoted(walletId, owner0), false); } - function test_proposeNewOperator_differentProposalResetsVotes() public { + function test_voteForOperator_differentCandidateResetsVotes() public { vm.prank(owner0); - wallet.proposeNewOperator(walletId, newOperator); - assertEq(wallet.getProposalVote(walletId, owner0), true); + wallet.voteForOperator(walletId, newOperator); + assertEq(wallet.hasVoted(walletId, owner0), true); - // owner1 proposes a different address — the prior tally is wiped. + // owner1 votes for a different candidate — the prior tally is wiped. address otherCandidate = makeAddr("otherCandidate"); vm.prank(owner1); - wallet.proposeNewOperator(walletId, otherCandidate); + wallet.voteForOperator(walletId, otherCandidate); - assertEq(wallet.getProposalVote(walletId, owner0), false); - assertEq(wallet.getProposalVote(walletId, owner1), true); - assertEq(wallet.getProposedNewOperator(walletId), otherCandidate); - assertEq(wallet.getOperator(walletId), operator); + assertEq(wallet.hasVoted(walletId, owner0), false); + assertEq(wallet.hasVoted(walletId, owner1), true); + assertEq(wallet.pendingOperator(walletId), otherCandidate); + assertEq(wallet.walletOperator(walletId), operator); } - function test_proposeNewOperator_revertsForZeroAddress() public { + function test_voteForOperator_revertsForZeroAddress() public { vm.expectRevert(AgentPayErrors.ZeroAddress.selector); vm.prank(owner0); - wallet.proposeNewOperator(walletId, address(0)); + wallet.voteForOperator(walletId, address(0)); } - function test_proposeNewOperator_revertsForNonOwner() public { + function test_voteForOperator_revertsForNonOwner() public { vm.expectRevert(AgentPayErrors.NotWalletOwner.selector); vm.prank(stranger); - wallet.proposeNewOperator(walletId, newOperator); + wallet.voteForOperator(walletId, newOperator); } - function test_proposeNewOperator_succeedsEvenWhenPaused() public { + function test_voteForOperator_succeedsEvenWhenPaused() public { wallet.pause(); vm.expectEmit(true, true, true, false, address(wallet)); - emit ProposeNewOperator(walletId, stranger, owner0); + emit OperatorVoted(walletId, stranger, owner0); vm.prank(owner0); - wallet.proposeNewOperator(walletId, stranger); + wallet.voteForOperator(walletId, stranger); - assertEq(wallet.getProposedNewOperator(walletId), stranger); + assertEq(wallet.pendingOperator(walletId), stranger); } // ========================================================================= @@ -308,7 +336,7 @@ contract CelerWalletTest is Test { vm.deal(stranger, 1 ether); vm.prank(stranger); wallet.depositNative{value: 5}(walletId); - assertEq(wallet.getBalance(walletId, address(0)), 100 + 5); + assertEq(wallet.balanceOf(walletId, address(0)), 100 + 5); } function test_unpause_revertsForNonOwner() public { @@ -342,7 +370,7 @@ contract CelerWalletTest is Test { vm.expectRevert(); vm.prank(operator); - wallet.transferToWallet(walletId, walletId2, address(token), owner0, 50); + wallet.transferBetweenWallets(walletId, walletId2, address(token), owner0, 50); vm.expectRevert(); vm.prank(operator); @@ -364,12 +392,12 @@ contract CelerWalletTest is Test { wallet.pause(); vm.expectEmit(true, true, false, true, address(wallet)); - emit DrainToken(address(0), drainRecipient, 100); + emit TokenDrained(address(0), drainRecipient, 100); wallet.drainToken(address(0), drainRecipient, 100); assertEq(drainRecipient.balance, 100); vm.expectEmit(true, true, false, true, address(wallet)); - emit DrainToken(address(token), stranger, 200); + emit TokenDrained(address(token), stranger, 200); wallet.drainToken(address(token), stranger, 200); assertEq(token.balanceOf(stranger), 200); } @@ -378,12 +406,13 @@ contract CelerWalletTest is Test { // Getters (revert paths) // ========================================================================= - function test_getProposalVote_revertsForNonOwner() public { - vm.expectRevert(AgentPayErrors.NotWalletOwner.selector); - wallet.getProposalVote(walletId, stranger); + function test_getProposalVote_returnsFalseForNonOwner() public view { + // The vote map is non-sensitive — non-owners can read freely; their + // unset entry returns the default (false). + assertEq(wallet.hasVoted(walletId, stranger), false); } - function test_getProposedNewOperator_zeroByDefault() public view { - assertEq(wallet.getProposedNewOperator(walletId), address(0)); + function test_pendingOperator_zeroByDefault() public view { + assertEq(wallet.pendingOperator(walletId), address(0)); } } diff --git a/test/gas_logs/CelerLedger-ERC20.txt b/test/gas_logs/CelerLedger-ERC20.txt index 955f84a..00c714e 100644 --- a/test/gas_logs/CelerLedger-ERC20.txt +++ b/test/gas_logs/CelerLedger-ERC20.txt @@ -1,8 +1,8 @@ ********** Gas Measurement: CelerLedger ERC20 ********** ***** Function Calls Gas Used ***** -openChannel() with zero deposit: 305410 -openChannel() with non-zero ERC20 deposits: 469440 -deposit(): 149979 -cooperativeWithdraw(): 89247 -cooperativeSettle(): 89090 +openChannel() with zero deposit: 305141 +openChannel() with non-zero ERC20 deposits: 468963 +deposit(): 149771 +cooperativeWithdraw(): 89024 +cooperativeSettle(): 88544 diff --git a/test/gas_logs/CelerLedger-ETH.txt b/test/gas_logs/CelerLedger-ETH.txt index 59ffdaf..1413831 100644 --- a/test/gas_logs/CelerLedger-ETH.txt +++ b/test/gas_logs/CelerLedger-ETH.txt @@ -4,26 +4,26 @@ VirtContractResolver Deploy Gas: 176893 NativeWrapMock Deploy Gas: 416605 PayRegistry Deploy Gas: 554814 -CelerWallet Deploy Gas: 1100195 -PayResolver Deploy Gas: 1806957 +CelerWallet Deploy Gas: 1055891 +PayResolver Deploy Gas: 1806985 CelerLedger Deploy Gas: 1820981 ***** Function Calls Gas Used ***** -openChannel() with zero deposit: 301437 -openChannel() using nativeWrap and msg.value: 433046 +openChannel() with zero deposit: 301168 +openChannel() using nativeWrap and msg.value: 432652 setBalanceLimits(): 24553 disableBalanceLimits(): 1132 enableBalanceLimits(): 29544 -deposit() via msg.value: 77964 -deposit() via nativeWrap: 121766 -depositInBatch() with 5 deposits: 526525 +deposit() via msg.value: 77839 +deposit() via nativeWrap: 121641 +depositInBatch() with 5 deposits: 525900 intendWithdraw(): 72783 vetoWithdraw(): 4017 -confirmWithdraw(): 57582 -cooperativeWithdraw(): 92451 +confirmWithdraw(): 57359 +cooperativeWithdraw(): 92228 snapshotStates() with one non-null simplex state: 77707 intendSettle() with a null state: 87577 intendSettle() with two 2-payment-hashList states: 279885 clearPays() with 2 payments: 17012 -confirmSettle(): 48742 -cooperativeSettle(): 93280 +confirmSettle(): 48519 +cooperativeSettle(): 92734 diff --git a/test/gas_logs/CelerLedger-Migrate.txt b/test/gas_logs/CelerLedger-Migrate.txt index 0fb23a4..ee060d1 100644 --- a/test/gas_logs/CelerLedger-Migrate.txt +++ b/test/gas_logs/CelerLedger-Migrate.txt @@ -1,6 +1,6 @@ ********** Gas Measurement: CelerLedger Migration ********** ***** Function Calls Gas Used ***** -migrateChannelFrom() an Operable ETH channel: 302029 -migrateChannelFrom() a Settling ETH channel: 321978 -migrateChannelFrom() an Operable ERC20 channel: 302029 +migrateChannelFrom() an Operable ETH channel: 304259 +migrateChannelFrom() a Settling ETH channel: 324208 +migrateChannelFrom() an Operable ERC20 channel: 304259 diff --git a/test/gas_logs/fine_granularity/DepositEthInBatch.txt b/test/gas_logs/fine_granularity/DepositEthInBatch.txt index 3bae485..48c507e 100644 --- a/test/gas_logs/fine_granularity/DepositEthInBatch.txt +++ b/test/gas_logs/fine_granularity/DepositEthInBatch.txt @@ -1,9 +1,9 @@ ********** Gas Measurement of depositInBatch() - ETH ********** batch size used gas -1 123351 -5 526478 -10 1030661 -25 2544858 -50 5075216 -75 7619654 +1 123226 +5 525853 +10 1029411 +25 2541733 +50 5068966 +75 7610279 diff --git a/test/invariants/ChannelInvariants.t.sol b/test/invariants/ChannelInvariants.t.sol index c7a842b..82abb12 100644 --- a/test/invariants/ChannelInvariants.t.sol +++ b/test/invariants/ChannelInvariants.t.sol @@ -168,7 +168,7 @@ contract ChannelInvariants is LedgerTestBase { bytes32 id = handler.channelIds(i); uint256 status = uint256(celerLedger.getChannelStatus(id)); address token = handler.channelToken(id); - uint256 walletBal = celerWallet.getBalance(id, token); + uint256 walletBal = celerWallet.balanceOf(id, token); (, uint256[2] memory deps, uint256[2] memory wds) = celerLedger.getBalanceMap(id); uint256 netLedger = deps[0] + deps[1] - wds[0] - wds[1]; diff --git a/test/invariants/handlers/ChannelHandler.sol b/test/invariants/handlers/ChannelHandler.sol index 561f354..9cf2618 100644 --- a/test/invariants/handlers/ChannelHandler.sol +++ b/test/invariants/handlers/ChannelHandler.sol @@ -128,7 +128,7 @@ contract ChannelHandler is CommonBase, StdCheats, StdUtils { } function _walletBalanceForChannel(bytes32 _id) internal view returns (uint256) { - return wallet.getBalance(_id, channelToken[_id]); + return wallet.balanceOf(_id, channelToken[_id]); } /// @dev Compute `confirmWithdraw`'s `withdrawLimit` for `_receiver` exactly