From ae364723551f70635ae667d0896cfd2d1e7b9123 Mon Sep 17 00:00:00 2001 From: Xiaozhou Li Date: Sat, 2 May 2026 23:16:49 -0700 Subject: [PATCH 1/3] replay protection --- docs/contracts.md | 10 ++ src/CelerWallet.sol | 3 +- src/PayResolver.sol | 2 + src/lib/data/PbEntity.sol | 12 ++ src/lib/data/proto/entity.proto | 11 +- src/lib/interface/ICelerLedger.sol | 6 +- src/lib/interface/ICelerWallet.sol | 4 +- src/lib/interface/IPayResolver.sol | 4 +- src/lib/ledgerlib/LedgerOperation.sol | 3 + test/CelerLedger.ETH.t.sol | 107 +++++++++++++++++- test/CelerWallet.t.sol | 4 +- test/GasReport.t.sol | 9 +- test/PayResolver.t.sol | 52 ++++++++- test/gas_logs/CelerLedger-ERC20.txt | 8 +- test/gas_logs/CelerLedger-ETH.txt | 24 ++-- test/gas_logs/PayResolver.txt | 4 +- test/gas_logs/fine_granularity/ClearPays.txt | 6 +- .../fine_granularity/DepositEthInBatch.txt | 8 +- .../IntendSettle-OneState.txt | 14 +-- test/utils/Fixtures.sol | 6 + test/utils/LedgerTestBase.t.sol | 12 +- test/utils/Proto.t.sol | 13 ++- 22 files changed, 267 insertions(+), 55 deletions(-) diff --git a/docs/contracts.md b/docs/contracts.md index 006671c..0ca105a 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -80,6 +80,15 @@ uint256 public walletNum; mapping(bytes32 => Wallet) private wallets; ``` +### Wallet ID derivation + +``` +walletId = keccak256(chainid, walletAddr, operatorAddr, nonce) +``` + +The `chainid` prefix prevents cross-chain channel-id collisions when wallet and +operator addresses coincide across chains. + --- ## CelerLedger @@ -205,6 +214,7 @@ constructor(address _registryAddr, address _virtResolverAddr) ### Resolution rules +- A payment's `chain_id` must equal `block.chainid`. - A payment must be resolved before `pay.resolveDeadline` (Unix timestamp, seconds). - A result equal to the maximum-transfer amount **finalizes immediately** — no challenge window. diff --git a/src/CelerWallet.sol b/src/CelerWallet.sol index 05eda82..49f1bc8 100644 --- a/src/CelerWallet.sol +++ b/src/CelerWallet.sol @@ -61,6 +61,7 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable { /** * @notice Create a new wallet + * @dev `walletId = keccak256(chainid, walletAddr, operatorAddr, nonce)`. * @param _owners owners of the wallet * @param _operator initial operator of the wallet * @param _nonce nonce given by caller to generate the wallet id @@ -73,7 +74,7 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable { { require(_operator != address(0), "New operator is address(0)"); - bytes32 walletId = keccak256(abi.encodePacked(address(this), msg.sender, _nonce)); + bytes32 walletId = keccak256(abi.encodePacked(block.chainid, address(this), msg.sender, _nonce)); Wallet storage w = wallets[walletId]; // wallet must be uninitialized require(w.operator == address(0), "Occupied wallet id"); diff --git a/src/PayResolver.sol b/src/PayResolver.sol index 69afbe0..3f2af19 100644 --- a/src/PayResolver.sol +++ b/src/PayResolver.sol @@ -91,6 +91,8 @@ contract PayResolver is IPayResolver { * @param _amount payment amount to resolve */ function _resolvePayment(PbEntity.ConditionalPay memory _pay, bytes32 _payHash, uint256 _amount) internal { + // bind the signed pay to its intended chain + require(_pay.chainId == block.chainid, "Wrong chain id for pay"); uint256 nowTs = block.timestamp; require(nowTs <= _pay.resolveDeadline, "Passed pay resolve deadline in condPay msg"); diff --git a/src/lib/data/PbEntity.sol b/src/lib/data/PbEntity.sol index 2640eca..0362e2c 100644 --- a/src/lib/data/PbEntity.sol +++ b/src/lib/data/PbEntity.sol @@ -274,6 +274,7 @@ library PbEntity { uint256 resolveDeadline; // tag: 6 uint256 resolveTimeout; // tag: 7 address payResolver; // tag: 8 + uint256 chainId; // tag: 9 } // end struct ConditionalPay function decConditionalPay(bytes memory raw) internal pure returns (ConditionalPay memory m) { @@ -313,6 +314,9 @@ library PbEntity { } else if (key == 66) { // tag 8 m.payResolver = buf.decAddress(); + } else if (key == 72) { + // tag 9 + m.chainId = buf.decVarint(); } else { buf.skipValue(Pb.WireType(key & 7)); // unknown tag or wrong wire } @@ -455,6 +459,8 @@ library PbEntity { uint256 openDeadline; // tag: 2 uint256 disputeTimeout; // tag: 3 uint256 msgValueReceiver; // tag: 4 + uint256 chainId; // tag: 5 + address ledgerAddress; // tag: 6 } // end struct PaymentChannelInitializer function decPaymentChannelInitializer(bytes memory raw) internal pure returns (PaymentChannelInitializer memory m) { @@ -475,6 +481,12 @@ library PbEntity { } else if (key == 32) { // tag 4 m.msgValueReceiver = buf.decVarint(); + } else if (key == 40) { + // tag 5 + m.chainId = buf.decVarint(); + } else if (key == 50) { + // tag 6 + m.ledgerAddress = buf.decAddress(); } else { buf.skipValue(Pb.WireType(key & 7)); // unknown tag or wrong wire } diff --git a/src/lib/data/proto/entity.proto b/src/lib/data/proto/entity.proto index aca6b94..bee5e6a 100644 --- a/src/lib/data/proto/entity.proto +++ b/src/lib/data/proto/entity.proto @@ -121,6 +121,9 @@ message ConditionalPay { // resolve_timeout is the dispute window after a resolve payment request is submitted uint64 resolve_timeout = 7 [(soltype) = "uint"]; bytes pay_resolver = 8 [(soltype) = "address"]; + // chain id of the intended target chain; binds the signed pay (and any + // vouched result over it) against cross-chain replay + uint64 chain_id = 9 [(soltype) = "uint"]; } // Next Tag: 3 @@ -169,7 +172,7 @@ message CooperativeWithdrawInfo { bytes recipient_channel_id = 5 [(soltype) = "bytes32"]; } -// Next Tag: 5 +// Next Tag: 7 message PaymentChannelInitializer { // require an ascending order based on addresses of init_distribution.distribution[].account TokenDistribution init_distribution = 1; @@ -178,6 +181,12 @@ message PaymentChannelInitializer { // index of channel peer who receives the blockchain native token // associated with the transaction (msg.value in case of ETH) uint64 msg_value_receiver = 4 [(soltype) = "uint"]; + // chain id of the intended target chain; binds the signed initializer + // against cross-chain replay + uint64 chain_id = 5 [(soltype) = "uint"]; + // address of the intended target CelerLedger; binds the signed initializer + // against same-chain wrong-ledger replay + bytes ledger_address = 6 [(soltype) = "address"]; } // Next Tag: 5 diff --git a/src/lib/interface/ICelerLedger.sol b/src/lib/interface/ICelerLedger.sol index e5cb2b5..6608d45 100644 --- a/src/lib/interface/ICelerLedger.sol +++ b/src/lib/interface/ICelerLedger.sol @@ -22,8 +22,10 @@ interface ICelerLedger { /** * @notice Open a fully-funded channel from a co-signed initializer in one tx. * @dev Atomically creates a wallet in the underlying CelerWallet, derives - * `channelId = keccak256(walletAddr, ledgerAddr, keccak256(initializer))`, and - * pulls the initial deposits. Native value is allowed via `msg.value`. + * `channelId = keccak256(chainid, walletAddr, ledgerAddr, keccak256(initializer))`, + * and pulls the initial deposits. Native value is allowed via `msg.value`. + * The initializer's `chain_id` and `ledger_address` must match this + * contract's execution domain. * @param _openChannelRequest ABI-encoded `PbChain.OpenChannelRequest` message. */ function openChannel(bytes calldata _openChannelRequest) external payable; diff --git a/src/lib/interface/ICelerWallet.sol b/src/lib/interface/ICelerWallet.sol index dd7bdc5..354a317 100644 --- a/src/lib/interface/ICelerWallet.sol +++ b/src/lib/interface/ICelerWallet.sol @@ -13,8 +13,8 @@ pragma solidity ^0.8.20; interface ICelerWallet { /** * @notice Create a new wallet. - * @dev `walletId = keccak256(walletContract, msg.sender, _nonce)`. Reverts if the - * derived id is already in use or if `_operator == address(0)`. + * @dev `walletId = keccak256(chainid, walletContract, msg.sender, _nonce)`. + * Reverts if the derived id is already in use or if `_operator == address(0)`. * @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. diff --git a/src/lib/interface/IPayResolver.sol b/src/lib/interface/IPayResolver.sol index dc86e69..36a0e9b 100644 --- a/src/lib/interface/IPayResolver.sol +++ b/src/lib/interface/IPayResolver.sol @@ -22,7 +22,8 @@ interface IPayResolver { * @dev All conditions must already be finalized; the call reverts otherwise. * Hash-lock conditions are validated against `_resolvePayRequest.hashPreimages` * in the order they appear in `pay.conditions`. Virtual-contract conditions - * must already be materialized via the VirtContractResolver. + * must already be materialized via the VirtContractResolver. The pay's + * `chain_id` must equal `block.chainid`. * @param _resolvePayRequest ABI-encoded `PbChain.ResolvePayByConditionsRequest`. */ function resolvePaymentByConditions(bytes calldata _resolvePayRequest) external; @@ -31,6 +32,7 @@ interface IPayResolver { * @notice Resolve a payment by submitting an off-chain result co-signed by * the payment's source and destination. * @dev The submitted amount must not exceed `pay.transferFunc.maxTransfer.receiver.amt`. + * The pay's `chain_id` must equal `block.chainid`. * @param _vouchedPayResult ABI-encoded `PbEntity.VouchedCondPayResult`. */ function resolvePaymentByVouchedResult(bytes calldata _vouchedPayResult) external; diff --git a/src/lib/ledgerlib/LedgerOperation.sol b/src/lib/ledgerlib/LedgerOperation.sol index 8b51865..bd3c388 100644 --- a/src/lib/ledgerlib/LedgerOperation.sol +++ b/src/lib/ledgerlib/LedgerOperation.sol @@ -33,6 +33,9 @@ library LedgerOperation { PbEntity.decPaymentChannelInitializer(openRequest.channelInitializer); require(channelInitializer.initDistribution.distribution.length == 2, "Wrong length"); require(block.timestamp <= channelInitializer.openDeadline, "Open deadline passed"); + // bind the co-signed initializer to its intended (chain, ledger) target + require(channelInitializer.chainId == block.chainid, "Wrong chain id for open"); + require(channelInitializer.ledgerAddress == address(this), "Wrong ledger for open"); PbEntity.TokenInfo memory token = channelInitializer.initDistribution.token; uint256[2] memory amounts = [ diff --git a/test/CelerLedger.ETH.t.sol b/test/CelerLedger.ETH.t.sol index 5315b3f..f68a87b 100644 --- a/test/CelerLedger.ETH.t.sol +++ b/test/CelerLedger.ETH.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; import {LedgerTestBase} from "./utils/LedgerTestBase.t.sol"; import {LedgerStruct} from "../src/lib/ledgerlib/LedgerStruct.sol"; +import {CelerLedger} from "../src/CelerLedger.sol"; import {Fixtures} from "./utils/Fixtures.sol"; import {SignUtil} from "./utils/SignUtil.sol"; @@ -87,6 +88,109 @@ contract CelerLedgerEthTest is LedgerTestBase { assertEq(deposits[1], 200); } + // ========================================================================= + // 2b. Open-channel replay protection (chain id + ledger address binding) + // ========================================================================= + + function test_openChannel_replayedOnWrongChain_reverts() public { + // Build a co-signed initializer claiming chainid = block.chainid + 1. + // Submitting on the live chain must revert with "Wrong chain id for open" + // even though the signatures are individually valid. + Fixtures.PaymentChannelInitializer memory init = Fixtures.PaymentChannelInitializer({ + tokenType: 1, + tokenAddress: address(0), + peers: [peer0, peer1], + amounts: [uint256(0), 0], + openDeadline: openDeadlineCursor++, + disputeTimeout: DISPUTE_TIMEOUT, + msgValueReceiver: 0, + chainId: block.chainid + 1, + ledgerAddress: address(celerLedger) + }); + bytes memory initializer = Fixtures.encPaymentChannelInitializer(init); + bytes[] memory sigs = SignUtil.coSign(peer0Pk, peer1Pk, initializer); + bytes memory request = Fixtures.encOpenChannelRequest(initializer, sigs); + + vm.expectRevert(bytes("Wrong chain id for open")); + celerLedger.openChannel(request); + } + + function test_openChannel_replayedOnWrongLedger_reverts() public { + // Initializer is bound to a sibling ledger (a fresh CelerLedger sharing + // the same wallet+pool+registry). Replaying the same co-signed request + // against the original `celerLedger` must revert. + address otherLedger = address(new CelerLedger(address(ethPool), address(payRegistry), address(celerWallet))); + + Fixtures.PaymentChannelInitializer memory init = Fixtures.PaymentChannelInitializer({ + tokenType: 1, + tokenAddress: address(0), + peers: [peer0, peer1], + amounts: [uint256(0), 0], + openDeadline: openDeadlineCursor++, + disputeTimeout: DISPUTE_TIMEOUT, + msgValueReceiver: 0, + chainId: block.chainid, + ledgerAddress: otherLedger + }); + bytes memory initializer = Fixtures.encPaymentChannelInitializer(init); + bytes[] memory sigs = SignUtil.coSign(peer0Pk, peer1Pk, initializer); + bytes memory request = Fixtures.encOpenChannelRequest(initializer, sigs); + + vm.expectRevert(bytes("Wrong ledger for open")); + celerLedger.openChannel(request); + } + + function test_openChannel_signedOnOneChain_replayedOnAnother_reverts() public { + // Direct cross-chain replay: peers co-sign a *valid* initializer for the + // current chain, then the same signed bytes are submitted after the + // chain context flips via vm.chainId. + Fixtures.PaymentChannelInitializer memory init = Fixtures.PaymentChannelInitializer({ + tokenType: 1, + tokenAddress: address(0), + peers: [peer0, peer1], + amounts: [uint256(0), 0], + openDeadline: openDeadlineCursor++, + disputeTimeout: DISPUTE_TIMEOUT, + msgValueReceiver: 0, + chainId: block.chainid, + ledgerAddress: address(celerLedger) + }); + bytes memory initializer = Fixtures.encPaymentChannelInitializer(init); + bytes[] memory sigs = SignUtil.coSign(peer0Pk, peer1Pk, initializer); + bytes memory request = Fixtures.encOpenChannelRequest(initializer, sigs); + + // Switch chain context — the same signed payload is now mismatched. + vm.chainId(block.chainid + 1); + + vm.expectRevert(bytes("Wrong chain id for open")); + celerLedger.openChannel(request); + } + + function test_openChannel_signedForOneLedger_replayedToSibling_reverts() public { + // Direct cross-ledger replay: peers co-sign a *valid* initializer for + // `celerLedger`; the same signed bytes are then submitted to a sibling + // ledger sharing the same wallet+pool+registry. + CelerLedger siblingLedger = new CelerLedger(address(ethPool), address(payRegistry), address(celerWallet)); + + Fixtures.PaymentChannelInitializer memory init = Fixtures.PaymentChannelInitializer({ + tokenType: 1, + tokenAddress: address(0), + peers: [peer0, peer1], + amounts: [uint256(0), 0], + openDeadline: openDeadlineCursor++, + disputeTimeout: DISPUTE_TIMEOUT, + msgValueReceiver: 0, + chainId: block.chainid, + ledgerAddress: address(celerLedger) + }); + bytes memory initializer = Fixtures.encPaymentChannelInitializer(init); + bytes[] memory sigs = SignUtil.coSign(peer0Pk, peer1Pk, initializer); + bytes memory request = Fixtures.encOpenChannelRequest(initializer, sigs); + + vm.expectRevert(bytes("Wrong ledger for open")); + siblingLedger.openChannel(request); + } + // ========================================================================= // 3. Balance-limit admin // ========================================================================= @@ -996,7 +1100,8 @@ contract CelerLedgerEthTest is LedgerTestBase { maxAmount: _maxAmount, resolveDeadline: 9_999_999, resolveTimeout: 5, - payResolver: address(payResolver) + payResolver: address(payResolver), + chainId: block.chainid }); bytes memory payBytes = Fixtures.encConditionalPay(pay); diff --git a/test/CelerWallet.t.sol b/test/CelerWallet.t.sol index c90b63c..5c925b0 100644 --- a/test/CelerWallet.t.sol +++ b/test/CelerWallet.t.sol @@ -83,10 +83,10 @@ contract CelerWalletTest is Test { /// @dev Replicates `WalletTestHelper.create` + `CelerWallet.create` id derivation: /// helper hashes the user nonce into `bytes32 n`, then the wallet derives - /// `id = keccak256(walletAddr, helperAddr, n)`. + /// `id = keccak256(chainid, walletAddr, helperAddr, n)`. function _walletIdViaHelper(uint256 _nonce) internal view returns (bytes32) { bytes32 n = keccak256(abi.encodePacked(_nonce)); - return keccak256(abi.encodePacked(address(wallet), address(walletHelper), n)); + return keccak256(abi.encodePacked(block.chainid, address(wallet), address(walletHelper), n)); } // ========================================================================= diff --git a/test/GasReport.t.sol b/test/GasReport.t.sol index 5d9fef9..35cc509 100644 --- a/test/GasReport.t.sol +++ b/test/GasReport.t.sol @@ -649,7 +649,8 @@ contract GasReport is LedgerTestBase { maxAmount: 10, resolveDeadline: 9_999_999, resolveTimeout: 5, - payResolver: address(payResolver) + payResolver: address(payResolver), + chainId: block.chainid }); bytes memory payBytes = Fixtures.encConditionalPay(pay); bytes[] memory preimages = new bytes[](1); @@ -676,7 +677,8 @@ contract GasReport is LedgerTestBase { maxAmount: 100, resolveDeadline: 9_999_999, resolveTimeout: 10, - payResolver: address(payResolver) + payResolver: address(payResolver), + chainId: block.chainid }); bytes memory payBytes = Fixtures.encConditionalPay(pay); bytes memory result = Fixtures.encCondPayResult(payBytes, 20); @@ -831,7 +833,8 @@ contract GasReport is LedgerTestBase { maxAmount: _maxAmount, resolveDeadline: 9_999_999, resolveTimeout: 5, - payResolver: address(payResolver) + payResolver: address(payResolver), + chainId: block.chainid }); bytes memory payBytes = Fixtures.encConditionalPay(pay); bytes[] memory preimages = new bytes[](1); diff --git a/test/PayResolver.t.sol b/test/PayResolver.t.sol index 8bb2453..ae0cd27 100644 --- a/test/PayResolver.t.sol +++ b/test/PayResolver.t.sol @@ -138,7 +138,8 @@ contract PayResolverTest is Test { maxAmount: _maxAmount, resolveDeadline: _resolveDeadline, resolveTimeout: RESOLVE_TIMEOUT, - payResolver: address(payResolver) + payResolver: address(payResolver), + chainId: block.chainid }); return Fixtures.encConditionalPay(pay); } @@ -362,7 +363,8 @@ contract PayResolverTest is Test { maxAmount: 50, resolveDeadline: RESOLVE_DEADLINE, resolveTimeout: RESOLVE_TIMEOUT, - payResolver: address(payResolver) + payResolver: address(payResolver), + chainId: block.chainid }); return Fixtures.encConditionalPay(pay); } @@ -401,6 +403,49 @@ contract PayResolverTest is Test { payResolver.resolvePaymentByConditions(Fixtures.encResolvePayByConditionsRequest(payBytes, preimages)); } + // ------------------------------------------------------------------------- + // Wrong chain id (replay protection) + // ------------------------------------------------------------------------- + + /// @dev Build a hash-lock-only ConditionalPay with an explicit `chainId` + /// override. Used by the wrong-chain-id revert tests below. + function _buildPayWithChainId(uint256 _payTimestamp, uint256 _chainId) internal view returns (bytes memory) { + Fixtures.Condition[] memory conds = new Fixtures.Condition[](1); + conds[0] = Fixtures.condHashLock(_hashLockTrue()); + + Fixtures.ConditionalPay memory pay = Fixtures.ConditionalPay({ + payTimestamp: _payTimestamp, + src: payerSrc, + dest: payerDest, + conditions: conds, + logicType: 0, // BOOLEAN_AND + maxAmount: 50, + resolveDeadline: RESOLVE_DEADLINE, + resolveTimeout: RESOLVE_TIMEOUT, + payResolver: address(payResolver), + chainId: _chainId + }); + return Fixtures.encConditionalPay(pay); + } + + function test_resolveByConditions_wrongChainId_reverts() public { + // Pay is bound to a chainid one greater than the current chain. + bytes memory payBytes = _buildPayWithChainId(30, block.chainid + 1); + + bytes[] memory preimages = new bytes[](1); + preimages[0] = TRUE_PREIMAGE; + vm.expectRevert(bytes("Wrong chain id for pay")); + payResolver.resolvePaymentByConditions(Fixtures.encResolvePayByConditionsRequest(payBytes, preimages)); + } + + function test_resolveByVouchedResult_wrongChainId_reverts() public { + // Same wrong-chainid pay submitted via the vouched-result path. + bytes memory payBytes = _buildPayWithChainId(31, block.chainid + 1); + + vm.expectRevert(bytes("Wrong chain id for pay")); + payResolver.resolvePaymentByVouchedResult(_vouched(payBytes, 20)); + } + // ------------------------------------------------------------------------- // Numeric logic // ------------------------------------------------------------------------- @@ -500,7 +545,8 @@ contract PayResolverTest is Test { maxAmount: 50, resolveDeadline: RESOLVE_DEADLINE, resolveTimeout: RESOLVE_TIMEOUT, - payResolver: address(payResolver) + payResolver: address(payResolver), + chainId: block.chainid }); bytes memory payBytes = Fixtures.encConditionalPay(pay); bytes32 expectedPayId = keccak256(abi.encodePacked(keccak256(payBytes), address(payResolver))); diff --git a/test/gas_logs/CelerLedger-ERC20.txt b/test/gas_logs/CelerLedger-ERC20.txt index 188f23c..4894aaa 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: 302980 -openChannel() with non-zero ERC20 deposits: 466694 +openChannel() with zero deposit: 305422 +openChannel() with non-zero ERC20 deposits: 469132 deposit(): 150001 -cooperativeWithdraw(): 89177 -cooperativeSettle(): 88887 +cooperativeWithdraw(): 89182 +cooperativeSettle(): 88893 diff --git a/test/gas_logs/CelerLedger-ETH.txt b/test/gas_logs/CelerLedger-ETH.txt index 176e09f..d4e346a 100644 --- a/test/gas_logs/CelerLedger-ETH.txt +++ b/test/gas_logs/CelerLedger-ETH.txt @@ -4,26 +4,26 @@ VirtContractResolver Deploy Gas: 206156 EthPool Deploy Gas: 558320 PayRegistry Deploy Gas: 565565 -CelerWallet Deploy Gas: 1147513 -PayResolver Deploy Gas: 1871491 -CelerLedger Deploy Gas: 1920297 +CelerWallet Deploy Gas: 1148513 +PayResolver Deploy Gas: 1893745 +CelerLedger Deploy Gas: 1920284 ***** Function Calls Gas Used ***** -openChannel() with zero deposit: 299046 -openChannel() using EthPool and msg.value: 410006 -setBalanceLimits(): 26122 +openChannel() with zero deposit: 301454 +openChannel() using EthPool and msg.value: 412371 +setBalanceLimits(): 26123 disableBalanceLimits(): 1688 enableBalanceLimits(): 32589 deposit() via msg.value: 78052 deposit() via EthPool: 91281 -depositInBatch() with 5 deposits: 382069 +depositInBatch() with 5 deposits: 382082 intendWithdraw(): 72739 vetoWithdraw(): 3995 confirmWithdraw(): 57548 -cooperativeWithdraw(): 92353 -snapshotStates() with one non-null simplex state: 77679 -intendSettle() with a null state: 87581 -intendSettle() with two 2-payment-hashList states: 279839 +cooperativeWithdraw(): 92387 +snapshotStates() with one non-null simplex state: 77674 +intendSettle() with a null state: 87600 +intendSettle() with two 2-payment-hashList states: 279887 clearPays() with 2 payments: 16990 confirmSettle(): 48721 -cooperativeSettle(): 93056 +cooperativeSettle(): 93083 diff --git a/test/gas_logs/PayResolver.txt b/test/gas_logs/PayResolver.txt index 90f50d0..17cd683 100644 --- a/test/gas_logs/PayResolver.txt +++ b/test/gas_logs/PayResolver.txt @@ -1,5 +1,5 @@ ********** Gas Measurement: PayResolver ********** ***** Function Calls Gas Used ***** -resolvePaymentByConditions(): 77247 -resolvePaymentByVouchedResult(): 95538 +resolvePaymentByConditions(): 78445 +resolvePaymentByVouchedResult(): 96729 diff --git a/test/gas_logs/fine_granularity/ClearPays.txt b/test/gas_logs/fine_granularity/ClearPays.txt index ceeeda0..d4e50b2 100644 --- a/test/gas_logs/fine_granularity/ClearPays.txt +++ b/test/gas_logs/fine_granularity/ClearPays.txt @@ -4,6 +4,6 @@ pay number per following payIdList used gas 1 12196 5 31370 10 55339 -25 128115 -50 252267 -100 511051 +25 128157 +50 252526 +100 512229 diff --git a/test/gas_logs/fine_granularity/DepositEthInBatch.txt b/test/gas_logs/fine_granularity/DepositEthInBatch.txt index 75d0915..5f1cb0f 100644 --- a/test/gas_logs/fine_granularity/DepositEthInBatch.txt +++ b/test/gas_logs/fine_granularity/DepositEthInBatch.txt @@ -3,7 +3,7 @@ batch size used gas 1 92848 5 382035 -10 743775 -25 1830363 -50 3647152 -75 5476464 +10 743813 +25 1830668 +50 3648846 +75 5481260 diff --git a/test/gas_logs/fine_granularity/IntendSettle-OneState.txt b/test/gas_logs/fine_granularity/IntendSettle-OneState.txt index 888c579..8b77c59 100644 --- a/test/gas_logs/fine_granularity/IntendSettle-OneState.txt +++ b/test/gas_logs/fine_granularity/IntendSettle-OneState.txt @@ -1,10 +1,10 @@ ********** Gas Measurement of intendSettle() one state with multi pays ********** pay number in head payIdList used gas -1 219426 -5 241280 -10 268574 -25 351962 -50 497143 -100 810884 -200 1540279 +1 219428 +5 241295 +10 268579 +25 352088 +50 497561 +100 812370 +200 1545265 diff --git a/test/utils/Fixtures.sol b/test/utils/Fixtures.sol index 73d1e0e..61cb82b 100644 --- a/test/utils/Fixtures.sol +++ b/test/utils/Fixtures.sol @@ -174,6 +174,7 @@ library Fixtures { uint256 resolveDeadline; uint256 resolveTimeout; address payResolver; + uint256 chainId; } function encConditionalPay(ConditionalPay memory _p) internal pure returns (bytes memory out) { @@ -187,6 +188,7 @@ library Fixtures { out = Proto.cat(out, Proto.uintField(6, _p.resolveDeadline)); out = Proto.cat(out, Proto.uintField(7, _p.resolveTimeout)); out = Proto.cat(out, Proto.addressField(8, _p.payResolver)); + out = Proto.cat(out, Proto.uintField(9, _p.chainId)); } /// @notice Encode `entity.CondPayResult { condPay: bytes, amount: uint256 }`. @@ -233,6 +235,8 @@ library Fixtures { uint256 openDeadline; uint256 disputeTimeout; uint256 msgValueReceiver; // peer index + uint256 chainId; + address ledgerAddress; } function encPaymentChannelInitializer(PaymentChannelInitializer memory _p) @@ -244,6 +248,8 @@ library Fixtures { out = Proto.cat(out, Proto.uintField(2, _p.openDeadline)); out = Proto.cat(out, Proto.uintField(3, _p.disputeTimeout)); out = Proto.cat(out, Proto.uintField(4, _p.msgValueReceiver)); + out = Proto.cat(out, Proto.uintField(5, _p.chainId)); + out = Proto.cat(out, Proto.addressField(6, _p.ledgerAddress)); } /// @notice Encode `entity.CooperativeSettleInfo`. diff --git a/test/utils/LedgerTestBase.t.sol b/test/utils/LedgerTestBase.t.sol index 83bc623..3547dc1 100644 --- a/test/utils/LedgerTestBase.t.sol +++ b/test/utils/LedgerTestBase.t.sol @@ -43,10 +43,10 @@ contract LedgerTestBase is BaseTest { /// @dev Derive the channel id that `openChannel` will produce for a given /// initializer-bytes payload. Mirrors `CelerWallet.create` id derivation: - /// `keccak256(wallet, ledger, keccak256(initializer))`. + /// `keccak256(chainid, wallet, ledger, keccak256(initializer))`. function _deriveChannelId(bytes memory _initializer) internal view returns (bytes32) { bytes32 nonce = keccak256(_initializer); - return keccak256(abi.encodePacked(address(celerWallet), address(celerLedger), nonce)); + return keccak256(abi.encodePacked(block.chainid, address(celerWallet), address(celerLedger), nonce)); } // ------------------------------------------------------------------------- @@ -67,7 +67,9 @@ contract LedgerTestBase is BaseTest { amounts: _amounts, openDeadline: _openDeadline, disputeTimeout: DISPUTE_TIMEOUT, - msgValueReceiver: _msgValueReceiver + msgValueReceiver: _msgValueReceiver, + chainId: block.chainid, + ledgerAddress: address(celerLedger) }); initializer = Fixtures.encPaymentChannelInitializer(init); @@ -89,7 +91,9 @@ contract LedgerTestBase is BaseTest { amounts: _amounts, openDeadline: _openDeadline, disputeTimeout: DISPUTE_TIMEOUT, - msgValueReceiver: 0 + msgValueReceiver: 0, + chainId: block.chainid, + ledgerAddress: address(celerLedger) }); initializer = Fixtures.encPaymentChannelInitializer(init); diff --git a/test/utils/Proto.t.sol b/test/utils/Proto.t.sol index 0045278..7a2ca39 100644 --- a/test/utils/Proto.t.sol +++ b/test/utils/Proto.t.sol @@ -68,7 +68,8 @@ contract ProtoTest is Test { maxAmount: 1000, resolveDeadline: 999999, resolveTimeout: 5, - payResolver: payResolver + payResolver: payResolver, + chainId: 31337 }); bytes memory encoded = Fixtures.encConditionalPay(pay); @@ -85,6 +86,7 @@ contract ProtoTest is Test { assertEq(decoded.resolveDeadline, 999999); assertEq(decoded.resolveTimeout, 5); assertEq(decoded.payResolver, payResolver); + assertEq(decoded.chainId, 31337); } function test_encConditionalPay_multipleConditions_roundTrips() public pure { @@ -102,7 +104,8 @@ contract ProtoTest is Test { maxAmount: 999, resolveDeadline: 100, resolveTimeout: 5, - payResolver: address(0xc) + payResolver: address(0xc), + chainId: 31337 }); bytes memory encoded = Fixtures.encConditionalPay(pay); @@ -164,7 +167,9 @@ contract ProtoTest is Test { amounts: [uint256(100), uint256(200)], openDeadline: 999999, disputeTimeout: 10, - msgValueReceiver: 0 + msgValueReceiver: 0, + chainId: 31337, + ledgerAddress: address(0xabcd) }); bytes memory encoded = Fixtures.encPaymentChannelInitializer(init); @@ -179,6 +184,8 @@ contract ProtoTest is Test { assertEq(decoded.openDeadline, 999999); assertEq(decoded.disputeTimeout, 10); assertEq(decoded.msgValueReceiver, 0); + assertEq(decoded.chainId, 31337); + assertEq(decoded.ledgerAddress, address(0xabcd)); } function test_encOpenChannelRequest_roundTrips() public pure { From 9cba6a5a3401d402595471caa7d743dbf88f8513 Mon Sep 17 00:00:00 2001 From: Xiaozhou Li Date: Sun, 3 May 2026 19:31:07 -0700 Subject: [PATCH 2/3] hardening: pay-resolver self-check, token-scope docs, pay_clear_deadline rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PayResolver: require pay.payResolver == address(this) so a pay can only be resolved by the resolver its body designates - Document plain-ERC20-only support in README, contracts.md, and openChannel NatSpec (fee-on-transfer / rebasing tokens are out of scope) - Rename SimplexPaymentChannel.last_pay_resolve_deadline → pay_clear_deadline for clearer semantics (= max(pay.resolveDeadline) + clearMargin); rename PayRegistry.getPayAmounts parameter to _maxResolveDeadline so the registry describes its own domain; add multi-segment confirmSettle regression tests --- README.md | 10 +++ docs/contracts.md | 15 +++- src/CelerLedger.sol | 12 +-- src/CelerLedgerMock.sol | 16 ++-- src/PayRegistry.sol | 8 +- src/PayResolver.sol | 3 +- src/lib/data/PbEntity.sol | 4 +- src/lib/data/proto/entity.proto | 13 +-- src/lib/interface/ICelerLedger.sol | 14 +-- src/lib/interface/IPayRegistry.sol | 13 +-- src/lib/interface/IPayResolver.sol | 6 +- src/lib/ledgerlib/LedgerChannel.sol | 8 +- src/lib/ledgerlib/LedgerOperation.sol | 26 +++--- src/lib/ledgerlib/LedgerStruct.sol | 2 +- test/CelerLedger.ETH.t.sol | 125 +++++++++++++++++++++++++- test/GasReport.t.sol | 2 +- test/PayRegistry.t.sol | 6 +- test/PayResolver.t.sol | 45 ++++++++++ test/gas_logs/CelerLedger-ETH.txt | 6 +- test/gas_logs/PayResolver.txt | 4 +- test/utils/Fixtures.sol | 4 +- test/utils/LedgerTestBase.t.sol | 4 +- test/utils/Proto.t.sol | 4 +- 23 files changed, 266 insertions(+), 84 deletions(-) diff --git a/README.md b/README.md index ecc865c..d256a5f 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,16 @@ gitignored. --- +## Supported tokens + +ETH and **plain ERC-20** only. Tokens whose `transferFrom` delivers anything +other than the requested amount — fee-on-transfer, deflationary, rebasing, +ERC-777 with hooks, etc. — are **not supported**: channel accounting credits +the requested amount, so any actual-vs-requested mismatch will desync the +wallet's internal balance from its real token holdings. + +--- + ## Dependencies | Package | Version | Purpose | diff --git a/docs/contracts.md b/docs/contracts.md index 0ca105a..a0e5f93 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -29,6 +29,11 @@ future versions) is an *operator* over individual wallets within it. The contrac deliberately minimal logic — it only knows how to deposit, withdraw, and transfer operatorship — to keep the audit surface small. +> **Supported tokens:** ETH and plain ERC-20 only. `depositERC20` credits the +> requested `_amount`, so tokens that deliver less than requested +> (fee-on-transfer, deflationary, rebasing, ERC-777 hooks) will desync this +> wallet's accounting from its real token balance. Use only standard ERC-20s. + ### Constructor ```solidity @@ -100,6 +105,11 @@ wrapper — the actual logic is split across five libraries under [`src/lib/ledgerlib/`](../src/lib/ledgerlib/) and attached via `using ... for ...`. See [Ledger libraries](#ledger-libraries) below. +> **Supported tokens:** ETH and plain ERC-20 only — see the same note under +> [CelerWallet](#celerwallet). Non-standard ERC-20s +> (fee-on-transfer / rebasing / ERC-777 hooks) will desync channel accounting +> from the wallet's real token balance. + ### Constructor ```solidity @@ -160,7 +170,7 @@ Balance limits are **enabled by default** post-deployment. Configure them via A wide set of getters: `getChannelStatus`, `getTokenContract`, `getTokenType`, `getTotalBalance`, `getBalanceMap`, `getStateSeqNumMap`, `getTransferOutMap`, -`getNextPayIdListHashMap`, `getLastPayResolveDeadlineMap`, `getPendingPayOutMap`, +`getNextPayIdListHashMap`, `getPayClearDeadlineMap`, `getPendingPayOutMap`, `getWithdrawIntent`, `getCooperativeWithdrawSeqNum`, `getSettleFinalizedTime`, `getDisputeTimeout`, `getMigratedTo`, `getChannelMigrationArgs`, `getPeersMigrationInfo`, `getChannelStatusNum`, `getEthPool`, `getPayRegistry`, @@ -215,6 +225,7 @@ constructor(address _registryAddr, address _virtResolverAddr) ### Resolution rules - A payment's `chain_id` must equal `block.chainid`. +- A payment's `pay_resolver` must equal the executing resolver's address. - A payment must be resolved before `pay.resolveDeadline` (Unix timestamp, seconds). - A result equal to the maximum-transfer amount **finalizes immediately** — no challenge window. @@ -255,7 +266,7 @@ No constructor (no state to initialize). | [`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 (verifies each pay's deadline ≤ a per-channel `lastPayResolveDeadline`). | +| [`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. | | `payInfoMap` (auto-getter) | Public mapping accessor. | diff --git a/src/CelerLedger.sol b/src/CelerLedger.sol index d840668..46de19c 100644 --- a/src/CelerLedger.sol +++ b/src/CelerLedger.sol @@ -391,18 +391,14 @@ contract CelerLedger is ICelerLedger, Ownable { } /** - * @notice Return lastPayResolveDeadline map of a duplex channel + * @notice Return payClearDeadline map of a duplex channel * @param _channelId ID of the channel to be viewed * @return peers' addresses - * @return lastPayResolveDeadlines of two simplex channels + * @return payClearDeadlines of two simplex channels */ - function getLastPayResolveDeadlineMap(bytes32 _channelId) - external - view - returns (address[2] memory, uint256[2] memory) - { + function getPayClearDeadlineMap(bytes32 _channelId) external view returns (address[2] memory, uint256[2] memory) { LedgerStruct.Channel storage c = ledger.channelMap[_channelId]; - return c.getLastPayResolveDeadlineMap(); + return c.getPayClearDeadlineMap(); } /** diff --git a/src/CelerLedgerMock.sol b/src/CelerLedgerMock.sol index 235e43c..fcad53c 100644 --- a/src/CelerLedgerMock.sol +++ b/src/CelerLedgerMock.sol @@ -202,7 +202,7 @@ contract CelerLedgerMock { uint256 _seqNum, uint256 _transferOut, bytes32 _nextPayIdListHash, - uint256 _lastPayResolveDeadline, + uint256 _payClearDeadline, uint256 _pendingPayOut ) external { LedgerStruct.Channel storage c = ledger.channelMap[_channelId]; @@ -212,7 +212,7 @@ contract CelerLedgerMock { state.seqNum = _seqNum; state.transferOut = _transferOut; state.nextPayIdListHash = _nextPayIdListHash; - state.lastPayResolveDeadline = _lastPayResolveDeadline; + state.payClearDeadline = _payClearDeadline; state.pendingPayOut = _pendingPayOut; _updateOverallStatesByIntendState(_channelId); @@ -431,18 +431,14 @@ contract CelerLedgerMock { } /** - * @notice Return lastPayResolveDeadline map of a duplex channel + * @notice Return payClearDeadline map of a duplex channel * @param _channelId ID of the channel to be viewed * @return peers' addresses - * @return lastPayResolveDeadlines of two simplex channels + * @return payClearDeadlines of two simplex channels */ - function getLastPayResolveDeadlineMap(bytes32 _channelId) - external - view - returns (address[2] memory, uint256[2] memory) - { + function getPayClearDeadlineMap(bytes32 _channelId) external view returns (address[2] memory, uint256[2] memory) { LedgerStruct.Channel storage c = ledger.channelMap[_channelId]; - return c.getLastPayResolveDeadlineMap(); + return c.getPayClearDeadlineMap(); } /** diff --git a/src/PayRegistry.sol b/src/PayRegistry.sol index 6ce65fb..a8ab701 100644 --- a/src/PayRegistry.sol +++ b/src/PayRegistry.sol @@ -103,7 +103,7 @@ contract PayRegistry is IPayRegistry { } /// @inheritdoc IPayRegistry - function getPayAmounts(bytes32[] calldata _payIds, uint256 _lastPayResolveDeadline) + function getPayAmounts(bytes32[] calldata _payIds, uint256 _maxResolveDeadline) external view returns (uint256[] memory) @@ -111,10 +111,10 @@ contract PayRegistry is IPayRegistry { uint256[] memory amounts = new uint256[](_payIds.length); for (uint256 i = 0; i < _payIds.length; i++) { if (payInfoMap[_payIds[i]].resolveDeadline == 0) { - // should pass last pay resolve deadline if never resolved - require(block.timestamp > _lastPayResolveDeadline, "Payment is not finalized"); + // unresolved pays are gated by the caller-supplied upper-bound deadline + require(block.timestamp > _maxResolveDeadline, "Payment is not finalized"); } else { - // should pass resolve deadline if resolved + // resolved pays are gated by their per-pay resolve deadline require(block.timestamp > payInfoMap[_payIds[i]].resolveDeadline, "Payment is not finalized"); } amounts[i] = payInfoMap[_payIds[i]].amount; diff --git a/src/PayResolver.sol b/src/PayResolver.sol index 3f2af19..79c5678 100644 --- a/src/PayResolver.sol +++ b/src/PayResolver.sol @@ -91,8 +91,9 @@ contract PayResolver is IPayResolver { * @param _amount payment amount to resolve */ function _resolvePayment(PbEntity.ConditionalPay memory _pay, bytes32 _payHash, uint256 _amount) internal { - // bind the signed pay to its intended chain + // bind the signed pay to its intended (chain, resolver) target require(_pay.chainId == block.chainid, "Wrong chain id for pay"); + require(_pay.payResolver == address(this), "Wrong resolver for pay"); uint256 nowTs = block.timestamp; require(nowTs <= _pay.resolveDeadline, "Passed pay resolve deadline in condPay msg"); diff --git a/src/lib/data/PbEntity.sol b/src/lib/data/PbEntity.sol index 0362e2c..160c6c9 100644 --- a/src/lib/data/PbEntity.sol +++ b/src/lib/data/PbEntity.sol @@ -173,7 +173,7 @@ library PbEntity { uint256 seqNum; // tag: 3 TokenTransfer transferToPeer; // tag: 4 PayIdList pendingPayIds; // tag: 5 - uint256 lastPayResolveDeadline; // tag: 6 + uint256 payClearDeadline; // tag: 6 uint256 totalPendingAmount; // tag: 7 } // end struct SimplexPaymentChannel @@ -200,7 +200,7 @@ library PbEntity { m.pendingPayIds = decPayIdList(buf.decBytes()); } else if (key == 48) { // tag 6 - m.lastPayResolveDeadline = buf.decVarint(); + m.payClearDeadline = buf.decVarint(); } else if (key == 58) { // tag 7 m.totalPendingAmount = buf.decUint256(); diff --git a/src/lib/data/proto/entity.proto b/src/lib/data/proto/entity.proto index bee5e6a..f29531d 100644 --- a/src/lib/data/proto/entity.proto +++ b/src/lib/data/proto/entity.proto @@ -52,11 +52,14 @@ message SimplexPaymentChannel { TokenTransfer transfer_to_peer = 4; // head of the idlist chain of all pending conditional pays. PayIdList pending_pay_ids = 5; - // The last resolve deadline of all pending conditional pays. - // confirmSettle must be called after all pending pays have been finalized, - // namely all pending pays have been resolved in the pay registry, - // or after the last_pay_resolve_deadline. - uint64 last_pay_resolve_deadline = 6 [(soltype) = "uint"]; + // Unix timestamp (seconds) after which confirmSettle becomes unconditionally + // eligible — i.e. max(pay.resolveDeadline) + clearMargin. The clearMargin + // must be large enough that, after all pays are resolved in PayRegistry, + // recipient peers have time to call clearPays for every pay-list segment + // before confirmSettle can close the channel. Off-chain producers MUST + // include the margin; a literal "last resolve deadline" would race with + // uncleared multi-segment pay lists. + uint64 pay_clear_deadline = 6 [(soltype) = "uint"]; // total amount of all pending pays. bytes total_pending_amount = 7 [(soltype) = "uint256"]; } diff --git a/src/lib/interface/ICelerLedger.sol b/src/lib/interface/ICelerLedger.sol index 6608d45..6fea15a 100644 --- a/src/lib/interface/ICelerLedger.sol +++ b/src/lib/interface/ICelerLedger.sol @@ -25,7 +25,9 @@ interface ICelerLedger { * `channelId = keccak256(chainid, walletAddr, ledgerAddr, keccak256(initializer))`, * and pulls the initial deposits. Native value is allowed via `msg.value`. * The initializer's `chain_id` and `ledger_address` must match this - * contract's execution domain. + * contract's execution domain. ERC-20 path assumes plain ERC-20 semantics + * (`balanceDelta == requested`); fee-on-transfer / rebasing / ERC-777 + * tokens are unsupported. * @param _openChannelRequest ABI-encoded `PbChain.OpenChannelRequest` message. */ function openChannel(bytes calldata _openChannelRequest) external payable; @@ -35,7 +37,8 @@ interface ICelerLedger { * @dev Anyone can deposit; total credited is `msg.value + _transferFromAmount`. * For ERC-20 channels, `msg.value` must be 0 and the depositor must have approved * this contract for `_transferFromAmount`. For ETH channels, `_transferFromAmount` - * is pulled from {IEthPool}. + * is pulled from {IEthPool}. ERC-20 path assumes plain ERC-20 semantics — + * fee-on-transfer / rebasing / ERC-777 tokens are unsupported. * @param _channelId Channel to credit. * @param _receiver Peer credited with the deposit. * @param _transferFromAmount Amount to pull via `transferFrom` (in addition to `msg.value`). @@ -264,11 +267,8 @@ interface ICelerLedger { /// @notice Per-peer next-list hashes for batched pay clearing during settlement. function getNextPayIdListHashMap(bytes32 _channelId) external view returns (address[2] memory, bytes32[2] memory); - /// @notice Per-peer latest pay-resolve deadlines used during settlement. - function getLastPayResolveDeadlineMap(bytes32 _channelId) - external - view - returns (address[2] memory, uint256[2] memory); + /// @notice Per-peer pay-clear deadlines (the threshold past which `confirmSettle` is unconditionally eligible). + function getPayClearDeadlineMap(bytes32 _channelId) external view returns (address[2] memory, uint256[2] memory); /// @notice Per-peer pending pay totals (locked amounts). function getPendingPayOutMap(bytes32 _channelId) external view returns (address[2] memory, uint256[2] memory); diff --git a/src/lib/interface/IPayRegistry.sol b/src/lib/interface/IPayRegistry.sol index fb4951c..18951e3 100644 --- a/src/lib/interface/IPayRegistry.sol +++ b/src/lib/interface/IPayRegistry.sol @@ -66,14 +66,17 @@ interface IPayRegistry { /** * @notice Bulk-read amounts for use during channel settlement. - * @dev Each pay's stored deadline must be less than or equal to - * `_lastPayResolveDeadline`; otherwise the call reverts. This guards against - * applying not-yet-finalized results during `intendSettle` / `confirmSettle`. + * @dev Each pay is either resolved (gated by its own `resolveDeadline`) or + * never resolved (gated by `_maxResolveDeadline`). Unresolved pays read + * back as 0 once `block.timestamp > _maxResolveDeadline`. * @param _payIds List of pay ids. - * @param _lastPayResolveDeadline Per-channel cap on the per-pay resolve deadline. + * @param _maxResolveDeadline Upper bound on per-pay resolve deadlines for + * the batch — callers pass any value ≥ max(pay.resolveDeadline). Channels + * typically pass `pay_clear_deadline` (= max resolve deadline + clear + * margin); the margin is a channel-clearance concern, transparent here. * @return Amounts indexed identically to `_payIds`. */ - function getPayAmounts(bytes32[] calldata _payIds, uint256 _lastPayResolveDeadline) + function getPayAmounts(bytes32[] calldata _payIds, uint256 _maxResolveDeadline) external view returns (uint256[] memory); diff --git a/src/lib/interface/IPayResolver.sol b/src/lib/interface/IPayResolver.sol index 36a0e9b..a5b5bc7 100644 --- a/src/lib/interface/IPayResolver.sol +++ b/src/lib/interface/IPayResolver.sol @@ -23,7 +23,8 @@ interface IPayResolver { * Hash-lock conditions are validated against `_resolvePayRequest.hashPreimages` * in the order they appear in `pay.conditions`. Virtual-contract conditions * must already be materialized via the VirtContractResolver. The pay's - * `chain_id` must equal `block.chainid`. + * `chain_id` must equal `block.chainid` and `pay_resolver` must equal + * `address(this)`. * @param _resolvePayRequest ABI-encoded `PbChain.ResolvePayByConditionsRequest`. */ function resolvePaymentByConditions(bytes calldata _resolvePayRequest) external; @@ -32,7 +33,8 @@ interface IPayResolver { * @notice Resolve a payment by submitting an off-chain result co-signed by * the payment's source and destination. * @dev The submitted amount must not exceed `pay.transferFunc.maxTransfer.receiver.amt`. - * The pay's `chain_id` must equal `block.chainid`. + * The pay's `chain_id` must equal `block.chainid` and `pay_resolver` must + * equal `address(this)`. * @param _vouchedPayResult ABI-encoded `PbEntity.VouchedCondPayResult`. */ function resolvePaymentByVouchedResult(bytes calldata _vouchedPayResult) external; diff --git a/src/lib/ledgerlib/LedgerChannel.sol b/src/lib/ledgerlib/LedgerChannel.sol index f7e6354..e8a03d1 100644 --- a/src/lib/ledgerlib/LedgerChannel.sol +++ b/src/lib/ledgerlib/LedgerChannel.sol @@ -218,12 +218,12 @@ library LedgerChannel { } /** - * @notice Return lastPayResolveDeadline map of a duplex channel + * @notice Return payClearDeadline map of a duplex channel * @param _c the channel to be viewed * @return peers' addresses - * @return lastPayResolveDeadlines of two simplex channels + * @return payClearDeadlines of two simplex channels */ - function getLastPayResolveDeadlineMap(LedgerStruct.Channel storage _c) + function getPayClearDeadlineMap(LedgerStruct.Channel storage _c) external view returns (address[2] memory, uint256[2] memory) @@ -231,7 +231,7 @@ library LedgerChannel { LedgerStruct.PeerProfile[2] memory peerProfiles = _c.peerProfiles; return ( [peerProfiles[0].peerAddr, peerProfiles[1].peerAddr], - [peerProfiles[0].state.lastPayResolveDeadline, peerProfiles[1].state.lastPayResolveDeadline] + [peerProfiles[0].state.payClearDeadline, peerProfiles[1].state.payClearDeadline] ); } diff --git a/src/lib/ledgerlib/LedgerOperation.sol b/src/lib/ledgerlib/LedgerOperation.sol index bd3c388..74d21cd 100644 --- a/src/lib/ledgerlib/LedgerOperation.sol +++ b/src/lib/ledgerlib/LedgerOperation.sol @@ -177,7 +177,7 @@ library LedgerOperation { LedgerStruct.PeerState storage state = c.peerProfiles[peerFromId].state; require(simplexState.seqNum > state.seqNum, "seqNum error"); - // no need to update nextPayIdListHash and lastPayResolveDeadline for snapshot purpose + // no need to update nextPayIdListHash and payClearDeadline for snapshot purpose state.seqNum = simplexState.seqNum; state.transferOut = simplexState.transferToPeer.receiver.amt; state.pendingPayOut = simplexState.totalPendingAmount; @@ -374,7 +374,7 @@ library LedgerOperation { state.seqNum = simplexState.seqNum; state.transferOut = simplexState.transferToPeer.receiver.amt; state.nextPayIdListHash = simplexState.pendingPayIds.nextListHash; - state.lastPayResolveDeadline = simplexState.lastPayResolveDeadline; + state.payClearDeadline = simplexState.payClearDeadline; // updating pendingPayOut is only needed when migrating ledger during settling phrase, which will // affect the withdraw limit after the migration. // if nextListHash is bytes32(0), state.pendingPayOut will be set as 0 by _clearPays() @@ -449,19 +449,17 @@ library LedgerOperation { require(nowTs >= c.settleFinalizedTime, "Settle is not finalized"); // require channel status of current intendSettle has been finalized, - // namely all payments have already been either cleared or expired - // Note: this lastPayResolveDeadline should use - // (the actual last resolve deadline of all pays + clearPays safe margin) - // to ensure that peers have enough time to clearPays before confirmSettle. - // However this only matters if there are multiple segments of the pending - // pay list, i.e. the nextPayIdListHash after intendSettle is not bytes32(0). - // TODO: add an additional clearSafeMargin param or change the semantics of - // lastPayResolveDeadline to also include clearPays safe margin and rename it. + // namely all payments have already been either cleared or expired. + // `payClearDeadline` is signed by peers as + // `max(pay.resolveDeadline) + clearMargin`, where the margin reserves + // wall-clock time after registry resolution for recipient peers to + // call clearPays for every pay-list segment before confirmSettle + // closes the channel. Single-segment lists are gated by + // `nextPayIdListHash == 0` directly. require( - (peerProfiles[0].state.nextPayIdListHash == bytes32(0) - || nowTs > peerProfiles[0].state.lastPayResolveDeadline) + (peerProfiles[0].state.nextPayIdListHash == bytes32(0) || nowTs > peerProfiles[0].state.payClearDeadline) && (peerProfiles[1].state.nextPayIdListHash == bytes32(0) - || nowTs > peerProfiles[1].state.lastPayResolveDeadline), + || nowTs > peerProfiles[1].state.payClearDeadline), "Payments are not finalized" ); @@ -703,7 +701,7 @@ library LedgerOperation { ) internal { LedgerStruct.Channel storage c = _self.channelMap[_channelId]; uint256[] memory outAmts = - _self.payRegistry.getPayAmounts(_payIdList.payIds, c.peerProfiles[_peerId].state.lastPayResolveDeadline); + _self.payRegistry.getPayAmounts(_payIdList.payIds, c.peerProfiles[_peerId].state.payClearDeadline); uint256 totalAmtOut = 0; for (uint256 i = 0; i < outAmts.length; i++) { diff --git a/src/lib/ledgerlib/LedgerStruct.sol b/src/lib/ledgerlib/LedgerStruct.sol index c781e10..cb04d04 100644 --- a/src/lib/ledgerlib/LedgerStruct.sol +++ b/src/lib/ledgerlib/LedgerStruct.sol @@ -38,7 +38,7 @@ library LedgerStruct { // Cumulative balance sent to the other peer; monotonically increasing. uint256 transferOut; bytes32 nextPayIdListHash; - uint256 lastPayResolveDeadline; + uint256 payClearDeadline; uint256 pendingPayOut; } diff --git a/test/CelerLedger.ETH.t.sol b/test/CelerLedger.ETH.t.sol index f68a87b..5d95203 100644 --- a/test/CelerLedger.ETH.t.sol +++ b/test/CelerLedger.ETH.t.sol @@ -608,7 +608,7 @@ contract CelerLedgerEthTest is LedgerTestBase { seqNum: 1, transferAmount: 10, pendingPayIds: bytes(""), - lastPayResolveDeadline: 0, + payClearDeadline: 0, totalPendingAmount: 0 }); bytes memory simplex = Fixtures.encSimplexPaymentChannel(s); @@ -890,6 +890,123 @@ contract CelerLedgerEthTest is LedgerTestBase { assertEq(transferOuts2[0], 35); } + /// @dev Build a co-signed peer0 simplex with a head pay-id list pointing + /// at a tail (multi-segment) and an explicit `payClearDeadline`. Used by + /// the multi-segment regression tests to keep stack depth manageable. + function _buildMultiSegmentSimplex( + bytes32 _channelId, + bytes32 _payId1, + bytes32 _payId2, + uint256 _clearDeadline, + uint256 _totalPending + ) internal view returns (bytes memory signedS0, bytes memory tailList) { + bytes32[] memory tailIds = new bytes32[](1); + tailIds[0] = _payId2; + tailList = Fixtures.encPayIdList(tailIds, bytes32(0)); + + bytes32[] memory headIds = new bytes32[](1); + headIds[0] = _payId1; + bytes memory headList = Fixtures.encPayIdList(headIds, keccak256(tailList)); + + Fixtures.SimplexState memory s0Spec = Fixtures.SimplexState({ + channelId: _channelId, + peerFrom: peer0, + seqNum: 1, + transferAmount: 0, + pendingPayIds: headList, + payClearDeadline: _clearDeadline, + totalPendingAmount: _totalPending + }); + bytes memory s0Bytes = Fixtures.encSimplexPaymentChannel(s0Spec); + bytes[] memory s0Sigs = SignUtil.coSign(peer0Pk, peer1Pk, s0Bytes); + signedS0 = Fixtures.encSignedSimplexState(s0Bytes, s0Sigs); + } + + function test_confirmSettle_multiSegment_unclearedTail_revertsBeforeClearDeadline() public { + // Documents the gap that pay_clear_deadline guards against: + // intendSettle clears only the head segment; until pay_clear_deadline + // expires, confirmSettle reverts so recipients have time to clearPays + // the tail segments. After the deadline, confirmSettle is unconditionally + // eligible — uncleared-tail pays are stranded. + celerLedger.disableBalanceLimits(); + bytes32 channelId = _openFundedEthChannel([uint256(200), 0]); + + uint256 clearDeadline = block.timestamp + 500; + (bytes memory signedS0,) = _buildMultiSegmentSimplex( + channelId, _resolveSingleHashLockPay(15, "p1", 1), _resolveSingleHashLockPay(20, "p2", 2), clearDeadline, 35 + ); + bytes memory array = _wrapStateArray(signedS0, _buildSignedSimplex(channelId, peer1, 1, 0)); + + // Advance past the head pays' resolveDeadline so intendSettle's + // implicit head-clear (via getPayAmounts) succeeds. + vm.warp(block.timestamp + 1); + vm.prank(peer0); + celerLedger.intendSettle(array); + + // Past dispute timeout but before pay_clear_deadline → reverts. + vm.warp(block.timestamp + DISPUTE_TIMEOUT + 1); + assertTrue(block.timestamp < clearDeadline); + vm.expectRevert(bytes("Payments are not finalized")); + celerLedger.confirmSettle(channelId); + + // Snapshot just before the post-deadline confirmSettle: head was cleared + // (transferOut[0] == 15) but the 20-unit tail is still outstanding + // (pendingPayOut[0] == 20). This is the stranded-funds setup. + (, uint256[2] memory transferOuts) = celerLedger.getTransferOutMap(channelId); + (, uint256[2] memory pendingOuts) = celerLedger.getPendingPayOutMap(channelId); + assertEq(transferOuts[0], 15, "only head cleared"); + assertEq(pendingOuts[0], 20, "tail still pending"); + uint256 peer0Before = peer0.balance; + uint256 peer1Before = peer1.balance; + + // After pay_clear_deadline → succeeds without the tail being cleared. + vm.warp(clearDeadline + 1); + celerLedger.confirmSettle(channelId); + assertEq(uint256(celerLedger.getChannelStatus(channelId)), uint256(LedgerStruct.ChannelStatus.Closed)); + + // Settlement strands the uncleared tail: peer1 only receives the head's 15 + // (not the full 35). The 20-unit tail effectively stays with peer0 since + // _validateSettleBalance settles from `transferOut`, ignoring `pendingPayOut`. + assertEq(peer1.balance, peer1Before + 15, "peer1 receives head amount only"); + assertEq(peer0.balance, peer0Before + 200 - 15, "peer0 keeps deposit minus head transferOut"); + } + + function test_confirmSettle_multiSegment_clearedTail_succeedsAfterDispute() public { + // Happy path: tail cleared before dispute window closes; confirmSettle + // is eligible right after dispute timeout, no need to wait for + // pay_clear_deadline because nextPayIdListHash[0] == 0. + celerLedger.disableBalanceLimits(); + bytes32 channelId = _openFundedEthChannel([uint256(200), 0]); + + bytes32 payId1 = _resolveSingleHashLockPay(15, "p1", 1); + bytes32 payId2 = _resolveSingleHashLockPay(20, "p2", 2); + + bytes32[] memory tailIds = new bytes32[](1); + tailIds[0] = payId2; + bytes memory tailList = Fixtures.encPayIdList(tailIds, bytes32(0)); + bytes32 tailHash = keccak256(tailList); + bytes32[] memory headIds = new bytes32[](1); + headIds[0] = payId1; + bytes memory headList = Fixtures.encPayIdList(headIds, tailHash); + + bytes memory signedS0 = _buildSimplexWithPayList(channelId, headList, 35); + bytes memory s1 = _buildSignedSimplex(channelId, peer1, 1, 0); + bytes memory array = _wrapStateArray(signedS0, s1); + + // Advance past the head pays' resolveDeadline so intendSettle's head-clear succeeds. + vm.warp(block.timestamp + 1); + vm.prank(peer0); + celerLedger.intendSettle(array); + celerLedger.clearPays(channelId, peer0, tailList); + + vm.warp(block.timestamp + DISPUTE_TIMEOUT + 1); + uint256 peer1Before = peer1.balance; + celerLedger.confirmSettle(channelId); + + // peer1 receives both pays = 35. + assertEq(peer1.balance, peer1Before + 35); + } + function test_clearPays_wrongStatus_reverts() public { celerLedger.disableBalanceLimits(); bytes32 channelId = _openZeroEthChannel(); @@ -1036,9 +1153,9 @@ contract CelerLedgerEthTest is LedgerTestBase { assertEq(hashes[1], bytes32(0)); } - function test_getLastPayResolveDeadlineMap_initiallyZero() public { + function test_getPayClearDeadlineMap_initiallyZero() public { bytes32 channelId = _openZeroEthChannel(); - (, uint256[2] memory deadlines) = celerLedger.getLastPayResolveDeadlineMap(channelId); + (, uint256[2] memory deadlines) = celerLedger.getPayClearDeadlineMap(channelId); assertEq(deadlines[0], 0); assertEq(deadlines[1], 0); } @@ -1135,7 +1252,7 @@ contract CelerLedgerEthTest is LedgerTestBase { seqNum: 1, transferAmount: 0, pendingPayIds: _payIdList, - lastPayResolveDeadline: block.timestamp + 1000, + payClearDeadline: block.timestamp + 1000, totalPendingAmount: _totalPending }); bytes memory simplex = Fixtures.encSimplexPaymentChannel(s); diff --git a/test/GasReport.t.sol b/test/GasReport.t.sol index 35cc509..c824fde 100644 --- a/test/GasReport.t.sol +++ b/test/GasReport.t.sol @@ -856,7 +856,7 @@ contract GasReport is LedgerTestBase { seqNum: _seqNum, transferAmount: 0, pendingPayIds: _payIdList, - lastPayResolveDeadline: block.timestamp + 1000, + payClearDeadline: block.timestamp + 1000, totalPendingAmount: _totalPending }); bytes memory simplex = Fixtures.encSimplexPaymentChannel(s); diff --git a/test/PayRegistry.t.sol b/test/PayRegistry.t.sol index 9515273..f9956cd 100644 --- a/test/PayRegistry.t.sol +++ b/test/PayRegistry.t.sol @@ -224,11 +224,11 @@ contract PayRegistryTest is Test { function test_getPayAmounts_unsetPay_passesIfChannelDeadlineExceeded() public { // No setPayInfo for payHash1 → resolveDeadline == 0 → falls through to the - // channel-level lastPayResolveDeadline check. + // channel-level payClearDeadline check. bytes32[] memory ids = new bytes32[](1); ids[0] = registry.calculatePayId(payHash1, setterA); - // Channel-level lastPayResolveDeadline is in the past → ok to read. + // Channel-level payClearDeadline is in the past → ok to read. vm.warp(block.timestamp + 10); uint256[] memory amounts = registry.getPayAmounts(ids, block.timestamp - 1); assertEq(amounts[0], 0); @@ -238,7 +238,7 @@ contract PayRegistryTest is Test { bytes32[] memory ids = new bytes32[](1); ids[0] = registry.calculatePayId(payHash1, setterA); - // Channel-level lastPayResolveDeadline is in the future → revert. + // Channel-level payClearDeadline is in the future → revert. vm.expectRevert(bytes("Payment is not finalized")); registry.getPayAmounts(ids, block.timestamp + 100); } diff --git a/test/PayResolver.t.sol b/test/PayResolver.t.sol index ae0cd27..9b58c6f 100644 --- a/test/PayResolver.t.sol +++ b/test/PayResolver.t.sol @@ -446,6 +446,51 @@ contract PayResolverTest is Test { payResolver.resolvePaymentByVouchedResult(_vouched(payBytes, 20)); } + // ------------------------------------------------------------------------- + // Wrong resolver (replay protection) + // ------------------------------------------------------------------------- + + /// @dev Build a hash-lock-only ConditionalPay with an explicit `payResolver` + /// override. Used by the wrong-resolver revert tests below. + function _buildPayWithResolver(uint256 _payTimestamp, address _resolver) internal view returns (bytes memory) { + Fixtures.Condition[] memory conds = new Fixtures.Condition[](1); + conds[0] = Fixtures.condHashLock(_hashLockTrue()); + + Fixtures.ConditionalPay memory pay = Fixtures.ConditionalPay({ + payTimestamp: _payTimestamp, + src: payerSrc, + dest: payerDest, + conditions: conds, + logicType: 0, // BOOLEAN_AND + maxAmount: 50, + resolveDeadline: RESOLVE_DEADLINE, + resolveTimeout: RESOLVE_TIMEOUT, + payResolver: _resolver, + chainId: block.chainid + }); + return Fixtures.encConditionalPay(pay); + } + + function test_resolveByConditions_wrongResolver_reverts() public { + // Pay designates a sibling PayResolver, submitted to the live resolver. + PayResolver siblingResolver = new PayResolver(address(payRegistry), address(virtResolver)); + bytes memory payBytes = _buildPayWithResolver(40, address(siblingResolver)); + + bytes[] memory preimages = new bytes[](1); + preimages[0] = TRUE_PREIMAGE; + vm.expectRevert(bytes("Wrong resolver for pay")); + payResolver.resolvePaymentByConditions(Fixtures.encResolvePayByConditionsRequest(payBytes, preimages)); + } + + function test_resolveByVouchedResult_wrongResolver_reverts() public { + // Same wrong-resolver pay submitted via the vouched-result path. + PayResolver siblingResolver = new PayResolver(address(payRegistry), address(virtResolver)); + bytes memory payBytes = _buildPayWithResolver(41, address(siblingResolver)); + + vm.expectRevert(bytes("Wrong resolver for pay")); + payResolver.resolvePaymentByVouchedResult(_vouched(payBytes, 20)); + } + // ------------------------------------------------------------------------- // Numeric logic // ------------------------------------------------------------------------- diff --git a/test/gas_logs/CelerLedger-ETH.txt b/test/gas_logs/CelerLedger-ETH.txt index d4e346a..4723512 100644 --- a/test/gas_logs/CelerLedger-ETH.txt +++ b/test/gas_logs/CelerLedger-ETH.txt @@ -5,13 +5,13 @@ VirtContractResolver Deploy Gas: 206156 EthPool Deploy Gas: 558320 PayRegistry Deploy Gas: 565565 CelerWallet Deploy Gas: 1148513 -PayResolver Deploy Gas: 1893745 -CelerLedger Deploy Gas: 1920284 +PayResolver Deploy Gas: 1910790 +CelerLedger Deploy Gas: 1920270 ***** Function Calls Gas Used ***** openChannel() with zero deposit: 301454 openChannel() using EthPool and msg.value: 412371 -setBalanceLimits(): 26123 +setBalanceLimits(): 26101 disableBalanceLimits(): 1688 enableBalanceLimits(): 32589 deposit() via msg.value: 78052 diff --git a/test/gas_logs/PayResolver.txt b/test/gas_logs/PayResolver.txt index 17cd683..5bff482 100644 --- a/test/gas_logs/PayResolver.txt +++ b/test/gas_logs/PayResolver.txt @@ -1,5 +1,5 @@ ********** Gas Measurement: PayResolver ********** ***** Function Calls Gas Used ***** -resolvePaymentByConditions(): 78445 -resolvePaymentByVouchedResult(): 96729 +resolvePaymentByConditions(): 78494 +resolvePaymentByVouchedResult(): 96778 diff --git a/test/utils/Fixtures.sol b/test/utils/Fixtures.sol index 61cb82b..7456a0d 100644 --- a/test/utils/Fixtures.sol +++ b/test/utils/Fixtures.sol @@ -83,7 +83,7 @@ library Fixtures { uint256 seqNum; uint256 transferAmount; bytes pendingPayIds; // pre-encoded PayIdList (use {encPayIdList}) - uint256 lastPayResolveDeadline; + uint256 payClearDeadline; uint256 totalPendingAmount; } @@ -100,7 +100,7 @@ library Fixtures { if (_s.pendingPayIds.length > 0) { out = Proto.cat(out, Proto.bytesField(5, _s.pendingPayIds)); } - out = Proto.cat(out, Proto.uintField(6, _s.lastPayResolveDeadline)); + out = Proto.cat(out, Proto.uintField(6, _s.payClearDeadline)); out = Proto.cat(out, Proto.uint256Field(7, _s.totalPendingAmount)); } diff --git a/test/utils/LedgerTestBase.t.sol b/test/utils/LedgerTestBase.t.sol index 3547dc1..0a2f8fd 100644 --- a/test/utils/LedgerTestBase.t.sol +++ b/test/utils/LedgerTestBase.t.sol @@ -183,7 +183,7 @@ contract LedgerTestBase is BaseTest { seqNum: _seqNum, transferAmount: _transferAmount, pendingPayIds: bytes(""), - lastPayResolveDeadline: 0, + payClearDeadline: 0, totalPendingAmount: 0 }); bytes memory simplex = Fixtures.encSimplexPaymentChannel(s); @@ -200,7 +200,7 @@ contract LedgerTestBase is BaseTest { seqNum: 0, transferAmount: 0, pendingPayIds: bytes(""), - lastPayResolveDeadline: 0, + payClearDeadline: 0, totalPendingAmount: 0 }); bytes memory simplex = Fixtures.encSimplexPaymentChannel(s); diff --git a/test/utils/Proto.t.sol b/test/utils/Proto.t.sol index 7a2ca39..d483969 100644 --- a/test/utils/Proto.t.sol +++ b/test/utils/Proto.t.sol @@ -144,7 +144,7 @@ contract ProtoTest is Test { seqNum: 7, transferAmount: 500, pendingPayIds: bytes(""), - lastPayResolveDeadline: 100, + payClearDeadline: 100, totalPendingAmount: 0 }); @@ -155,7 +155,7 @@ contract ProtoTest is Test { assertEq(decoded.peerFrom, address(0xaaaa)); assertEq(decoded.seqNum, 7); assertEq(decoded.transferToPeer.receiver.amt, 500); - assertEq(decoded.lastPayResolveDeadline, 100); + assertEq(decoded.payClearDeadline, 100); assertEq(decoded.totalPendingAmount, 0); } From 8669201af32617380eaccf273e06942a1c2de7ea Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 02:52:09 +0000 Subject: [PATCH 3/3] docs: fix walletId derivation to use creatorAddr (msg.sender) instead of operatorAddr Agent-Logs-Url: https://github.com/celer-network/agent-pay-contracts/sessions/e41b91f0-25b6-4807-84af-fd6617976e76 Co-authored-by: hhl42 <708956+hhl42@users.noreply.github.com> --- docs/contracts.md | 8 +++++--- src/CelerWallet.sol | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/contracts.md b/docs/contracts.md index a0e5f93..67de775 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -88,11 +88,13 @@ mapping(bytes32 => Wallet) private wallets; ### Wallet ID derivation ``` -walletId = keccak256(chainid, walletAddr, operatorAddr, nonce) +walletId = keccak256(chainid, walletAddr, creatorAddr, nonce) ``` -The `chainid` prefix prevents cross-chain channel-id collisions when wallet and -operator addresses coincide across chains. +`creatorAddr` is `msg.sender` at the time `CelerWallet.create` is called — typically +the `CelerLedger` contract, not the end-user operator. The `chainid` prefix prevents +cross-chain wallet-id collisions when the same creator and nonce are reused across +chains. --- diff --git a/src/CelerWallet.sol b/src/CelerWallet.sol index 49f1bc8..e07396a 100644 --- a/src/CelerWallet.sol +++ b/src/CelerWallet.sol @@ -61,7 +61,8 @@ contract CelerWallet is ICelerWallet, Pausable, Ownable { /** * @notice Create a new wallet - * @dev `walletId = keccak256(chainid, walletAddr, operatorAddr, nonce)`. + * @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