From ef5f218740fd736b901832e1b2aab80e96c25a5b Mon Sep 17 00:00:00 2001
From: Xiaozhou Li <xl@cs.princeton.edu>
Date: Fri, 8 May 2026 16:58:45 -0700
Subject: [PATCH 1/4] improve CelerWallet

---
 src/CelerWallet.sol                           | 275 +++++++-----------
 src/lib/AgentPayErrors.sol                    |   6 +
 test/CelerWallet.t.sol                        |  37 ++-
 test/gas_logs/CelerLedger-ERC20.txt           |  10 +-
 test/gas_logs/CelerLedger-ETH.txt             |  22 +-
 test/gas_logs/CelerLedger-Migrate.txt         |   6 +-
 .../fine_granularity/DepositEthInBatch.txt    |  12 +-
 7 files changed, 164 insertions(+), 204 deletions(-)

diff --git a/src/CelerWallet.sol b/src/CelerWallet.sol
index 372804a..6cd4d30 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
@@ -41,40 +39,42 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
 
     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];
@@ -88,62 +88,46 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
         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);
+        wallets[_walletId].balances[address(0)] += amount;
         emit DepositToWallet(_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);
+    /// @inheritdoc ICelerWallet
+    function depositERC20(bytes32 _walletId, address _tokenAddress, uint256 _amount) external whenNotPaused {
+        wallets[_walletId].balances[_tokenAddress] += _amount;
         emit DepositToWallet(_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 Reentrancy considerations: state is debited *before* the external
+     *  transfer (CEI), so re-entry cannot double-spend. `onlyOperator` blocks
+     *  reentrant `withdraw` calls (msg.sender of any re-entry would be the
+     *  receiver, not the operator). Reentering `depositNative` for the same
+     *  wallet is harmless — it credits the wallet but doesn't drain it; the
+     *  net effect is a voluntary donation that CelerLedger's per-peer
+     *  accounting won't reflect, but no balance invariant is violated.
      */
     function withdraw(bytes32 _walletId, address _tokenAddress, address _receiver, uint256 _amount)
-        public
+        external
         whenNotPaused
         onlyOperator(_walletId)
         onlyWalletOwner(_walletId, _receiver)
     {
-        _updateBalance(_walletId, _tokenAddress, _amount, MathOperation.Sub);
+        // 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 WithdrawFromWallet(_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
-     */
+    /// @inheritdoc ICelerWallet
     function transferToWallet(
         bytes32 _fromWalletId,
         bytes32 _toWalletId,
@@ -151,38 +135,31 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
         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);
+        wallets[_fromWalletId].balances[_tokenAddress] -= _amount;
+        wallets[_toWalletId].balances[_tokenAddress] += _amount;
         emit TransferToWallet(_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) {
+    /// @inheritdoc ICelerWallet
+    function proposeNewOperator(bytes32 _walletId, address _newOperator)
+        external
+        onlyWalletOwner(_walletId, msg.sender)
+    {
         require(_newOperator != address(0), AgentPayErrors.ZeroAddress());
 
         Wallet storage w = wallets[_walletId];
@@ -194,83 +171,65 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
         w.proposalVotes[msg.sender] = true;
         emit ProposeNewOperator(_walletId, _newOperator, msg.sender);
 
+        // _changeOperator clears the proposal + vote tally on success.
         if (_checkAllVotes(w)) {
             _changeOperator(_walletId, _newOperator);
-            _clearVotes(w);
         }
     }
 
-    /**
-     * @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 {
+    /// @inheritdoc ICelerWallet
+    function drainToken(address _tokenAddress, address _receiver, uint256 _amount) external whenPaused onlyOwner {
         emit DrainToken(_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
-     */
+    /// @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 getWalletOwners(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 getOperator(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 getBalance(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
-     */
+    /// @inheritdoc ICelerWallet
     function getProposedNewOperator(bytes32 _walletId) external view returns (address) {
         return wallets[_walletId].proposedNewOperator;
     }
 
-    /**
-     * @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)
-    {
+    /// @inheritdoc ICelerWallet
+    function getProposalVote(bytes32 _walletId, address _owner) external view returns (bool) {
         return wallets[_walletId].proposalVotes[_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,53 +239,32 @@ 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 `proposedNewOperator`.
     function _clearVotes(Wallet storage _w) internal {
         for (uint256 i = 0; i < _w.owners.length; i++) {
             _w.proposalVotes[_w.owners[i]] = false;
         }
     }
 
-    /**
-     * @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 new-operator
+    ///  proposal — the just-completed change supersedes any pending vote, and
+    ///  leaving stale state confuses subsequent `proposeNewOperator` 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;
+
+        if (w.proposedNewOperator != address(0)) {
+            _clearVotes(w);
+            delete w.proposedNewOperator;
+        }
+
         emit ChangeOperator(_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 `proposedNewOperator`.
     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) {
@@ -336,12 +274,7 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
         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++) {
@@ -351,14 +284,4 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
         }
         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/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/test/CelerWallet.t.sol b/test/CelerWallet.t.sol
index 48dde7c..05086ab 100644
--- a/test/CelerWallet.t.sol
+++ b/test/CelerWallet.t.sol
@@ -120,6 +120,16 @@ 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);
         assertEq(owners.length, 2);
@@ -239,6 +249,26 @@ contract CelerWalletTest is Test {
         assertEq(wallet.getOperator(walletId), newOperator);
         assertEq(wallet.getProposalVote(walletId, owner0), false);
         assertEq(wallet.getProposalVote(walletId, owner1), false);
+        // The proposed-new-operator slot is also cleared on success — leaving
+        // it stale would confuse the next `proposeNewOperator` call's reset
+        // condition (`_newOperator != w.proposedNewOperator`).
+        assertEq(wallet.getProposedNewOperator(walletId), address(0));
+    }
+
+    function test_transferOperatorship_clearsPendingProposal() public {
+        // owner0 has an in-flight proposal for `newOperator`.
+        vm.prank(owner0);
+        wallet.proposeNewOperator(walletId, newOperator);
+        assertEq(wallet.getProposedNewOperator(walletId), newOperator);
+        assertEq(wallet.getProposalVote(walletId, owner0), true);
+
+        // Current operator transfers operatorship directly — should also wipe
+        // any pending proposal + vote tally.
+        vm.prank(operator);
+        wallet.transferOperatorship(walletId, newOperator);
+
+        assertEq(wallet.getProposedNewOperator(walletId), address(0));
+        assertEq(wallet.getProposalVote(walletId, owner0), false);
     }
 
     function test_proposeNewOperator_differentProposalResetsVotes() public {
@@ -378,9 +408,10 @@ 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.getProposalVote(walletId, stranger), false);
     }
 
     function test_getProposedNewOperator_zeroByDefault() public view {
diff --git a/test/gas_logs/CelerLedger-ERC20.txt b/test/gas_logs/CelerLedger-ERC20.txt
index 955f84a..c1aaee4 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: 469007
+deposit(): 149815
+cooperativeWithdraw(): 89107
+cooperativeSettle(): 88810
diff --git a/test/gas_logs/CelerLedger-ETH.txt b/test/gas_logs/CelerLedger-ETH.txt
index 59ffdaf..2ea2387 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: 1055086
+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: 432607
 setBalanceLimits(): 24553
 disableBalanceLimits(): 1132
 enableBalanceLimits(): 29544
-deposit() via msg.value: 77964
-deposit() via nativeWrap: 121766
-depositInBatch() with 5 deposits: 526525
+deposit() via msg.value: 77794
+deposit() via nativeWrap: 121596
+depositInBatch() with 5 deposits: 525675
 intendWithdraw(): 72783
 vetoWithdraw(): 4017
-confirmWithdraw(): 57582
-cooperativeWithdraw(): 92451
+confirmWithdraw(): 57442
+cooperativeWithdraw(): 92311
 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(): 48602
+cooperativeSettle(): 93000
diff --git a/test/gas_logs/CelerLedger-Migrate.txt b/test/gas_logs/CelerLedger-Migrate.txt
index 0fb23a4..80e8101 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: 304147
+migrateChannelFrom() a Settling ETH channel: 324096
+migrateChannelFrom() an Operable ERC20 channel: 304147
diff --git a/test/gas_logs/fine_granularity/DepositEthInBatch.txt b/test/gas_logs/fine_granularity/DepositEthInBatch.txt
index 3bae485..d044f96 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	123181
+5	525628
+10	1028961
+25	2540608
+50	5066716
+75	7606904

From 8fbd3dd79092f91aa9f61db7c57a813e4e4e56a5 Mon Sep 17 00:00:00 2001
From: Xiaozhou Li <xl@cs.princeton.edu>
Date: Fri, 8 May 2026 17:23:25 -0700
Subject: [PATCH 2/4] further improvements

---
 docs/architecture-summary.md                  |   2 +-
 docs/contracts.md                             |  18 +--
 src/CelerWallet.sol                           |  97 ++++++++-------
 src/interfaces/ICelerWallet.sol               |  68 ++++++-----
 src/lib/ledgerlib/LedgerMigrate.sol           |   4 +-
 src/lib/ledgerlib/LedgerOperation.sol         |   2 +-
 test/CelerLedger.Migrate.t.sol                |   2 +-
 test/CelerWallet.t.sol                        | 110 +++++++++---------
 test/gas_logs/CelerLedger-ERC20.txt           |  10 +-
 test/gas_logs/CelerLedger-ETH.txt             |  22 ++--
 test/gas_logs/CelerLedger-Migrate.txt         |   6 +-
 .../fine_granularity/DepositEthInBatch.txt    |  12 +-
 test/invariants/ChannelInvariants.t.sol       |   2 +-
 test/invariants/handlers/ChannelHandler.sol   |   2 +-
 14 files changed, 188 insertions(+), 169 deletions(-)

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..386009b 100644
--- a/docs/contracts.md
+++ b/docs/contracts.md
@@ -57,17 +57,17 @@ can `pause` / `unpause` and (when paused) `drainToken` to recover stuck funds.
 | [`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). |
+| [`transferBetweenWallets`](../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). |
+| [`voteForOperator`](../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. |
 | `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;
 ```
 
diff --git a/src/CelerWallet.sol b/src/CelerWallet.sol
index 6cd4d30..2c8f711 100644
--- a/src/CelerWallet.sol
+++ b/src/CelerWallet.sol
@@ -30,11 +30,13 @@ 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) {}
@@ -82,9 +84,9 @@ 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;
     }
 
@@ -92,13 +94,13 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
     function depositNative(bytes32 _walletId) external payable whenNotPaused {
         uint256 amount = msg.value;
         wallets[_walletId].balances[address(0)] += amount;
-        emit DepositToWallet(_walletId, address(0), amount);
+        emit Deposited(_walletId, address(0), amount);
     }
 
     /// @inheritdoc ICelerWallet
     function depositERC20(bytes32 _walletId, address _tokenAddress, uint256 _amount) external whenNotPaused {
         wallets[_walletId].balances[_tokenAddress] += _amount;
-        emit DepositToWallet(_walletId, _tokenAddress, _amount);
+        emit Deposited(_walletId, _tokenAddress, _amount);
 
         IERC20(_tokenAddress).safeTransferFrom(msg.sender, address(this), _amount);
     }
@@ -122,13 +124,13 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
         // 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 WithdrawFromWallet(_walletId, _tokenAddress, _receiver, _amount);
+        emit Withdrawn(_walletId, _tokenAddress, _receiver, _amount);
 
         _withdrawToken(_tokenAddress, _receiver, _amount);
     }
 
     /// @inheritdoc ICelerWallet
-    function transferToWallet(
+    function transferBetweenWallets(
         bytes32 _fromWalletId,
         bytes32 _toWalletId,
         address _tokenAddress,
@@ -143,7 +145,7 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
     {
         wallets[_fromWalletId].balances[_tokenAddress] -= _amount;
         wallets[_toWalletId].balances[_tokenAddress] += _amount;
-        emit TransferToWallet(_fromWalletId, _toWalletId, _tokenAddress, _receiver, _amount);
+        emit TransferredBetweenWallets(_fromWalletId, _toWalletId, _tokenAddress, _receiver, _amount);
     }
 
     /// @inheritdoc ICelerWallet
@@ -156,30 +158,29 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
     }
 
     /// @inheritdoc ICelerWallet
-    function proposeNewOperator(bytes32 _walletId, address _newOperator)
-        external
-        onlyWalletOwner(_walletId, msg.sender)
-    {
-        require(_newOperator != address(0), AgentPayErrors.ZeroAddress());
+    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 proposal + vote tally on success.
+        // _changeOperator clears the pendingOperator + vote tally on success.
         if (_checkAllVotes(w)) {
-            _changeOperator(_walletId, _newOperator);
+            _changeOperator(_walletId, _candidate);
         }
     }
 
     /// @inheritdoc ICelerWallet
     function drainToken(address _tokenAddress, address _receiver, uint256 _amount) external whenPaused onlyOwner {
-        emit DrainToken(_tokenAddress, _receiver, _amount);
+        emit TokenDrained(_tokenAddress, _receiver, _amount);
 
         _withdrawToken(_tokenAddress, _receiver, _amount);
     }
@@ -199,28 +200,28 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
     // -------------------------------------------------------------------------
 
     /// @inheritdoc ICelerWallet
-    function getWalletOwners(bytes32 _walletId) external view returns (address[] memory) {
+    function walletOwners(bytes32 _walletId) external view returns (address[] memory) {
         return wallets[_walletId].owners;
     }
 
     /// @inheritdoc ICelerWallet
-    function getOperator(bytes32 _walletId) external view returns (address) {
+    function walletOperator(bytes32 _walletId) external view returns (address) {
         return wallets[_walletId].operator;
     }
 
     /// @inheritdoc ICelerWallet
-    function getBalance(bytes32 _walletId, address _tokenAddress) external view returns (uint256) {
+    function balanceOf(bytes32 _walletId, address _tokenAddress) external view returns (uint256) {
         return wallets[_walletId].balances[_tokenAddress];
     }
 
     /// @inheritdoc ICelerWallet
-    function getProposedNewOperator(bytes32 _walletId) external view returns (address) {
-        return wallets[_walletId].proposedNewOperator;
+    function pendingOperator(bytes32 _walletId) external view returns (address) {
+        return wallets[_walletId].pendingOperator;
     }
 
     /// @inheritdoc ICelerWallet
-    function getProposalVote(bytes32 _walletId, address _owner) external view returns (bool) {
-        return wallets[_walletId].proposalVotes[_owner];
+    function hasVoted(bytes32 _walletId, address _owner) external view returns (bool) {
+        return wallets[_walletId].votes[_owner];
     }
 
     // -------------------------------------------------------------------------
@@ -239,16 +240,20 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
         }
     }
 
-    /// @notice Clear all owners' votes on the current `proposedNewOperator`.
+    /// @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 operator change. Also clears any in-flight new-operator
-    ///  proposal — the just-completed change supersedes any pending vote, and
-    ///  leaving stale state confuses subsequent `proposeNewOperator` calls.
+    /// @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());
 
@@ -256,20 +261,24 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
         address oldOperator = w.operator;
         w.operator = _newOperator;
 
-        if (w.proposedNewOperator != address(0)) {
+        if (w.pendingOperator != address(0)) {
             _clearVotes(w);
-            delete w.proposedNewOperator;
+            delete w.pendingOperator;
         }
 
-        emit ChangeOperator(_walletId, oldOperator, _newOperator);
+        emit OperatorChanged(_walletId, oldOperator, _newOperator);
     }
 
-    /// @notice True iff every owner has voted for the current `proposedNewOperator`.
+    /// @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;
     }
@@ -277,10 +286,14 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
     /// @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;
     }
diff --git a/src/interfaces/ICelerWallet.sol b/src/interfaces/ICelerWallet.sol
index ec5a75c..faae3ad 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,33 @@ 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);
+    event WalletCreated(bytes32 indexed walletId, address[] 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 +139,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/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 05086ab..f116d13 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 newOperator, address indexed proposer);
+    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 {
@@ -131,7 +129,7 @@ contract CelerWalletTest is Test {
     }
 
     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);
@@ -144,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 {
@@ -157,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);
     }
 
     // =========================================================================
@@ -170,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);
     }
 
@@ -196,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);
     }
 
     // =========================================================================
@@ -216,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 {
@@ -232,82 +230,82 @@ contract CelerWalletTest is Test {
     function test_proposeNewOperator_unanimous_changesOperator() public {
         // First owner proposes — 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.getOperator(walletId), newOperator);
-        assertEq(wallet.getProposalVote(walletId, owner0), false);
-        assertEq(wallet.getProposalVote(walletId, owner1), false);
+        assertEq(wallet.walletOperator(walletId), newOperator);
+        assertEq(wallet.hasVoted(walletId, owner0), false);
+        assertEq(wallet.hasVoted(walletId, owner1), false);
         // The proposed-new-operator slot is also cleared on success — leaving
         // it stale would confuse the next `proposeNewOperator` call's reset
         // condition (`_newOperator != w.proposedNewOperator`).
-        assertEq(wallet.getProposedNewOperator(walletId), address(0));
+        assertEq(wallet.pendingOperator(walletId), address(0));
     }
 
     function test_transferOperatorship_clearsPendingProposal() public {
         // owner0 has an in-flight proposal for `newOperator`.
         vm.prank(owner0);
-        wallet.proposeNewOperator(walletId, newOperator);
-        assertEq(wallet.getProposedNewOperator(walletId), newOperator);
-        assertEq(wallet.getProposalVote(walletId, owner0), true);
+        wallet.voteForOperator(walletId, newOperator);
+        assertEq(wallet.pendingOperator(walletId), newOperator);
+        assertEq(wallet.hasVoted(walletId, owner0), true);
 
         // Current operator transfers operatorship directly — should also wipe
         // any pending proposal + vote tally.
         vm.prank(operator);
         wallet.transferOperatorship(walletId, newOperator);
 
-        assertEq(wallet.getProposedNewOperator(walletId), address(0));
-        assertEq(wallet.getProposalVote(walletId, owner0), false);
+        assertEq(wallet.pendingOperator(walletId), address(0));
+        assertEq(wallet.hasVoted(walletId, owner0), false);
     }
 
     function test_proposeNewOperator_differentProposalResetsVotes() 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.
         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 {
         vm.expectRevert(AgentPayErrors.ZeroAddress.selector);
         vm.prank(owner0);
-        wallet.proposeNewOperator(walletId, address(0));
+        wallet.voteForOperator(walletId, address(0));
     }
 
     function test_proposeNewOperator_revertsForNonOwner() public {
         vm.expectRevert(AgentPayErrors.NotWalletOwner.selector);
         vm.prank(stranger);
-        wallet.proposeNewOperator(walletId, newOperator);
+        wallet.voteForOperator(walletId, newOperator);
     }
 
     function test_proposeNewOperator_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);
     }
 
     // =========================================================================
@@ -338,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 {
@@ -372,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);
@@ -394,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);
     }
@@ -411,10 +409,10 @@ contract CelerWalletTest is Test {
     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.getProposalVote(walletId, stranger), false);
+        assertEq(wallet.hasVoted(walletId, stranger), false);
     }
 
     function test_getProposedNewOperator_zeroByDefault() public view {
-        assertEq(wallet.getProposedNewOperator(walletId), address(0));
+        assertEq(wallet.pendingOperator(walletId), address(0));
     }
 }
diff --git a/test/gas_logs/CelerLedger-ERC20.txt b/test/gas_logs/CelerLedger-ERC20.txt
index c1aaee4..a34c316 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: 305141
-openChannel() with non-zero ERC20 deposits: 469007
-deposit(): 149815
-cooperativeWithdraw(): 89107
-cooperativeSettle(): 88810
+openChannel() with zero deposit: 305761
+openChannel() with non-zero ERC20 deposits: 469583
+deposit(): 149771
+cooperativeWithdraw(): 89024
+cooperativeSettle(): 88544
diff --git a/test/gas_logs/CelerLedger-ETH.txt b/test/gas_logs/CelerLedger-ETH.txt
index 2ea2387..0bc726f 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: 1055086
-PayResolver Deploy Gas: 1806985
+CelerWallet Deploy Gas: 1058902
+PayResolver Deploy Gas: 1806981
 CelerLedger Deploy Gas: 1820981
 
 ***** Function Calls Gas Used *****
-openChannel() with zero deposit: 301168
-openChannel() using nativeWrap and msg.value: 432607
+openChannel() with zero deposit: 301788
+openChannel() using nativeWrap and msg.value: 433272
 setBalanceLimits(): 24553
 disableBalanceLimits(): 1132
 enableBalanceLimits(): 29544
-deposit() via msg.value: 77794
-deposit() via nativeWrap: 121596
-depositInBatch() with 5 deposits: 525675
+deposit() via msg.value: 77839
+deposit() via nativeWrap: 121641
+depositInBatch() with 5 deposits: 525900
 intendWithdraw(): 72783
 vetoWithdraw(): 4017
-confirmWithdraw(): 57442
-cooperativeWithdraw(): 92311
+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(): 48602
-cooperativeSettle(): 93000
+confirmSettle(): 48519
+cooperativeSettle(): 92734
diff --git a/test/gas_logs/CelerLedger-Migrate.txt b/test/gas_logs/CelerLedger-Migrate.txt
index 80e8101..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: 304147
-migrateChannelFrom() a Settling ETH channel: 324096
-migrateChannelFrom() an Operable ERC20 channel: 304147
+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 d044f96..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	123181
-5	525628
-10	1028961
-25	2540608
-50	5066716
-75	7606904
+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

From 5f9e99cbd0ab002401d09ed7f56c9675af61598d Mon Sep 17 00:00:00 2001
From: Xiaozhou Li <xl@cs.princeton.edu>
Date: Fri, 8 May 2026 17:30:21 -0700
Subject: [PATCH 3/4] address feedback

---
 docs/contracts.md                   |  4 ++--
 src/interfaces/ICelerWallet.sol     |  9 +++++++--
 test/CelerWallet.t.sol              | 24 ++++++++++++------------
 test/gas_logs/CelerLedger-ERC20.txt |  4 ++--
 test/gas_logs/CelerLedger-ETH.txt   |  8 ++++----
 5 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/docs/contracts.md b/docs/contracts.md
index 386009b..745c648 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.
@@ -59,7 +59,7 @@ can `pause` / `unpause` and (when paused) `drainToken` to recover stuck funds.
 | [`withdraw`](../src/CelerWallet.sol#L119) | operator only | Withdraw funds to a receiver. |
 | [`transferBetweenWallets`](../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). |
-| [`voteForOperator`](../src/CelerWallet.sol#L179) | wallet owner | Propose a new operator; takes effect when *all* owners propose the same address (manual fallback for stuck migrations). |
+| [`voteForOperator`](../src/CelerWallet.sol#L179) | 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#L207) | contract owner, when paused | Emergency token recovery. |
 | `pause` / `unpause` | contract owner | Pause guard for deposits / withdrawals / operator changes. |
 | `walletOwners` / `walletOperator` / `balanceOf` / `pendingOperator` / `hasVoted` | view | Wallet introspection. |
diff --git a/src/interfaces/ICelerWallet.sol b/src/interfaces/ICelerWallet.sol
index faae3ad..a79e6e3 100644
--- a/src/interfaces/ICelerWallet.sol
+++ b/src/interfaces/ICelerWallet.sol
@@ -120,8 +120,13 @@ interface ICelerWallet {
     /// @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 WalletCreated(bytes32 indexed walletId, address[] 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 Deposited(bytes32 indexed walletId, address indexed tokenAddress, uint256 amount);
diff --git a/test/CelerWallet.t.sol b/test/CelerWallet.t.sol
index f116d13..3e0b1c2 100644
--- a/test/CelerWallet.t.sol
+++ b/test/CelerWallet.t.sol
@@ -39,7 +39,7 @@ contract CelerWalletTest is Test {
 
     event Paused(address account);
     event OperatorChanged(bytes32 indexed walletId, address indexed oldOperator, address indexed newOperator);
-    event OperatorVoted(bytes32 indexed walletId, address indexed newOperator, address indexed proposer);
+    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(
@@ -227,8 +227,8 @@ 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 OperatorVoted(walletId, newOperator, owner0);
         vm.prank(owner0);
@@ -247,9 +247,9 @@ contract CelerWalletTest is Test {
         assertEq(wallet.walletOperator(walletId), newOperator);
         assertEq(wallet.hasVoted(walletId, owner0), false);
         assertEq(wallet.hasVoted(walletId, owner1), false);
-        // The proposed-new-operator slot is also cleared on success — leaving
-        // it stale would confuse the next `proposeNewOperator` call's reset
-        // condition (`_newOperator != w.proposedNewOperator`).
+        // 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));
     }
 
@@ -269,12 +269,12 @@ contract CelerWalletTest is Test {
         assertEq(wallet.hasVoted(walletId, owner0), false);
     }
 
-    function test_proposeNewOperator_differentProposalResetsVotes() public {
+    function test_voteForOperator_differentCandidateResetsVotes() public {
         vm.prank(owner0);
         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.voteForOperator(walletId, otherCandidate);
@@ -285,19 +285,19 @@ contract CelerWalletTest is Test {
         assertEq(wallet.walletOperator(walletId), operator);
     }
 
-    function test_proposeNewOperator_revertsForZeroAddress() public {
+    function test_voteForOperator_revertsForZeroAddress() public {
         vm.expectRevert(AgentPayErrors.ZeroAddress.selector);
         vm.prank(owner0);
         wallet.voteForOperator(walletId, address(0));
     }
 
-    function test_proposeNewOperator_revertsForNonOwner() public {
+    function test_voteForOperator_revertsForNonOwner() public {
         vm.expectRevert(AgentPayErrors.NotWalletOwner.selector);
         vm.prank(stranger);
         wallet.voteForOperator(walletId, newOperator);
     }
 
-    function test_proposeNewOperator_succeedsEvenWhenPaused() public {
+    function test_voteForOperator_succeedsEvenWhenPaused() public {
         wallet.pause();
 
         vm.expectEmit(true, true, true, false, address(wallet));
@@ -412,7 +412,7 @@ contract CelerWalletTest is Test {
         assertEq(wallet.hasVoted(walletId, stranger), false);
     }
 
-    function test_getProposedNewOperator_zeroByDefault() public view {
+    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 a34c316..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: 305761
-openChannel() with non-zero ERC20 deposits: 469583
+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 0bc726f..1413831 100644
--- a/test/gas_logs/CelerLedger-ETH.txt
+++ b/test/gas_logs/CelerLedger-ETH.txt
@@ -4,13 +4,13 @@
 VirtContractResolver Deploy Gas: 176893
 NativeWrapMock Deploy Gas: 416605
 PayRegistry Deploy Gas: 554814
-CelerWallet Deploy Gas: 1058902
-PayResolver Deploy Gas: 1806981
+CelerWallet Deploy Gas: 1055891
+PayResolver Deploy Gas: 1806985
 CelerLedger Deploy Gas: 1820981
 
 ***** Function Calls Gas Used *****
-openChannel() with zero deposit: 301788
-openChannel() using nativeWrap and msg.value: 433272
+openChannel() with zero deposit: 301168
+openChannel() using nativeWrap and msg.value: 432652
 setBalanceLimits(): 24553
 disableBalanceLimits(): 1132
 enableBalanceLimits(): 29544

From 619b64f147a079b6899b5153d1588691c6594805 Mon Sep 17 00:00:00 2001
From: Xiaozhou Li <xl@cs.princeton.edu>
Date: Fri, 8 May 2026 17:44:58 -0700
Subject: [PATCH 4/4] update docs and comments

---
 docs/contracts.md   | 72 ++++++++++++++++++++++-----------------------
 src/CelerWallet.sol | 11 +++----
 2 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/docs/contracts.md b/docs/contracts.md
index 745c648..a69e926 100644
--- a/docs/contracts.md
+++ b/docs/contracts.md
@@ -53,14 +53,14 @@ 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. |
-| [`transferBetweenWallets`](../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). |
-| [`voteForOperator`](../src/CelerWallet.sol#L179) | 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#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. |
 | `walletOwners` / `walletOperator` / `balanceOf` / `pendingOperator` / `hasVoted` | view | Wallet introspection. |
 
@@ -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 2c8f711..607515c 100644
--- a/src/CelerWallet.sol
+++ b/src/CelerWallet.sol
@@ -107,13 +107,10 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable {
 
     /**
      * @inheritdoc ICelerWallet
-     * @dev Reentrancy considerations: state is debited *before* the external
-     *  transfer (CEI), so re-entry cannot double-spend. `onlyOperator` blocks
-     *  reentrant `withdraw` calls (msg.sender of any re-entry would be the
-     *  receiver, not the operator). Reentering `depositNative` for the same
-     *  wallet is harmless — it credits the wallet but doesn't drain it; the
-     *  net effect is a voluntary donation that CelerLedger's per-peer
-     *  accounting won't reflect, but no balance invariant is violated.
+     * @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)
         external