From 674870110f531f2e5c90f5412f1abdd16c12ef34 Mon Sep 17 00:00:00 2001 From: Alex Sedighi Date: Thu, 21 May 2026 11:37:01 +1200 Subject: [PATCH 1/5] feat(envelope): security hardening for production deployment Implements findings from final security review: H-1: Balance-delta measurement for ERC-20 deposits (fee-on-transfer safety) - _pullTokensViaApprovalFrom measures balanceOf delta, not requested amount - Batch functions use actual received / count for per-link amounts - Raffle links revert with InsufficientTokensReceived for FOT tokens H-2: Make mfaAuthorizer mutable for key rotation - Removed immutable, added setMfaAuthorizer(address) onlyOwner - Emits MfaAuthorizerUpdated event M-1: Guard _isMfaSignatureValid and _verifyMfaSignature against address(0) - claimWithMFA reverts with MfaAuthorizerIsZero when authorizer is unset - Paymaster validation returns false instead of matching address(0) M-2: Reject unbound links in claimAsBoundRecipient - Reverts with LinkNotRecipientBound if link.parties.recipient == address(0) M-3: Reject recipientAddress == address(0) in all claim paths - _executeClaim reverts with ZeroRecipientAddress - _isValidClaim returns false for zero recipient (paymaster path) M-4: Fee-authorization replay protection - usedFeeAuthorizations mapping tracks consumed signature hashes - Reverts with FeeAuthorizationAlreadyUsed on replay L-1: Remove dead DOMAIN_SEPARATOR state and EIP712Domain struct/hash function L-8: Explicit InvalidContractType revert in _pullUniformBatchAssets L-9: Remove constructor debug MessageEvent emit Also adds EnvelopeSecurity.t.sol with 13 targeted tests covering all findings, updates documentation with security properties section, and fixes spellcheck. --- .cspell.json | 1 + src/envelope/EnvelopeLinks.sol | 97 +++--- src/envelope/doc/EnvelopeLinks.md | 46 ++- src/envelope/doc/README.md | 6 +- test/envelope/EnvelopeSecurity.t.sol | 290 ++++++++++++++++++ test/envelope/SigWithdraw.t.sol | 69 +++-- .../envelope/mocks/FeeOnTransferERC20Mock.sol | 24 ++ 7 files changed, 459 insertions(+), 74 deletions(-) create mode 100644 test/envelope/EnvelopeSecurity.t.sol create mode 100644 test/envelope/mocks/FeeOnTransferERC20Mock.sol diff --git a/.cspell.json b/.cspell.json index 0d2338cb..f4e78a4c 100644 --- a/.cspell.json +++ b/.cspell.json @@ -16,6 +16,7 @@ "src/swarms/doc/iso3166-2" ], "ignoreWords": [ + "AMPL", "NODL", "Nodle", "depin", diff --git a/src/envelope/EnvelopeLinks.sol b/src/envelope/EnvelopeLinks.sol index a8e89b06..d855a837 100644 --- a/src/envelope/EnvelopeLinks.sol +++ b/src/envelope/EnvelopeLinks.sol @@ -44,6 +44,11 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow error EthNotAcceptedForNonEthLink(); error Erc721BatchNotSupported(); error UnsupportedRaffleContractType(); + error InsufficientTokensReceived(); + error ZeroRecipientAddress(); + error LinkNotRecipientBound(); + error FeeAuthorizationAlreadyUsed(); + error MfaAuthorizerIsZero(); // ── Data Structures ────────────────────────────────────────────────────────── @@ -111,21 +116,9 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow bytes32 public constant OPEN_CLAIM_MODE = 0x0000000000000000000000000000000000000000000000000000000000000000; // default. Any address can trigger the withdrawal function bytes32 public constant BOUND_CLAIM_MODE = 0x2bb5bef2b248d3edba501ad918c3ab524cce2aea54d4c914414e1c4401dc4ff4; // keccak256("only recipient") - only the signed recipient can trigger the withdrawal function - bytes32 public DOMAIN_SEPARATOR; // initialized in the constructor - - bytes32 public constant EIP712DOMAIN_TYPEHASH = - keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); - - /// @notice Address authorized to issue MFA signatures gating claimWithMFA calls. - /// @dev Configurable per deployment. Address(0) disables MFA — claimWithMFA will revert. - address public immutable mfaAuthorizer; - - struct EIP712Domain { - string name; - string version; - uint256 chainId; - address verifyingContract; - } + /// @notice Address authorized to issue MFA signatures gating claimWithMFA calls and fee authorizations. + /// @dev Rotatable by owner. Address(0) disables MFA — claimWithMFA will revert. + address public mfaAuthorizer; Link[] internal links; // array of links @@ -135,6 +128,9 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow /// @notice Accumulated fees per token address (address(0) for ETH; feeToken for link-creation fees). mapping(address => uint256) public accumulatedFees; + /// @notice Tracks consumed fee authorizations to prevent replay. + mapping(bytes32 => bool) public usedFeeAuthorizations; + // events event LinkCreated(uint256 indexed _index, uint8 indexed _contractType, uint256 _amount, address indexed _creator); event LinkRedeemed( @@ -142,30 +138,14 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow ); event FeeCollected(uint256 indexed _index, address indexed tokenAddress, uint256 serviceFee, uint256 gaslessFee); event FeesWithdrawn(address indexed tokenAddress, uint256 amount); - event MessageEvent(string message); + event MfaAuthorizerUpdated(address indexed oldAuthorizer, address indexed newAuthorizer); /// @param _mfaAuthorizer address authorized to sign backend fee and MFA approvals (use address(0) to disable). /// @param _owner initial owner of the contract (receives accumulated fees). /// @param _feeToken ERC-20 token used for fees; address(0) disables non-zero fee authorizations. constructor(address _mfaAuthorizer, address _owner, address _feeToken) Ownable(_owner) { - emit MessageEvent("Hello World, have a nutty day!"); mfaAuthorizer = _mfaAuthorizer; feeToken = IERC20(_feeToken); - DOMAIN_SEPARATOR = hash( - EIP712Domain({name: "Envelope", version: "4.4", chainId: block.chainid, verifyingContract: address(this)}) - ); - } - - function hash(EIP712Domain memory eip712Domain) internal pure returns (bytes32) { - return keccak256( - abi.encode( - EIP712DOMAIN_TYPEHASH, - keccak256(bytes(eip712Domain.name)), - keccak256(bytes(eip712Domain.version)), - eip712Domain.chainId, - eip712Domain.verifyingContract - ) - ); } /** @@ -322,14 +302,15 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow address[] calldata _claimKeys ) external payable nonReentrant returns (uint256[] memory) { uint256 totalAmount = _amount * _claimKeys.length; - _pullUniformBatchAssets(msg.sender, _tokenAddress, _contractType, totalAmount, _tokenId); + uint256 actualTotal = _pullUniformBatchAssets(msg.sender, _tokenAddress, _contractType, totalAmount, _tokenId); + uint256 perLinkAmount = _claimKeys.length > 0 ? actualTotal / _claimKeys.length : 0; uint256[] memory linkIndexes = new uint256[](_claimKeys.length); for (uint256 i = 0; i < _claimKeys.length; ++i) { linkIndexes[i] = _storeLink( _tokenAddress, _contractType, - _amount, + perLinkAmount, _tokenId, _claimKeys[i], msg.sender, @@ -353,13 +334,14 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow address[] calldata _claimKeys ) external payable nonReentrant { uint256 totalAmount = _amount * _claimKeys.length; - _pullUniformBatchAssets(msg.sender, _tokenAddress, _contractType, totalAmount, _tokenId); + uint256 actualTotal = _pullUniformBatchAssets(msg.sender, _tokenAddress, _contractType, totalAmount, _tokenId); + uint256 perLinkAmount = _claimKeys.length > 0 ? actualTotal / _claimKeys.length : 0; for (uint256 i = 0; i < _claimKeys.length; ++i) { _storeLink( _tokenAddress, _contractType, - _amount, + perLinkAmount, _tokenId, _claimKeys[i], msg.sender, @@ -495,6 +477,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow returns (bool) { if (_recipientAddress != msg.sender) revert NotTheRecipient(); + if (_index >= links.length) revert LinkIndexOutOfBounds(); + if (links[_index].parties.recipient == address(0)) revert LinkNotRecipientBound(); return _executeClaim(_index, _recipientAddress, BOUND_CLAIM_MODE, _signature, false); } @@ -547,6 +531,15 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow // Owner Functions // ══════════════════════════════════════════════════════════════════════════════ + /** + * @notice Update the MFA authorizer address. Only callable by owner. + * @param _newAuthorizer new MFA signer address (address(0) disables MFA). + */ + function setMfaAuthorizer(address _newAuthorizer) external onlyOwner { + emit MfaAuthorizerUpdated(mfaAuthorizer, _newAuthorizer); + mfaAuthorizer = _newAuthorizer; + } + /** * @notice Withdraw accumulated fees for a given token. Only callable by owner. * @param _tokenAddress token to withdraw fees for (address(0) for ETH) @@ -665,6 +658,7 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow uint256 _deadline, bytes memory _MFASignature ) internal view { + if (mfaAuthorizer == address(0)) revert MfaAuthorizerIsZero(); // deadline == 0 means no expiry if (_deadline != 0 && block.timestamp > _deadline) revert MfaSignatureExpired(); @@ -682,6 +676,7 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow view returns (bool) { + if (mfaAuthorizer == address(0)) return false; if (_deadline != 0 && block.timestamp > _deadline) return false; bytes32 digest = MessageHashUtils.toEthSignedMessageHash( @@ -694,7 +689,6 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow function _verifyFeeAuthorization(LinkRequest calldata _request, FeeAuthorization calldata _feeAuthorization) internal - view { uint256 totalFee = _feeAuthorization.serviceFee + _feeAuthorization.gaslessFee; if (totalFee == 0 && !_feeAuthorization.gaslessSponsored && _feeAuthorization.signature.length == 0) return; @@ -704,6 +698,12 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow } bytes32 digest = _feeAuthorizationDigest(_request, _feeAuthorization, msg.sender); + + // Replay protection: mark this authorization as consumed. + bytes32 authHash = keccak256(_feeAuthorization.signature); + if (usedFeeAuthorizations[authHash]) revert FeeAuthorizationAlreadyUsed(); + usedFeeAuthorizations[authHash] = true; + address authorizationSigner = getSigner(digest, _feeAuthorization.signature); if (authorizationSigner != mfaAuthorizer) revert WrongFeeAuthorizationSignature(); } @@ -863,6 +863,7 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow bytes memory _signature, bool _authorized ) internal view returns (bool) { + if (_recipientAddress == address(0)) return false; Link memory deposit = links[_index]; if (deposit.status.redeemed) return false; if (deposit.status.requiresMFA && !_authorized) return false; @@ -975,21 +976,27 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow uint8 _contractType, uint256 _totalAmount, uint256 _tokenId - ) internal { + ) internal returns (uint256) { if (_contractType == 0) { if (msg.value != _totalAmount) revert InvalidTotalEtherSent(); - return; + return _totalAmount; } if (msg.value != 0) revert EthNotAcceptedForNonEthLink(); if (_contractType == 1) { - if (_totalAmount > 0) IERC20(_tokenAddress).safeTransferFrom(_from, address(this), _totalAmount); + if (_totalAmount > 0) { + uint256 balanceBefore = IERC20(_tokenAddress).balanceOf(address(this)); + IERC20(_tokenAddress).safeTransferFrom(_from, address(this), _totalAmount); + return IERC20(_tokenAddress).balanceOf(address(this)) - balanceBefore; + } + return 0; } else if (_contractType == 2) { revert Erc721BatchNotSupported(); } else if (_contractType == 3) { if (_totalAmount > 0) { IERC1155(_tokenAddress).safeTransferFrom(_from, address(this), _tokenId, _totalAmount, ""); } + return _totalAmount; } else { revert InvalidContractType(); } @@ -1015,7 +1022,12 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow if (msg.value != totalAmount) revert InvalidTotalEtherSent(); } else { if (msg.value != 0) revert EthNotAcceptedForNonEthLink(); - if (totalAmount > 0) IERC20(_tokenAddress).safeTransferFrom(msg.sender, address(this), totalAmount); + if (totalAmount > 0) { + uint256 balanceBefore = IERC20(_tokenAddress).balanceOf(address(this)); + IERC20(_tokenAddress).safeTransferFrom(msg.sender, address(this), totalAmount); + uint256 actualReceived = IERC20(_tokenAddress).balanceOf(address(this)) - balanceBefore; + if (actualReceived < totalAmount) revert InsufficientTokensReceived(); + } } uint256[] memory linkIndexes = new uint256[](_amounts.length); @@ -1058,7 +1070,9 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow if (_contractType == 0) { if (_amount != _ethAmount) revert WrongEthAmount(); } else if (_contractType == 1) { + uint256 balanceBefore = IERC20(_tokenAddress).balanceOf(address(this)); IERC20(_tokenAddress).safeTransferFrom(_from, address(this), _amount); + _amount = IERC20(_tokenAddress).balanceOf(address(this)) - balanceBefore; } else if (_contractType == 2) { if (_amount != 1) revert Erc721AmountMustBeOne(); IERC721(_tokenAddress).safeTransferFrom(_from, address(this), _tokenId, "Internal transfer"); @@ -1076,6 +1090,7 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow bytes memory _signature, bool _authorized ) internal returns (bool) { + if (_recipientAddress == address(0)) revert ZeroRecipientAddress(); if (_index >= links.length) revert LinkIndexOutOfBounds(); Link memory link = links[_index]; if (link.status.redeemed) revert LinkAlreadyRedeemed(); diff --git a/src/envelope/doc/EnvelopeLinks.md b/src/envelope/doc/EnvelopeLinks.md index 23d38af2..96e79c30 100644 --- a/src/envelope/doc/EnvelopeLinks.md +++ b/src/envelope/doc/EnvelopeLinks.md @@ -200,11 +200,51 @@ constructor(address mfaAuthorizer, address owner, address feeToken) | Param | Purpose | | --------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `mfaAuthorizer` | Backend signer for MFA claim approvals and link-creation-time fee authorizations. `address(0)` disables non-zero fee authorizations and makes MFA withdrawals fail. | -| `owner` | Owns the vault and can withdraw accumulated fees. | +| `mfaAuthorizer` | Backend signer for MFA claim approvals and link-creation-time fee authorizations. `address(0)` disables non-zero fee authorizations and makes MFA withdrawals fail. Rotatable by owner via `setMfaAuthorizer`. | +| `owner` | Owns the vault, can withdraw accumulated fees, and rotate the `mfaAuthorizer`. | | `feeToken` | ERC-20 used for Nodle service and gasless sponsorship fees, for example NODL. `address(0)` permits only zero-fee deposits. | -The constructor also sets the EIP-712 domain separator used by the vault-side validation helpers. +## Owner Functions + +| Function | Purpose | +| ----------------------------------- | ---------------------------------------------------------------------------------------- | +| `setMfaAuthorizer(address)` | Rotate the MFA/fee-authorization signer. Invalidates all in-flight signatures from the old key. | +| `withdrawFees(address tokenAddress)`| Withdraw accumulated service and gasless fees for a given token. | + +## Security Properties + +### Fee-On-Transfer Token Safety + +For ERC-20 deposits, the vault measures the actual `balanceOf` delta rather than trusting the requested `amount`. This prevents insolvency when fee-on-transfer or rebasing tokens are deposited. The recorded `link.asset.amount` reflects what the vault actually received and can transfer back. + +For raffle-style links (which have per-link variable amounts), a fee-on-transfer token will cause the deposit to revert because the vault asserts the received total matches the requested total. + +### Fee Authorization Replay Protection + +Each `FeeAuthorization` signature can only be used once. The vault tracks consumed authorizations via `usedFeeAuthorizations[keccak256(signature)]` and reverts with `FeeAuthorizationAlreadyUsed` on replay attempts. + +### Recipient Validation + +- Claims to `address(0)` are rejected with `ZeroRecipientAddress`. +- `claimAsBoundRecipient` reverts with `LinkNotRecipientBound` if the link has no stored recipient, preventing misuse of the bound-mode signature on open links. + +### MFA Authorizer Rotation + +The `mfaAuthorizer` is mutable (not immutable). In case of backend key compromise, the owner can rotate the signer immediately via `setMfaAuthorizer`. All in-flight MFA and fee authorization signatures from the old key become invalid after rotation. + +### Unsupported Token Types + +The following token types are **not supported** and should not be deposited: + +- **Rebasing tokens** (e.g., stETH, AMPL): balance changes between deposit and claim may cause under/overpayment. +- **Tokens with transfer hooks that modify balances** beyond a simple fee deduction. +- **ERC-777 tokens**: the vault does not implement `tokensReceived` and relies on `nonReentrant` guards. + +ERC-20 tokens that charge a fixed transfer fee (e.g., USDT on some chains) are supported — the vault records the actual received amount. + +### View-Only Functions (Off-Chain Only) + +`getLinkIndexesCreatedBy` and `getAllLinkIndexes` iterate over the entire links array. These are O(n) and intended for off-chain use only. On-chain callers will encounter out-of-gas for large link counts. ## Deposit Model diff --git a/src/envelope/doc/README.md b/src/envelope/doc/README.md index fbbaf228..fe36f615 100644 --- a/src/envelope/doc/README.md +++ b/src/envelope/doc/README.md @@ -78,9 +78,9 @@ The vault no longer contains an internal paymaster callback, and the EIP-3009 ga ## Deploy -| Script | Purpose | -| ---------------------------------- | ----------------------------------------------------------- | -| `hardhat-deploy/DeployEnvelope.ts` | Deploys `EnvelopeLinks` and optionally `EnvelopePaymaster`. | +| Script | Purpose | +| ----------------------------------- | ------------------------------------------------------------------------------------------- | +| `hardhat-deploy/DeployEnvelope.ts` | Deploys `EnvelopeLinks` and optionally `EnvelopePaymaster`. | | `script/DeployEnvelopeZkSync.s.sol` | Forge deployment script for `EnvelopeLinks` and optional `EnvelopePaymaster` on ZkSync Era. | Important environment variables: diff --git a/test/envelope/EnvelopeSecurity.t.sol b/test/envelope/EnvelopeSecurity.t.sol new file mode 100644 index 00000000..be5fe76f --- /dev/null +++ b/test/envelope/EnvelopeSecurity.t.sol @@ -0,0 +1,290 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.26; + +// Tests for security hardening findings: +// H-1 — Balance-delta measurement for fee-on-transfer tokens +// H-2 — Mutable mfaAuthorizer (key rotation) +// M-1 — Guard _isMfaSignatureValid against address(0) +// M-2 — Reject unbound links in claimAsBoundRecipient +// M-3 — Reject recipientAddress == address(0) in claims +// M-4 — Fee-authorization replay protection + +import {Test} from "forge-std/Test.sol"; +import {EnvelopeLinks} from "../../src/envelope/EnvelopeLinks.sol"; +import {EnvelopeFeeAuthTestUtils} from "./EnvelopeFeeAuthTestUtils.sol"; +import {ERC20Mock} from "./mocks/ERC20Mock.sol"; +import {FeeOnTransferERC20Mock} from "./mocks/FeeOnTransferERC20Mock.sol"; +import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; + +contract EnvelopeSecurityTest is Test { + EnvelopeLinks public vault; + EnvelopeLinks public mfaVault; + ERC20Mock public feeToken; + FeeOnTransferERC20Mock public fotToken; + + uint256 constant LINK_PRIV = 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; + uint256 constant MFA_PRIV = uint256(keccak256("security-test-mfa-signer")); + address linkPubKey; + address mfaSigner; + + address constant ALICE = address(0xA11CE); + address constant BOB = address(0xB0B); + + function setUp() public { + linkPubKey = vm.addr(LINK_PRIV); + mfaSigner = vm.addr(MFA_PRIV); + + feeToken = new ERC20Mock(); + fotToken = new FeeOnTransferERC20Mock(); + + vault = new EnvelopeLinks(address(0), address(this), address(0)); + mfaVault = new EnvelopeLinks(mfaSigner, address(this), address(feeToken)); + } + + receive() external payable {} + + // ══════════════════════════════════════════════════════════════════════════════ + // H-1: Balance-delta measurement for fee-on-transfer tokens + // ══════════════════════════════════════════════════════════════════════════════ + + function test_H1_feeOnTransferRecordsActualAmount() public { + fotToken.mint(address(this), 10000); + fotToken.approve(address(vault), 10000); + + // Deposit 1000 tokens; FOT takes 1% → vault receives 990. + uint256 idx = vault.createLink(address(fotToken), 1, 1000, 0, linkPubKey); + EnvelopeLinks.LinkAsset memory asset = vault.getLinkAsset(idx); + // The stored amount must reflect the actual received amount (990), not requested (1000). + assertEq(asset.amount, 990, "Should store balance-delta, not requested amount"); + } + + function test_H1_batchFeeOnTransferRecordsActualPerLinkAmount() public { + fotToken.mint(address(this), 100000); + fotToken.approve(address(vault), 100000); + + address[] memory keys = new address[](5); + for (uint256 i = 0; i < 5; i++) keys[i] = linkPubKey; + + uint256[] memory indexes = vault.createLinks(address(fotToken), 1, 1000, 0, keys); + + // Each link: requested 1000 * 5 = 5000, received 4950, per-link = 990. + for (uint256 i = 0; i < indexes.length; i++) { + EnvelopeLinks.LinkAsset memory asset = vault.getLinkAsset(indexes[i]); + assertEq(asset.amount, 990, "Batch per-link should use balance delta"); + } + } + + function test_H1_raffleFeeOnTransferReverts() public { + fotToken.mint(address(this), 100000); + fotToken.approve(address(vault), 100000); + + uint256[] memory amounts = new uint256[](3); + amounts[0] = 1000; + amounts[1] = 2000; + amounts[2] = 3000; + + // Raffle links with FOT token should revert because received < expected. + vm.expectRevert(EnvelopeLinks.InsufficientTokensReceived.selector); + vault.createRaffleLinks(address(fotToken), 1, amounts, linkPubKey); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // H-2: Mutable mfaAuthorizer (key rotation) + // ══════════════════════════════════════════════════════════════════════════════ + + function test_H2_ownerCanRotateMfaAuthorizer() public { + assertEq(mfaVault.mfaAuthorizer(), mfaSigner); + + address newSigner = address(0x1234); + mfaVault.setMfaAuthorizer(newSigner); + assertEq(mfaVault.mfaAuthorizer(), newSigner); + } + + function test_H2_nonOwnerCannotRotateMfaAuthorizer() public { + vm.prank(ALICE); + vm.expectRevert(); + mfaVault.setMfaAuthorizer(address(0x9999)); + } + + function test_H2_rotationInvalidatesOldSignatures() public { + // Create an MFA-gated link. + uint256 idx = mfaVault.createMFALink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + + // Sign MFA with the old key. + bytes memory mfaSig = _signMfa(address(mfaVault), idx, ALICE, 0, MFA_PRIV); + bytes memory claimSig = _signOpen(address(mfaVault), idx, ALICE); + + // Rotate key. + uint256 newMfaPriv = uint256(keccak256("new-mfa-key")); + mfaVault.setMfaAuthorizer(vm.addr(newMfaPriv)); + + // Old MFA signature should now fail. + vm.expectRevert(EnvelopeLinks.WrongMfaSignature.selector); + mfaVault.claimWithMFA(idx, ALICE, claimSig, mfaSig, 0); + + // New signature works. + bytes memory newMfaSig = _signMfa(address(mfaVault), idx, ALICE, 0, newMfaPriv); + mfaVault.claimWithMFA(idx, ALICE, claimSig, newMfaSig, 0); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // M-1: Guard against mfaAuthorizer == address(0) + // ══════════════════════════════════════════════════════════════════════════════ + + function test_M1_claimWithMfaRevertsWhenAuthorizerIsZero() public { + // vault has mfaAuthorizer == address(0). + uint256 idx = vault.createMFALink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + bytes memory sig = _signOpen(address(vault), idx, ALICE); + + vm.expectRevert(EnvelopeLinks.MfaAuthorizerIsZero.selector); + vault.claimWithMFA(idx, ALICE, sig, hex"", 0); + } + + function test_M1_isValidGaslessOperationReturnsFalseWhenAuthorizerZero() public { + uint256 idx = vault.createMFALink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + bytes memory sig = _signOpen(address(vault), idx, ALICE); + bytes memory mfaSig = hex"00"; + + bytes memory callData = + abi.encodeCall(EnvelopeLinks.claimWithMFA, (idx, ALICE, sig, mfaSig, 0)); + + bool valid = vault.isValidGaslessOperation(ALICE, callData); + assertFalse(valid, "Should reject when mfaAuthorizer is zero"); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // M-2: Reject unbound links in claimAsBoundRecipient + // ══════════════════════════════════════════════════════════════════════════════ + + function test_M2_claimAsBoundRecipientRevertsOnUnboundLink() public { + uint256 idx = vault.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + bytes memory sig = _signBound(address(vault), idx, ALICE); + + vm.prank(ALICE); + vm.expectRevert(EnvelopeLinks.LinkNotRecipientBound.selector); + vault.claimAsBoundRecipient(idx, ALICE, sig); + } + + function test_M2_claimAsBoundRecipientWorksOnBoundLink() public { + uint256 idx = vault.createCustomLink{value: 1 ether}( + address(0), 0, 1 ether, 0, linkPubKey, address(this), false, ALICE, 0 + ); + bytes memory sig = _signBound(address(vault), idx, ALICE); + + vm.prank(ALICE); + vault.claimAsBoundRecipient(idx, ALICE, sig); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // M-3: Reject recipientAddress == address(0) in claims + // ══════════════════════════════════════════════════════════════════════════════ + + function test_M3_claimRevertsWithZeroRecipient() public { + uint256 idx = vault.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + bytes memory sig = _signOpen(address(vault), idx, address(0)); + + vm.expectRevert(EnvelopeLinks.ZeroRecipientAddress.selector); + vault.claim(idx, address(0), sig); + } + + function test_M3_isValidGaslessReturnsFalseForZeroRecipient() public { + uint256 idx = vault.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + bytes memory sig = _signOpen(address(vault), idx, address(0)); + + bytes memory callData = abi.encodeCall(EnvelopeLinks.claim, (idx, address(0), sig)); + bool valid = vault.isValidGaslessOperation(address(0), callData); + assertFalse(valid, "Should reject zero recipient in gasless validation"); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // M-4: Fee-authorization replay protection + // ══════════════════════════════════════════════════════════════════════════════ + + function test_M4_feeAuthorizationCannotBeReused() public { + feeToken.mint(address(this), 1 ether); + feeToken.approve(address(mfaVault), 1 ether); + + EnvelopeLinks.LinkRequest memory request = EnvelopeLinks.LinkRequest({ + tokenAddress: address(0), + contractType: 0, + amount: 0.1 ether, + tokenId: 0, + claimKey: linkPubKey, + onBehalfOf: address(this), + withMFA: false, + recipient: address(0), + reclaimableAfter: 0 + }); + + uint256 serviceFee = 100; + uint256 gaslessFee = 0; + uint256 deadline = block.timestamp + 1 hours; + + bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( + mfaVault.ENVELOPE_SALT(), + address(mfaVault), + request, + address(this), + serviceFee, + gaslessFee, + false, + deadline + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(MFA_PRIV, digest); + bytes memory sig = abi.encodePacked(r, s, v); + + EnvelopeLinks.FeeAuthorization memory feeAuth = EnvelopeLinks.FeeAuthorization({ + serviceFee: serviceFee, + gaslessFee: gaslessFee, + gaslessSponsored: false, + deadline: deadline, + signature: sig + }); + + // First use succeeds. + mfaVault.createLinkWithFees{value: 0.1 ether}(request, feeAuth); + + // Second use with the same authorization reverts. + vm.expectRevert(EnvelopeLinks.FeeAuthorizationAlreadyUsed.selector); + mfaVault.createLinkWithFees{value: 0.1 ether}(request, feeAuth); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // Helpers + // ══════════════════════════════════════════════════════════════════════════════ + + function _signOpen(address vaultAddr, uint256 idx, address recipient) internal view returns (bytes memory) { + EnvelopeLinks v = EnvelopeLinks(payable(vaultAddr)); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + keccak256( + abi.encodePacked(v.ENVELOPE_SALT(), block.chainid, vaultAddr, idx, recipient, v.OPEN_CLAIM_MODE()) + ) + ); + (uint8 vv, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); + return abi.encodePacked(r, s, vv); + } + + function _signBound(address vaultAddr, uint256 idx, address recipient) internal view returns (bytes memory) { + EnvelopeLinks v = EnvelopeLinks(payable(vaultAddr)); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + keccak256( + abi.encodePacked(v.ENVELOPE_SALT(), block.chainid, vaultAddr, idx, recipient, v.BOUND_CLAIM_MODE()) + ) + ); + (uint8 vv, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); + return abi.encodePacked(r, s, vv); + } + + function _signMfa(address vaultAddr, uint256 idx, address recipient, uint256 deadline, uint256 privKey) + internal + view + returns (bytes memory) + { + EnvelopeLinks v = EnvelopeLinks(payable(vaultAddr)); + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + keccak256(abi.encodePacked(v.ENVELOPE_SALT(), block.chainid, vaultAddr, idx, recipient, deadline)) + ); + (uint8 vv, bytes32 r, bytes32 s) = vm.sign(privKey, digest); + return abi.encodePacked(r, s, vv); + } +} diff --git a/test/envelope/SigWithdraw.t.sol b/test/envelope/SigWithdraw.t.sol index 8ae71a59..fed4a905 100644 --- a/test/envelope/SigWithdraw.t.sol +++ b/test/envelope/SigWithdraw.t.sol @@ -3,57 +3,72 @@ pragma solidity ^0.8.19; import "forge-std/Test.sol"; import "../../src/envelope/EnvelopeLinks.sol"; -import "./mocks/ERC20Mock.sol"; -import "./mocks/ERC721Mock.sol"; -import "./mocks/ERC1155Mock.sol"; -import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol"; -import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol"; +import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; contract TestSigWithdrawEther is Test { EnvelopeLinks public vault; - // sample inputs - address _pubkey20 = 0x8fd379246834eac74B8419FfdA202CF8051F7A03; + uint256 constant LINK_PRIV = 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; + address _pubkey20; address _recipientAddress = 0x6B3751c5b04Aa818EA90115AA06a4D9A36A16f02; - bytes public signatureAnybody = - hex"02a37d0548c14c6b07eba4ef1438eb946cdada4f481164755129eb3725f7e8c13d7c052308e73314338f4d484a5f4aef20c7519a1dbc283e4826253b742817241c"; - bytes public signatureRecipient = - hex"364c17bca8823977b29b7646c954353996f363549f08ce3943969171c050f0d74006eabb597df680e9e4229631f473bfbedf995336a03d2fd3be7f1fff22d2511b"; - receive() external payable {} // necessary to receive ether + receive() external payable {} function setUp() public { - console.log("Setting up test"); vault = new EnvelopeLinks(address(0), address(this), address(0)); + _pubkey20 = vm.addr(LINK_PRIV); + } + + function _signOpen(uint256 idx, address recipient) internal view returns (bytes memory) { + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + keccak256( + abi.encodePacked(vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, recipient, vault.OPEN_CLAIM_MODE()) + ) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); + return abi.encodePacked(r, s, v); + } + + function _signBound(uint256 idx, address recipient) internal view returns (bytes memory) { + bytes32 digest = MessageHashUtils.toEthSignedMessageHash( + keccak256( + abi.encodePacked(vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, recipient, vault.BOUND_CLAIM_MODE()) + ) + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); + return abi.encodePacked(r, s, v); } - // test sender withdrawal of ETH function testSigWithdrawEther(uint64 amount) public { vm.assume(amount > 0); uint256 depositIdx = vault.createLink{value: amount}(address(0), 0, amount, 0, _pubkey20); + bytes memory sigAnybody = _signOpen(depositIdx, _recipientAddress); - // Can't use withdrawDepositAsRecipient - vm.expectRevert(EnvelopeLinks.NotTheRecipient.selector); - vault.claimAsBoundRecipient(depositIdx, _recipientAddress, signatureAnybody); + // Can't use claimAsBoundRecipient on unbound link + vm.prank(_recipientAddress); + vm.expectRevert(EnvelopeLinks.LinkNotRecipientBound.selector); + vault.claimAsBoundRecipient(depositIdx, _recipientAddress, sigAnybody); - // Anybody can withdraw - vault.claim(depositIdx, _recipientAddress, signatureAnybody); + // Anybody can withdraw with open-mode signature + vault.claim(depositIdx, _recipientAddress, sigAnybody); } function testWithdrawDepositAsRecipient(uint64 amount) public { vm.assume(amount > 0); - uint256 depositIdx = vault.createLink{value: amount}(address(0), 0, amount, 0, _pubkey20); + uint256 depositIdx = vault.createCustomLink{value: amount}( + address(0), 0, amount, 0, _pubkey20, address(this), false, _recipientAddress, 0 + ); + bytes memory sigBound = _signBound(depositIdx, _recipientAddress); - // Can't use pure withdrawDeposit + // Can't use open claim with bound-mode signature vm.expectRevert(EnvelopeLinks.WrongSignature.selector); - vault.claim(depositIdx, _recipientAddress, signatureRecipient); + vault.claim(depositIdx, _recipientAddress, sigBound); - // Only the recipient is able to withdraw via withdrawDepositAsRecipient + // Non-recipient caller is rejected vm.expectRevert(EnvelopeLinks.NotTheRecipient.selector); - vault.claimAsBoundRecipient(depositIdx, _recipientAddress, signatureRecipient); + vault.claimAsBoundRecipient(depositIdx, _recipientAddress, sigBound); - vm.prank(_recipientAddress); // Withdraw! - vault.claimAsBoundRecipient(depositIdx, _recipientAddress, signatureRecipient); + vm.prank(_recipientAddress); + vault.claimAsBoundRecipient(depositIdx, _recipientAddress, sigBound); } } diff --git a/test/envelope/mocks/FeeOnTransferERC20Mock.sol b/test/envelope/mocks/FeeOnTransferERC20Mock.sol new file mode 100644 index 00000000..0c0a5378 --- /dev/null +++ b/test/envelope/mocks/FeeOnTransferERC20Mock.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +/// @dev ERC-20 mock that takes a 1% fee on every transfer (simulates fee-on-transfer tokens). +contract FeeOnTransferERC20Mock is ERC20 { + constructor() ERC20("FeeOnTransfer", "FOT") {} + + function mint(address account, uint256 amount) external { + _mint(account, amount); + } + + function _update(address from, address to, uint256 value) internal override { + if (from != address(0) && to != address(0)) { + // Burn 1% as a fee + uint256 fee = value / 100; + super._update(from, address(0), fee); + super._update(from, to, value - fee); + } else { + super._update(from, to, value); + } + } +} From 4c822343ff95bc82436546cc9fd3402aa746c0d2 Mon Sep 17 00:00:00 2001 From: Alex Sedighi Date: Thu, 21 May 2026 13:27:54 +1200 Subject: [PATCH 2/5] fix(envelope): finalize security hardening follow-up changes --- src/envelope/EnvelopeLinks.sol | 7 ++++++- src/envelope/doc/EnvelopeLinks.md | 16 ++++++++-------- test/envelope/EnvelopeSecurity.t.sol | 22 ++++++---------------- test/envelope/SigWithdraw.t.sol | 8 ++++++-- 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/envelope/EnvelopeLinks.sol b/src/envelope/EnvelopeLinks.sol index d855a837..ae750ad0 100644 --- a/src/envelope/EnvelopeLinks.sol +++ b/src/envelope/EnvelopeLinks.sol @@ -719,7 +719,12 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow encoded, _request.tokenAddress, _request.contractType, _request.amount, _request.tokenId ); _writeFeeAuthorizationParties( - encoded, _request.claimKey, _request.onBehalfOf, _request.withMFA, _request.recipient, _request.reclaimableAfter + encoded, + _request.claimKey, + _request.onBehalfOf, + _request.withMFA, + _request.recipient, + _request.reclaimableAfter ); _writeFeeAuthorizationFees( encoded, diff --git a/src/envelope/doc/EnvelopeLinks.md b/src/envelope/doc/EnvelopeLinks.md index 96e79c30..6a1663bc 100644 --- a/src/envelope/doc/EnvelopeLinks.md +++ b/src/envelope/doc/EnvelopeLinks.md @@ -198,18 +198,18 @@ Gasless eligibility is independent of the gift amount. The paymaster must still constructor(address mfaAuthorizer, address owner, address feeToken) ``` -| Param | Purpose | -| --------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Param | Purpose | +| --------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `mfaAuthorizer` | Backend signer for MFA claim approvals and link-creation-time fee authorizations. `address(0)` disables non-zero fee authorizations and makes MFA withdrawals fail. Rotatable by owner via `setMfaAuthorizer`. | -| `owner` | Owns the vault, can withdraw accumulated fees, and rotate the `mfaAuthorizer`. | -| `feeToken` | ERC-20 used for Nodle service and gasless sponsorship fees, for example NODL. `address(0)` permits only zero-fee deposits. | +| `owner` | Owns the vault, can withdraw accumulated fees, and rotate the `mfaAuthorizer`. | +| `feeToken` | ERC-20 used for Nodle service and gasless sponsorship fees, for example NODL. `address(0)` permits only zero-fee deposits. | ## Owner Functions -| Function | Purpose | -| ----------------------------------- | ---------------------------------------------------------------------------------------- | -| `setMfaAuthorizer(address)` | Rotate the MFA/fee-authorization signer. Invalidates all in-flight signatures from the old key. | -| `withdrawFees(address tokenAddress)`| Withdraw accumulated service and gasless fees for a given token. | +| Function | Purpose | +| ------------------------------------ | ----------------------------------------------------------------------------------------------- | +| `setMfaAuthorizer(address)` | Rotate the MFA/fee-authorization signer. Invalidates all in-flight signatures from the old key. | +| `withdrawFees(address tokenAddress)` | Withdraw accumulated service and gasless fees for a given token. | ## Security Properties diff --git a/test/envelope/EnvelopeSecurity.t.sol b/test/envelope/EnvelopeSecurity.t.sol index be5fe76f..50e7fb43 100644 --- a/test/envelope/EnvelopeSecurity.t.sol +++ b/test/envelope/EnvelopeSecurity.t.sol @@ -63,7 +63,9 @@ contract EnvelopeSecurityTest is Test { fotToken.approve(address(vault), 100000); address[] memory keys = new address[](5); - for (uint256 i = 0; i < 5; i++) keys[i] = linkPubKey; + for (uint256 i = 0; i < 5; i++) { + keys[i] = linkPubKey; + } uint256[] memory indexes = vault.createLinks(address(fotToken), 1, 1000, 0, keys); @@ -145,8 +147,7 @@ contract EnvelopeSecurityTest is Test { bytes memory sig = _signOpen(address(vault), idx, ALICE); bytes memory mfaSig = hex"00"; - bytes memory callData = - abi.encodeCall(EnvelopeLinks.claimWithMFA, (idx, ALICE, sig, mfaSig, 0)); + bytes memory callData = abi.encodeCall(EnvelopeLinks.claimWithMFA, (idx, ALICE, sig, mfaSig, 0)); bool valid = vault.isValidGaslessOperation(ALICE, callData); assertFalse(valid, "Should reject when mfaAuthorizer is zero"); @@ -221,24 +222,13 @@ contract EnvelopeSecurityTest is Test { uint256 deadline = block.timestamp + 1 hours; bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( - mfaVault.ENVELOPE_SALT(), - address(mfaVault), - request, - address(this), - serviceFee, - gaslessFee, - false, - deadline + mfaVault.ENVELOPE_SALT(), address(mfaVault), request, address(this), serviceFee, gaslessFee, false, deadline ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(MFA_PRIV, digest); bytes memory sig = abi.encodePacked(r, s, v); EnvelopeLinks.FeeAuthorization memory feeAuth = EnvelopeLinks.FeeAuthorization({ - serviceFee: serviceFee, - gaslessFee: gaslessFee, - gaslessSponsored: false, - deadline: deadline, - signature: sig + serviceFee: serviceFee, gaslessFee: gaslessFee, gaslessSponsored: false, deadline: deadline, signature: sig }); // First use succeeds. diff --git a/test/envelope/SigWithdraw.t.sol b/test/envelope/SigWithdraw.t.sol index fed4a905..f4e48132 100644 --- a/test/envelope/SigWithdraw.t.sol +++ b/test/envelope/SigWithdraw.t.sol @@ -22,7 +22,9 @@ contract TestSigWithdrawEther is Test { function _signOpen(uint256 idx, address recipient) internal view returns (bytes memory) { bytes32 digest = MessageHashUtils.toEthSignedMessageHash( keccak256( - abi.encodePacked(vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, recipient, vault.OPEN_CLAIM_MODE()) + abi.encodePacked( + vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, recipient, vault.OPEN_CLAIM_MODE() + ) ) ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); @@ -32,7 +34,9 @@ contract TestSigWithdrawEther is Test { function _signBound(uint256 idx, address recipient) internal view returns (bytes memory) { bytes32 digest = MessageHashUtils.toEthSignedMessageHash( keccak256( - abi.encodePacked(vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, recipient, vault.BOUND_CLAIM_MODE()) + abi.encodePacked( + vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, recipient, vault.BOUND_CLAIM_MODE() + ) ) ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); From 2d9c8b72b0bb33227d197b987eb3786265959c18 Mon Sep 17 00:00:00 2001 From: Alex Sedighi Date: Thu, 21 May 2026 14:08:39 +1200 Subject: [PATCH 3/5] feat(envelope): migrate signatures to EIP-712 typed structured data Replace all custom EIP-191 signature schemes with EIP-712: - Inherit OpenZeppelin EIP712 (domain: 'EnvelopeLinks', version '5') - Define CLAIM_TYPEHASH, MFA_APPROVAL_TYPEHASH, FEE_AUTHORIZATION_TYPEHASH - Remove ENVELOPE_SALT constant and assembly-based fee auth digest helpers - All three signature sites now use _hashTypedDataV4(keccak256(abi.encode(...))) Benefits: - Domain separator includes chainId + verifyingContract (cross-chain/contract replay protection) - Wallet-readable signing prompts (EIP-712 structured data) - Compliant with EIP-5267 (eip712Domain() getter) - Removes ~80 lines of hand-rolled assembly for fee auth digest Adds EnvelopeEIP712.t.sol with 19 dedicated tests covering: - Domain separator correctness and uniqueness - Cross-chain and cross-contract replay protection - Typehash verification - Claim mode discrimination - Fuzz tests for arbitrary recipients and deadlines - EIP-5267 getter validation Updates all existing test helpers to use shared EnvelopeEIP712Utils library. All 1072 tests pass. Spellcheck clean. --- .cspell.json | 1 + src/envelope/EnvelopeLinks.sol | 152 +++------- test/envelope/Coverage.t.sol | 27 +- test/envelope/EnvelopeBatching.t.sol | 11 +- test/envelope/EnvelopeEIP712.t.sol | 331 +++++++++++++++++++++ test/envelope/EnvelopeEIP712Utils.sol | 72 +++++ test/envelope/EnvelopeEdgeCases.t.sol | 18 +- test/envelope/EnvelopeFeeAuthTestUtils.sol | 30 +- test/envelope/EnvelopeHardening.t.sol | 35 +-- test/envelope/EnvelopeSecurity.t.sol | 23 +- test/envelope/Gasless.t.sol | 16 +- test/envelope/MFA.t.sol | 23 +- test/envelope/SigWithdraw.t.sol | 18 +- test/paymasters/EnvelopePaymaster.t.sol | 17 +- 14 files changed, 492 insertions(+), 282 deletions(-) create mode 100644 test/envelope/EnvelopeEIP712.t.sol create mode 100644 test/envelope/EnvelopeEIP712Utils.sol diff --git a/.cspell.json b/.cspell.json index f4e78a4c..84d43097 100644 --- a/.cspell.json +++ b/.cspell.json @@ -19,6 +19,7 @@ "AMPL", "NODL", "Nodle", + "Typehashes", "depin", "contentsign", "matterlabs", diff --git a/src/envelope/EnvelopeLinks.sol b/src/envelope/EnvelopeLinks.sol index ae750ad0..be4c7cb1 100644 --- a/src/envelope/EnvelopeLinks.sol +++ b/src/envelope/EnvelopeLinks.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.26; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; +import {EIP712} from "@openzeppelin/contracts/utils/cryptography/EIP712.sol"; import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; @@ -15,7 +15,7 @@ import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import {Ownable2Step, Ownable} from "@openzeppelin/contracts/access/Ownable2Step.sol"; -contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ownable2Step { +contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ownable2Step, EIP712 { using SafeERC20 for IERC20; // ── Custom Errors ──────────────────────────────────────────────────────────── @@ -109,12 +109,19 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow bytes signature; } - // We may include this hash in peanut-specific signatures to make sure - // that the message signed by the user has effects only in peanut contracts. - bytes32 public constant ENVELOPE_SALT = 0x70adbbeba9d4f0c82e28dd574f15466f75df0543b65f24460fc445813b5d94e0; // keccak256("Konrad makes tokens go woosh tadam"); + // ── EIP-712 Typehashes ─────────────────────────────────────────────────────── - bytes32 public constant OPEN_CLAIM_MODE = 0x0000000000000000000000000000000000000000000000000000000000000000; // default. Any address can trigger the withdrawal function - bytes32 public constant BOUND_CLAIM_MODE = 0x2bb5bef2b248d3edba501ad918c3ab524cce2aea54d4c914414e1c4401dc4ff4; // keccak256("only recipient") - only the signed recipient can trigger the withdrawal function + bytes32 public constant CLAIM_TYPEHASH = keccak256("Claim(uint256 index,address recipient,bytes32 mode)"); + + bytes32 public constant MFA_APPROVAL_TYPEHASH = + keccak256("MfaApproval(uint256 index,address recipient,uint256 deadline)"); + + bytes32 public constant FEE_AUTHORIZATION_TYPEHASH = keccak256( + "FeeAuthorization(address feePayer,address tokenAddress,uint8 contractType,uint256 amount,uint256 tokenId,address claimKey,address onBehalfOf,bool withMFA,address recipient,uint40 reclaimableAfter,uint256 serviceFee,uint256 gaslessFee,bool gaslessSponsored,uint256 deadline)" + ); + + bytes32 public constant OPEN_CLAIM_MODE = 0x0000000000000000000000000000000000000000000000000000000000000000; + bytes32 public constant BOUND_CLAIM_MODE = 0x2bb5bef2b248d3edba501ad918c3ab524cce2aea54d4c914414e1c4401dc4ff4; // keccak256("only recipient") /// @notice Address authorized to issue MFA signatures gating claimWithMFA calls and fee authorizations. /// @dev Rotatable by owner. Address(0) disables MFA — claimWithMFA will revert. @@ -143,7 +150,10 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow /// @param _mfaAuthorizer address authorized to sign backend fee and MFA approvals (use address(0) to disable). /// @param _owner initial owner of the contract (receives accumulated fees). /// @param _feeToken ERC-20 token used for fees; address(0) disables non-zero fee authorizations. - constructor(address _mfaAuthorizer, address _owner, address _feeToken) Ownable(_owner) { + constructor(address _mfaAuthorizer, address _owner, address _feeToken) + Ownable(_owner) + EIP712("EnvelopeLinks", "5") + { mfaAuthorizer = _mfaAuthorizer; feeToken = IERC20(_feeToken); } @@ -662,10 +672,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow // deadline == 0 means no expiry if (_deadline != 0 && block.timestamp > _deadline) revert MfaSignatureExpired(); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked(ENVELOPE_SALT, block.chainid, address(this), _index, _recipientAddress, _deadline) - ) + bytes32 digest = _hashTypedDataV4( + keccak256(abi.encode(MFA_APPROVAL_TYPEHASH, _index, _recipientAddress, _deadline)) ); address authorizationSigner = getSigner(digest, _MFASignature); if (authorizationSigner != mfaAuthorizer) revert WrongMfaSignature(); @@ -679,10 +687,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow if (mfaAuthorizer == address(0)) return false; if (_deadline != 0 && block.timestamp > _deadline) return false; - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked(ENVELOPE_SALT, block.chainid, address(this), _index, _recipientAddress, _deadline) - ) + bytes32 digest = _hashTypedDataV4( + keccak256(abi.encode(MFA_APPROVAL_TYPEHASH, _index, _recipientAddress, _deadline)) ); return _recoverSigner(digest, _signature) == mfaAuthorizer; } @@ -713,89 +719,27 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow FeeAuthorization calldata _feeAuthorization, address _feePayer ) internal view returns (bytes32) { - bytes memory encoded = new bytes(17 * 32); - _writeFeeAuthorizationContext(encoded, _feePayer); - _writeFeeAuthorizationAsset( - encoded, _request.tokenAddress, _request.contractType, _request.amount, _request.tokenId - ); - _writeFeeAuthorizationParties( - encoded, - _request.claimKey, - _request.onBehalfOf, - _request.withMFA, - _request.recipient, - _request.reclaimableAfter - ); - _writeFeeAuthorizationFees( - encoded, - _feeAuthorization.serviceFee, - _feeAuthorization.gaslessFee, - _feeAuthorization.gaslessSponsored, - _feeAuthorization.deadline + return _hashTypedDataV4( + keccak256( + abi.encode( + FEE_AUTHORIZATION_TYPEHASH, + _feePayer, + _request.tokenAddress, + _request.contractType, + _request.amount, + _request.tokenId, + _request.claimKey, + _request.onBehalfOf, + _request.withMFA, + _request.recipient, + _request.reclaimableAfter, + _feeAuthorization.serviceFee, + _feeAuthorization.gaslessFee, + _feeAuthorization.gaslessSponsored, + _feeAuthorization.deadline + ) + ) ); - - return MessageHashUtils.toEthSignedMessageHash(keccak256(encoded)); - } - - function _writeFeeAuthorizationContext(bytes memory encoded, address _feePayer) internal view { - bytes32 salt = ENVELOPE_SALT; - assembly ("memory-safe") { - let ptr := add(encoded, 32) - mstore(ptr, salt) - mstore(add(ptr, 32), chainid()) - mstore(add(ptr, 64), address()) - mstore(add(ptr, 96), _feePayer) - } - } - - function _writeFeeAuthorizationAsset( - bytes memory encoded, - address _tokenAddress, - uint8 _contractType, - uint256 _amount, - uint256 _tokenId - ) internal pure { - assembly ("memory-safe") { - let ptr := add(encoded, 160) - mstore(ptr, _tokenAddress) - mstore(add(ptr, 32), _contractType) - mstore(add(ptr, 64), _amount) - mstore(add(ptr, 96), _tokenId) - } - } - - function _writeFeeAuthorizationParties( - bytes memory encoded, - address claimKey, - address _onBehalfOf, - bool _withMFA, - address _recipient, - uint40 _reclaimableAfter - ) internal pure { - assembly ("memory-safe") { - let ptr := add(encoded, 288) - mstore(ptr, claimKey) - mstore(add(ptr, 32), _onBehalfOf) - mstore(add(ptr, 64), _withMFA) - mstore(add(ptr, 96), _recipient) - mstore(add(ptr, 128), _reclaimableAfter) - } - } - - function _writeFeeAuthorizationFees( - bytes memory encoded, - uint256 _serviceFee, - uint256 _gaslessFee, - bool _gaslessSponsored, - uint256 _deadline - ) internal pure { - assembly ("memory-safe") { - let ptr := add(encoded, 448) - mstore(ptr, _serviceFee) - mstore(add(ptr, 32), _gaslessFee) - mstore(add(ptr, 64), _gaslessSponsored) - mstore(add(ptr, 96), _deadline) - } } function _collectLinkFees(uint256 _index, address _feePayer, uint256 _serviceFee, uint256 _gaslessFee) internal { @@ -875,10 +819,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow if (deposit.parties.recipient != address(0) && _recipientAddress != deposit.parties.recipient) return false; if (deposit.status.claimKey != address(0)) { - bytes32 _claimHash = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked(ENVELOPE_SALT, block.chainid, address(this), _index, _recipientAddress, _extraData) - ) + bytes32 _claimHash = _hashTypedDataV4( + keccak256(abi.encode(CLAIM_TYPEHASH, _index, _recipientAddress, _extraData)) ); if (_recoverSigner(_claimHash, _signature) != deposit.status.claimKey) return false; } @@ -1102,10 +1044,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow address claimSigner; if (_signature.length > 0) { - bytes32 _claimHash = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked(ENVELOPE_SALT, block.chainid, address(this), _index, _recipientAddress, _extraData) - ) + bytes32 _claimHash = _hashTypedDataV4( + keccak256(abi.encode(CLAIM_TYPEHASH, _index, _recipientAddress, _extraData)) ); claimSigner = getSigner(_claimHash, _signature); } diff --git a/test/envelope/Coverage.t.sol b/test/envelope/Coverage.t.sol index 32d2651b..aad15154 100644 --- a/test/envelope/Coverage.t.sol +++ b/test/envelope/Coverage.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; import "forge-std/Test.sol"; import "../../src/envelope/EnvelopeLinks.sol"; import {EnvelopeFeeAuthTestUtils} from "./EnvelopeFeeAuthTestUtils.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; import "./mocks/ERC20Mock.sol"; import "./mocks/ERC721Mock.sol"; import "./mocks/ERC1155Mock.sol"; @@ -71,21 +72,13 @@ contract EnvelopeCoverageTest is Test { view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked(vault.ENVELOPE_SALT(), block.chainid, address(vault), index, recipient, mode) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), index, recipient, mode); (uint8 v, bytes32 r, bytes32 s) = vm.sign(linkPrivKey, digest); return abi.encodePacked(r, s, v); } function _signMfa(uint256 index, address recipient, uint256 deadline) internal view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked(vault.ENVELOPE_SALT(), block.chainid, address(vault), index, recipient, deadline) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.mfaDigest(address(vault), index, recipient, deadline); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BACKEND_PRIVKEY, digest); return abi.encodePacked(r, s, v); } @@ -111,7 +104,7 @@ contract EnvelopeCoverageTest is Test { uint256 deadline ) internal view returns (bytes memory) { bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( - vault.ENVELOPE_SALT(), vaultAddr, req, feePayer, serviceFee, gaslessFee, gaslessSponsored, deadline + vaultAddr, req, feePayer, serviceFee, gaslessFee, gaslessSponsored, deadline ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BACKEND_PRIVKEY, digest); return abi.encodePacked(r, s, v); @@ -228,8 +221,8 @@ contract EnvelopeCoverageTest is Test { // ══════════════════════════════════════════════════════════════════════════════ function test_withdrawFees_eth() public { - // Seed accumulatedFees[address(0)] with ETH balance - bytes32 slot = keccak256(abi.encode(address(0), uint256(5))); + // Seed accumulatedFees[address(0)] with ETH balance (slot 7 for accumulatedFees mapping) + bytes32 slot = keccak256(abi.encode(address(0), uint256(7))); vm.store(address(vault), slot, bytes32(uint256(0.5 ether))); vm.deal(address(vault), 0.5 ether); @@ -1103,8 +1096,8 @@ contract EnvelopeCoverageTest is Test { // Seed directly: we can call withdrawFees with ETH balance. vm.deal(address(rejVault), 1 ether); // Write to the accumulatedFees[address(0)] storage slot - // accumulatedFees is at storage slot 5 in the contract layout - bytes32 slot = keccak256(abi.encode(address(0), uint256(5))); + // accumulatedFees is at storage slot 7 in the contract layout + bytes32 slot = keccak256(abi.encode(address(0), uint256(7))); vm.store(address(rejVault), slot, bytes32(uint256(1 ether))); vm.prank(address(rejecter)); @@ -1280,9 +1273,7 @@ contract EnvelopeCoverageTest is Test { uint256 deadline = block.timestamp + 1 hours; bytes memory claimSig = _signClaim(LINK_PRIVKEY, idx, RECIPIENT, vault.OPEN_CLAIM_MODE()); // Use a wrong signature (signed by LINK_PRIVKEY instead of BACKEND) - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256(abi.encodePacked(vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, RECIPIENT, deadline)) - ); + bytes32 digest = EnvelopeEIP712Utils.mfaDigest(address(vault), idx, RECIPIENT, deadline); (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIVKEY, digest); bytes memory wrongMfaSig = abi.encodePacked(r, s, v); diff --git a/test/envelope/EnvelopeBatching.t.sol b/test/envelope/EnvelopeBatching.t.sol index 96cd30cd..8bfbb930 100644 --- a/test/envelope/EnvelopeBatching.t.sol +++ b/test/envelope/EnvelopeBatching.t.sol @@ -4,12 +4,12 @@ pragma solidity ^0.8.19; import "forge-std/Test.sol"; import {EnvelopeLinks} from "../../src/envelope/EnvelopeLinks.sol"; import {EnvelopeFeeAuthTestUtils} from "./EnvelopeFeeAuthTestUtils.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; import "./mocks/ERC20Mock.sol"; import "./mocks/ERC721Mock.sol"; import "./mocks/ERC1155Mock.sol"; import "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol"; import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol"; -import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; contract EnvelopeBatchingTest is Test, ERC1155Holder, ERC721Holder { EnvelopeLinks public vault; @@ -411,7 +411,6 @@ contract EnvelopeBatchingTest is Test, ERC1155Holder, ERC721Holder { uint256 deadline ) internal view returns (bytes memory) { bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( - targetVault.ENVELOPE_SALT(), address(targetVault), request, feePayer, @@ -429,13 +428,7 @@ contract EnvelopeBatchingTest is Test, ERC1155Holder, ERC721Holder { view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - targetVault.ENVELOPE_SALT(), block.chainid, address(targetVault), depositIndex, recipient, mode - ) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(targetVault), depositIndex, recipient, mode); (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIVKEY, digest); return abi.encodePacked(r, s, v); } diff --git a/test/envelope/EnvelopeEIP712.t.sol b/test/envelope/EnvelopeEIP712.t.sol new file mode 100644 index 00000000..b4b396c7 --- /dev/null +++ b/test/envelope/EnvelopeEIP712.t.sol @@ -0,0 +1,331 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.26; + +// Thorough EIP-712 tests for EnvelopeLinks: +// - Domain separator correctness +// - Cross-chain replay protection (domain separator includes chainId) +// - Cross-contract replay protection (domain separator includes verifyingContract) +// - Typehash verification +// - Structured data correctness for all three signed message types +// - eip712Domain() getter (EIP-5267) + +import {Test} from "forge-std/Test.sol"; +import {EnvelopeLinks} from "../../src/envelope/EnvelopeLinks.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; +import {EnvelopeFeeAuthTestUtils} from "./EnvelopeFeeAuthTestUtils.sol"; +import {ERC20Mock} from "./mocks/ERC20Mock.sol"; + +contract EnvelopeEIP712Test is Test { + EnvelopeLinks public vault; + EnvelopeLinks public vault2; // second instance for cross-contract tests + ERC20Mock public feeToken; + + uint256 constant LINK_PRIV = 0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; + uint256 constant MFA_PRIV = uint256(keccak256("eip712-test-mfa-signer")); + address linkPubKey; + address mfaSigner; + + address constant ALICE = address(0xA11CE); + address constant BOB = address(0xB0B); + + function setUp() public { + linkPubKey = vm.addr(LINK_PRIV); + mfaSigner = vm.addr(MFA_PRIV); + feeToken = new ERC20Mock(); + + vault = new EnvelopeLinks(mfaSigner, address(this), address(feeToken)); + vault2 = new EnvelopeLinks(mfaSigner, address(this), address(feeToken)); + } + + receive() external payable {} + + // ══════════════════════════════════════════════════════════════════════════════ + // Domain separator correctness + // ══════════════════════════════════════════════════════════════════════════════ + + function test_domainSeparator_matchesExpected() public view { + bytes32 expected = keccak256( + abi.encode( + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), + keccak256("EnvelopeLinks"), + keccak256("5"), + block.chainid, + address(vault) + ) + ); + assertEq(EnvelopeEIP712Utils.domainSeparator(address(vault)), expected); + } + + function test_domainSeparator_differsBetweenInstances() public view { + bytes32 ds1 = EnvelopeEIP712Utils.domainSeparator(address(vault)); + bytes32 ds2 = EnvelopeEIP712Utils.domainSeparator(address(vault2)); + assertTrue(ds1 != ds2, "Different contract addresses must have different domain separators"); + } + + function test_domainSeparator_includesChainId() public { + bytes32 ds1 = EnvelopeEIP712Utils.domainSeparator(address(vault)); + + // Fork to a different chain ID + vm.chainId(999); + bytes32 ds2 = EnvelopeEIP712Utils.domainSeparator(address(vault)); + + assertTrue(ds1 != ds2, "Different chain IDs must produce different domain separators"); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // EIP-5267: eip712Domain() getter + // ══════════════════════════════════════════════════════════════════════════════ + + function test_eip712Domain_returnsCorrectValues() public view { + ( + bytes1 fields, + string memory name, + string memory version, + uint256 chainId, + address verifyingContract, + bytes32 salt, + uint256[] memory extensions + ) = vault.eip712Domain(); + + assertEq(uint8(fields), 0x0f, "Fields should indicate name, version, chainId, verifyingContract"); + assertEq(keccak256(bytes(name)), keccak256("EnvelopeLinks")); + assertEq(keccak256(bytes(version)), keccak256("5")); + assertEq(chainId, block.chainid); + assertEq(verifyingContract, address(vault)); + assertEq(salt, bytes32(0)); + assertEq(extensions.length, 0); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // Typehash verification + // ══════════════════════════════════════════════════════════════════════════════ + + function test_claimTypehash() public view { + bytes32 expected = keccak256("Claim(uint256 index,address recipient,bytes32 mode)"); + assertEq(vault.CLAIM_TYPEHASH(), expected); + } + + function test_mfaApprovalTypehash() public view { + bytes32 expected = keccak256("MfaApproval(uint256 index,address recipient,uint256 deadline)"); + assertEq(vault.MFA_APPROVAL_TYPEHASH(), expected); + } + + function test_feeAuthorizationTypehash() public view { + bytes32 expected = keccak256( + "FeeAuthorization(address feePayer,address tokenAddress,uint8 contractType,uint256 amount,uint256 tokenId,address claimKey,address onBehalfOf,bool withMFA,address recipient,uint40 reclaimableAfter,uint256 serviceFee,uint256 gaslessFee,bool gaslessSponsored,uint256 deadline)" + ); + assertEq(vault.FEE_AUTHORIZATION_TYPEHASH(), expected); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // Cross-chain replay protection + // ══════════════════════════════════════════════════════════════════════════════ + + function test_claimSignature_invalidOnDifferentChain() public { + // Create a link on the original chain + uint256 idx = vault.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + + // Sign the claim on chain 31337 (default foundry chain id) + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), idx, ALICE, vault.OPEN_CLAIM_MODE()); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); + bytes memory sig = abi.encodePacked(r, s, v); + + // Switch to a different chain and verify the signature fails + vm.chainId(42161); // Arbitrum chain ID + + vm.expectRevert(EnvelopeLinks.WrongSignature.selector); + vault.claim(idx, ALICE, sig); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // Cross-contract replay protection + // ══════════════════════════════════════════════════════════════════════════════ + + function test_claimSignature_invalidOnDifferentContract() public { + // Create identical links on both vaults + uint256 idx1 = vault.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + uint256 idx2 = vault2.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + assertEq(idx1, idx2, "Both should be index 0"); + + // Sign for vault1 + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), idx1, ALICE, vault.OPEN_CLAIM_MODE()); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); + bytes memory sig = abi.encodePacked(r, s, v); + + // Claim on vault1 works + vault.claim(idx1, ALICE, sig); + + // Same signature on vault2 fails + vm.expectRevert(EnvelopeLinks.WrongSignature.selector); + vault2.claim(idx2, ALICE, sig); + } + + function test_mfaSignature_invalidOnDifferentContract() public { + // Create MFA links on both vaults + uint256 idx1 = vault.createMFALink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + uint256 idx2 = vault2.createMFALink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + + // Sign MFA for vault1 + bytes32 mfaDigest = EnvelopeEIP712Utils.mfaDigest(address(vault), idx1, ALICE, 0); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(MFA_PRIV, mfaDigest); + bytes memory mfaSig = abi.encodePacked(r, s, v); + + bytes32 claimDigest1 = EnvelopeEIP712Utils.claimDigest(address(vault), idx1, ALICE, vault.OPEN_CLAIM_MODE()); + (v, r, s) = vm.sign(LINK_PRIV, claimDigest1); + bytes memory claimSig1 = abi.encodePacked(r, s, v); + + // Works on vault1 + vault.claimWithMFA(idx1, ALICE, claimSig1, mfaSig, 0); + + // MFA sig from vault1 fails on vault2 + bytes32 claimDigest2 = EnvelopeEIP712Utils.claimDigest(address(vault2), idx2, ALICE, vault2.OPEN_CLAIM_MODE()); + (v, r, s) = vm.sign(LINK_PRIV, claimDigest2); + bytes memory claimSig2 = abi.encodePacked(r, s, v); + + vm.expectRevert(EnvelopeLinks.WrongMfaSignature.selector); + vault2.claimWithMFA(idx2, ALICE, claimSig2, mfaSig, 0); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // Fee authorization cross-contract replay + // ══════════════════════════════════════════════════════════════════════════════ + + function test_feeAuthorization_invalidOnDifferentContract() public { + feeToken.mint(address(this), 10 ether); + feeToken.approve(address(vault), type(uint256).max); + feeToken.approve(address(vault2), type(uint256).max); + + EnvelopeLinks.LinkRequest memory request = EnvelopeLinks.LinkRequest({ + tokenAddress: address(0), + contractType: 0, + amount: 0.1 ether, + tokenId: 0, + claimKey: linkPubKey, + onBehalfOf: address(this), + withMFA: false, + recipient: address(0), + reclaimableAfter: 0 + }); + + uint256 serviceFee = 100; + uint256 deadline = block.timestamp + 1 hours; + + // Sign fee auth for vault1 + bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( + address(vault), request, address(this), serviceFee, 0, false, deadline + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(MFA_PRIV, digest); + bytes memory sig = abi.encodePacked(r, s, v); + + EnvelopeLinks.FeeAuthorization memory feeAuth = EnvelopeLinks.FeeAuthorization({ + serviceFee: serviceFee, + gaslessFee: 0, + gaslessSponsored: false, + deadline: deadline, + signature: sig + }); + + // Works on vault1 + vault.createLinkWithFees{value: 0.1 ether}(request, feeAuth); + + // Fails on vault2 (different verifyingContract in domain separator) + vm.expectRevert(EnvelopeLinks.WrongFeeAuthorizationSignature.selector); + vault2.createLinkWithFees{value: 0.1 ether}(request, feeAuth); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // Claim mode discrimination + // ══════════════════════════════════════════════════════════════════════════════ + + function test_openClaimSignature_cannotBeUsedAsBound() public { + // Create an address-bound link + uint256 idx = vault.createCustomLink{value: 1 ether}( + address(0), 0, 1 ether, 0, linkPubKey, address(this), false, ALICE, 0 + ); + + // Sign with OPEN_CLAIM_MODE + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), idx, ALICE, vault.OPEN_CLAIM_MODE()); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); + bytes memory openSig = abi.encodePacked(r, s, v); + + // Using open mode sig in bound recipient claim should fail + // (claimAsBoundRecipient passes BOUND_CLAIM_MODE as extraData) + vm.prank(ALICE); + vm.expectRevert(EnvelopeLinks.WrongSignature.selector); + vault.claimAsBoundRecipient(idx, ALICE, openSig); + } + + function test_boundClaimSignature_cannotBeUsedAsOpen() public { + // Create an open link (recipient = address(0)) + uint256 idx = vault.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + + // Sign with BOUND_CLAIM_MODE + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), idx, ALICE, vault.BOUND_CLAIM_MODE()); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); + bytes memory boundSig = abi.encodePacked(r, s, v); + + // Trying to use bound mode sig in an open claim (which uses OPEN_CLAIM_MODE internally) + vm.expectRevert(EnvelopeLinks.WrongSignature.selector); + vault.claim(idx, ALICE, boundSig); + } + + // ══════════════════════════════════════════════════════════════════════════════ + // Structured data fields completeness + // ══════════════════════════════════════════════════════════════════════════════ + + function test_claimDigest_changesWithIndex() public { + bytes32 d1 = EnvelopeEIP712Utils.claimDigest(address(vault), 0, ALICE, vault.OPEN_CLAIM_MODE()); + bytes32 d2 = EnvelopeEIP712Utils.claimDigest(address(vault), 1, ALICE, vault.OPEN_CLAIM_MODE()); + assertTrue(d1 != d2, "Different index must produce different digest"); + } + + function test_claimDigest_changesWithRecipient() public { + bytes32 d1 = EnvelopeEIP712Utils.claimDigest(address(vault), 0, ALICE, vault.OPEN_CLAIM_MODE()); + bytes32 d2 = EnvelopeEIP712Utils.claimDigest(address(vault), 0, BOB, vault.OPEN_CLAIM_MODE()); + assertTrue(d1 != d2, "Different recipient must produce different digest"); + } + + function test_claimDigest_changesWithMode() public { + bytes32 d1 = EnvelopeEIP712Utils.claimDigest(address(vault), 0, ALICE, vault.OPEN_CLAIM_MODE()); + bytes32 d2 = EnvelopeEIP712Utils.claimDigest(address(vault), 0, ALICE, vault.BOUND_CLAIM_MODE()); + assertTrue(d1 != d2, "Different mode must produce different digest"); + } + + function test_mfaDigest_changesWithDeadline() public { + bytes32 d1 = EnvelopeEIP712Utils.mfaDigest(address(vault), 0, ALICE, 0); + bytes32 d2 = EnvelopeEIP712Utils.mfaDigest(address(vault), 0, ALICE, 1000); + assertTrue(d1 != d2, "Different deadline must produce different digest"); + } + + function testFuzz_claimSignature_worksForAnyRecipient(address recipient) public { + vm.assume(recipient != address(0)); + vm.assume(recipient.code.length == 0); // avoid contracts that reject ETH + + uint256 idx = vault.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), idx, recipient, vault.OPEN_CLAIM_MODE()); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); + bytes memory sig = abi.encodePacked(r, s, v); + + vm.deal(recipient, 0); // start at 0 to verify receipt + vault.claim(idx, recipient, sig); + assertEq(recipient.balance, 1 ether); + } + + function testFuzz_mfaSignature_worksWithAnyDeadline(uint256 deadline) public { + // Ensure deadline is either 0 (no expiry) or in the future + vm.assume(deadline == 0 || deadline > block.timestamp); + + uint256 idx = vault.createMFALink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); + + bytes32 mfaDigest = EnvelopeEIP712Utils.mfaDigest(address(vault), idx, ALICE, deadline); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(MFA_PRIV, mfaDigest); + bytes memory mfaSig = abi.encodePacked(r, s, v); + + bytes32 claimDigest_ = EnvelopeEIP712Utils.claimDigest(address(vault), idx, ALICE, vault.OPEN_CLAIM_MODE()); + (v, r, s) = vm.sign(LINK_PRIV, claimDigest_); + bytes memory claimSig = abi.encodePacked(r, s, v); + + vault.claimWithMFA(idx, ALICE, claimSig, mfaSig, deadline); + } +} diff --git a/test/envelope/EnvelopeEIP712Utils.sol b/test/envelope/EnvelopeEIP712Utils.sol new file mode 100644 index 00000000..9a496642 --- /dev/null +++ b/test/envelope/EnvelopeEIP712Utils.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity ^0.8.20; + +import {EnvelopeLinks} from "../../src/envelope/EnvelopeLinks.sol"; + +/// @dev Shared EIP-712 digest helpers for EnvelopeLinks test suites. +library EnvelopeEIP712Utils { + bytes32 internal constant DOMAIN_TYPEHASH = + keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + + bytes32 internal constant NAME_HASH = keccak256("EnvelopeLinks"); + bytes32 internal constant VERSION_HASH = keccak256("5"); + + function domainSeparator(address vaultAddr) internal view returns (bytes32) { + return keccak256(abi.encode(DOMAIN_TYPEHASH, NAME_HASH, VERSION_HASH, block.chainid, vaultAddr)); + } + + function claimDigest(address vaultAddr, uint256 index, address recipient, bytes32 mode) + internal + view + returns (bytes32) + { + bytes32 structHash = + keccak256(abi.encode(EnvelopeLinks(payable(vaultAddr)).CLAIM_TYPEHASH(), index, recipient, mode)); + return _hashTypedData(vaultAddr, structHash); + } + + function mfaDigest(address vaultAddr, uint256 index, address recipient, uint256 deadline) + internal + view + returns (bytes32) + { + bytes32 structHash = + keccak256(abi.encode(EnvelopeLinks(payable(vaultAddr)).MFA_APPROVAL_TYPEHASH(), index, recipient, deadline)); + return _hashTypedData(vaultAddr, structHash); + } + + function feeAuthDigest( + address vaultAddr, + address feePayer, + EnvelopeLinks.LinkRequest memory request, + uint256 serviceFee, + uint256 gaslessFee, + bool gaslessSponsored, + uint256 deadline + ) internal view returns (bytes32) { + bytes32 structHash = keccak256( + abi.encode( + EnvelopeLinks(payable(vaultAddr)).FEE_AUTHORIZATION_TYPEHASH(), + feePayer, + request.tokenAddress, + request.contractType, + request.amount, + request.tokenId, + request.claimKey, + request.onBehalfOf, + request.withMFA, + request.recipient, + request.reclaimableAfter, + serviceFee, + gaslessFee, + gaslessSponsored, + deadline + ) + ); + return _hashTypedData(vaultAddr, structHash); + } + + function _hashTypedData(address vaultAddr, bytes32 structHash) private view returns (bytes32) { + return keccak256(abi.encodePacked("\x19\x01", domainSeparator(vaultAddr), structHash)); + } +} diff --git a/test/envelope/EnvelopeEdgeCases.t.sol b/test/envelope/EnvelopeEdgeCases.t.sol index 0f9ca7f9..fd325fef 100644 --- a/test/envelope/EnvelopeEdgeCases.t.sol +++ b/test/envelope/EnvelopeEdgeCases.t.sol @@ -10,7 +10,7 @@ import {EnvelopeLinks} from "../../src/envelope/EnvelopeLinks.sol"; import {ERC20Mock} from "./mocks/ERC20Mock.sol"; import {ERC721Mock} from "./mocks/ERC721Mock.sol"; import {ERC1155Mock} from "./mocks/ERC1155Mock.sol"; -import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; import {ERC721Holder} from "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol"; import {ERC1155Holder} from "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol"; @@ -72,13 +72,7 @@ contract EnvelopeEdgeCasesTest is Test, ERC721Holder, ERC1155Holder { // ── helpers ──────────────────────────────────────────────────────────── function _signWithdrawal(uint256 idx, address recipient, uint256 privKey) internal view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, recipient, vault.OPEN_CLAIM_MODE() - ) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), idx, recipient, vault.OPEN_CLAIM_MODE()); (uint8 v, bytes32 r, bytes32 s) = vm.sign(privKey, digest); return abi.encodePacked(r, s, v); } @@ -139,13 +133,7 @@ contract EnvelopeEdgeCasesTest is Test, ERC721Holder, ERC1155Holder { function test_RevertWhen_WithdrawAsRecipientCallerMismatch() public { // Recipient-mode signature; caller must equal the recipient. uint256 idx = _depositEth(1 ether); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, ALICE, vault.BOUND_CLAIM_MODE() - ) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), idx, ALICE, vault.BOUND_CLAIM_MODE()); (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); bytes memory sig = abi.encodePacked(r, s, v); diff --git a/test/envelope/EnvelopeFeeAuthTestUtils.sol b/test/envelope/EnvelopeFeeAuthTestUtils.sol index f13d576f..5a3059f5 100644 --- a/test/envelope/EnvelopeFeeAuthTestUtils.sol +++ b/test/envelope/EnvelopeFeeAuthTestUtils.sol @@ -1,12 +1,11 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity ^0.8.20; -import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import {EnvelopeLinks} from "../../src/envelope/EnvelopeLinks.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; library EnvelopeFeeAuthTestUtils { function feeAuthorizationDigest( - bytes32 salt, address vaultAddr, EnvelopeLinks.LinkRequest memory request, address feePayer, @@ -15,29 +14,8 @@ library EnvelopeFeeAuthTestUtils { bool gaslessSponsored, uint256 deadline ) internal view returns (bytes32) { - bytes32 digest; - assembly ("memory-safe") { - let ptr := mload(0x40) - mstore(0x40, add(ptr, 544)) - mstore(ptr, salt) - mstore(add(ptr, 32), chainid()) - mstore(add(ptr, 64), vaultAddr) - mstore(add(ptr, 96), feePayer) - mstore(add(ptr, 128), mload(request)) - mstore(add(ptr, 160), mload(add(request, 32))) - mstore(add(ptr, 192), mload(add(request, 64))) - mstore(add(ptr, 224), mload(add(request, 96))) - mstore(add(ptr, 256), mload(add(request, 128))) - mstore(add(ptr, 288), mload(add(request, 160))) - mstore(add(ptr, 320), mload(add(request, 192))) - mstore(add(ptr, 352), mload(add(request, 224))) - mstore(add(ptr, 384), mload(add(request, 256))) - mstore(add(ptr, 416), serviceFee) - mstore(add(ptr, 448), gaslessFee) - mstore(add(ptr, 480), gaslessSponsored) - mstore(add(ptr, 512), deadline) - digest := keccak256(ptr, 544) - } - return MessageHashUtils.toEthSignedMessageHash(digest); + return EnvelopeEIP712Utils.feeAuthDigest( + vaultAddr, feePayer, request, serviceFee, gaslessFee, gaslessSponsored, deadline + ); } } \ No newline at end of file diff --git a/test/envelope/EnvelopeHardening.t.sol b/test/envelope/EnvelopeHardening.t.sol index 8a36f18c..ac2d29fc 100644 --- a/test/envelope/EnvelopeHardening.t.sol +++ b/test/envelope/EnvelopeHardening.t.sol @@ -11,6 +11,7 @@ import {EnvelopeLinks} from "../../src/envelope/EnvelopeLinks.sol"; import {ERC20Mock} from "./mocks/ERC20Mock.sol"; import {ERC721Mock} from "./mocks/ERC721Mock.sol"; import {ERC1155Mock} from "./mocks/ERC1155Mock.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; import {ERC721Holder} from "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol"; import {ERC1155Holder} from "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol"; @@ -78,30 +79,14 @@ contract EnvelopeHardeningTest is Test, ERC721Holder, ERC1155Holder { uint256 idx = nodleVault.createMFALinkFor{value: 1 wei}(address(0), 0, 1, 0, depositSigner, address(this)); // withdrawal signature (signed by deposit pubkey) - bytes32 wdHash = MessageHashUtilsLite.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - nodleVault.ENVELOPE_SALT(), - block.chainid, - address(nodleVault), - idx, - address(this), - nodleVault.OPEN_CLAIM_MODE() - ) - ) - ); + bytes32 wdHash = + EnvelopeEIP712Utils.claimDigest(address(nodleVault), idx, address(this), nodleVault.OPEN_CLAIM_MODE()); (uint8 wv, bytes32 wr, bytes32 ws) = vm.sign(depositPrivKey, wdHash); bytes memory wdSig = abi.encodePacked(wr, ws, wv); // MFA signature (signed by configured mfaAuthorizer, includes deadline) uint256 deadline = 0; // no expiry - bytes32 mfaHash = MessageHashUtilsLite.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - nodleVault.ENVELOPE_SALT(), block.chainid, address(nodleVault), idx, address(this), deadline - ) - ) - ); + bytes32 mfaHash = EnvelopeEIP712Utils.mfaDigest(address(nodleVault), idx, address(this), deadline); (uint8 mv, bytes32 mr, bytes32 ms) = vm.sign(mfaPrivKey, mfaHash); bytes memory mfaSig = abi.encodePacked(mr, ms, mv); @@ -122,15 +107,3 @@ contract EnvelopeHardeningTest is Test, ERC721Holder, ERC1155Holder { vault.claimWithMFA(idx, address(this), wdSig, mfaSig, 0); } } - -/// @dev Local copy of OZ's MessageHashUtils.toEthSignedMessageHash to avoid pulling -/// the full library into a test-only file. -library MessageHashUtilsLite { - function toEthSignedMessageHash(bytes32 messageHash) internal pure returns (bytes32 digest) { - assembly ("memory-safe") { - mstore(0x00, "\x19Ethereum Signed Message:\n32") - mstore(0x1c, messageHash) - digest := keccak256(0x00, 0x3c) - } - } -} diff --git a/test/envelope/EnvelopeSecurity.t.sol b/test/envelope/EnvelopeSecurity.t.sol index 50e7fb43..8e1cd5eb 100644 --- a/test/envelope/EnvelopeSecurity.t.sol +++ b/test/envelope/EnvelopeSecurity.t.sol @@ -12,9 +12,9 @@ pragma solidity ^0.8.26; import {Test} from "forge-std/Test.sol"; import {EnvelopeLinks} from "../../src/envelope/EnvelopeLinks.sol"; import {EnvelopeFeeAuthTestUtils} from "./EnvelopeFeeAuthTestUtils.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; import {ERC20Mock} from "./mocks/ERC20Mock.sol"; import {FeeOnTransferERC20Mock} from "./mocks/FeeOnTransferERC20Mock.sol"; -import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; contract EnvelopeSecurityTest is Test { EnvelopeLinks public vault; @@ -222,7 +222,7 @@ contract EnvelopeSecurityTest is Test { uint256 deadline = block.timestamp + 1 hours; bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( - mfaVault.ENVELOPE_SALT(), address(mfaVault), request, address(this), serviceFee, gaslessFee, false, deadline + address(mfaVault), request, address(this), serviceFee, gaslessFee, false, deadline ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(MFA_PRIV, digest); bytes memory sig = abi.encodePacked(r, s, v); @@ -244,22 +244,16 @@ contract EnvelopeSecurityTest is Test { // ══════════════════════════════════════════════════════════════════════════════ function _signOpen(address vaultAddr, uint256 idx, address recipient) internal view returns (bytes memory) { - EnvelopeLinks v = EnvelopeLinks(payable(vaultAddr)); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked(v.ENVELOPE_SALT(), block.chainid, vaultAddr, idx, recipient, v.OPEN_CLAIM_MODE()) - ) + bytes32 digest = EnvelopeEIP712Utils.claimDigest( + vaultAddr, idx, recipient, EnvelopeLinks(payable(vaultAddr)).OPEN_CLAIM_MODE() ); (uint8 vv, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); return abi.encodePacked(r, s, vv); } function _signBound(address vaultAddr, uint256 idx, address recipient) internal view returns (bytes memory) { - EnvelopeLinks v = EnvelopeLinks(payable(vaultAddr)); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked(v.ENVELOPE_SALT(), block.chainid, vaultAddr, idx, recipient, v.BOUND_CLAIM_MODE()) - ) + bytes32 digest = EnvelopeEIP712Utils.claimDigest( + vaultAddr, idx, recipient, EnvelopeLinks(payable(vaultAddr)).BOUND_CLAIM_MODE() ); (uint8 vv, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); return abi.encodePacked(r, s, vv); @@ -270,10 +264,7 @@ contract EnvelopeSecurityTest is Test { view returns (bytes memory) { - EnvelopeLinks v = EnvelopeLinks(payable(vaultAddr)); - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256(abi.encodePacked(v.ENVELOPE_SALT(), block.chainid, vaultAddr, idx, recipient, deadline)) - ); + bytes32 digest = EnvelopeEIP712Utils.mfaDigest(vaultAddr, idx, recipient, deadline); (uint8 vv, bytes32 r, bytes32 s) = vm.sign(privKey, digest); return abi.encodePacked(r, s, vv); } diff --git a/test/envelope/Gasless.t.sol b/test/envelope/Gasless.t.sol index c59098c5..551b5897 100644 --- a/test/envelope/Gasless.t.sol +++ b/test/envelope/Gasless.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; import "forge-std/Test.sol"; import "../../src/envelope/EnvelopeLinks.sol"; import {EnvelopeFeeAuthTestUtils} from "./EnvelopeFeeAuthTestUtils.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; import "./mocks/ERC20Mock.sol"; contract EnvelopeLinksGaslessTest is Test { @@ -70,7 +71,6 @@ contract EnvelopeLinksGaslessTest is Test { uint256 deadline ) internal view returns (bytes memory) { bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( - vault.ENVELOPE_SALT(), address(vault), request, feePayer, @@ -113,23 +113,13 @@ contract EnvelopeLinksGaslessTest is Test { view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked(vault.ENVELOPE_SALT(), block.chainid, address(vault), depositIndex, recipient, mode) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), depositIndex, recipient, mode); (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIVKEY, digest); return abi.encodePacked(r, s, v); } function _signMfa(uint256 depositIndex, address recipient, uint256 deadline) internal view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - vault.ENVELOPE_SALT(), block.chainid, address(vault), depositIndex, recipient, deadline - ) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.mfaDigest(address(vault), depositIndex, recipient, deadline); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BACKEND_PRIVKEY, digest); return abi.encodePacked(r, s, v); } diff --git a/test/envelope/MFA.t.sol b/test/envelope/MFA.t.sol index 51052d2e..e1d192e6 100644 --- a/test/envelope/MFA.t.sol +++ b/test/envelope/MFA.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.0; import "forge-std/Test.sol"; import "../../src/envelope/EnvelopeLinks.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; contract EnvelopeLinksMFATest is Test { EnvelopeLinks public vault; @@ -19,30 +20,14 @@ contract EnvelopeLinksMFATest is Test { } function _signMfa(uint256 depositIndex, address recipient, uint256 deadline) internal view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - vault.ENVELOPE_SALT(), block.chainid, address(vault), depositIndex, recipient, deadline - ) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.mfaDigest(address(vault), depositIndex, recipient, deadline); (uint8 v, bytes32 r, bytes32 s) = vm.sign(MFA_PRIVKEY, digest); return abi.encodePacked(r, s, v); } function _signWithdrawal(uint256 depositIndex, address recipient) internal view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - vault.ENVELOPE_SALT(), - block.chainid, - address(vault), - depositIndex, - recipient, - vault.OPEN_CLAIM_MODE() - ) - ) - ); + bytes32 digest = + EnvelopeEIP712Utils.claimDigest(address(vault), depositIndex, recipient, vault.OPEN_CLAIM_MODE()); (uint8 v, bytes32 r, bytes32 s) = vm.sign(uint256(SAMPLE_PRIVKEY), digest); return abi.encodePacked(r, s, v); } diff --git a/test/envelope/SigWithdraw.t.sol b/test/envelope/SigWithdraw.t.sol index f4e48132..f93681a6 100644 --- a/test/envelope/SigWithdraw.t.sol +++ b/test/envelope/SigWithdraw.t.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.19; import "forge-std/Test.sol"; import "../../src/envelope/EnvelopeLinks.sol"; -import "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; +import {EnvelopeEIP712Utils} from "./EnvelopeEIP712Utils.sol"; contract TestSigWithdrawEther is Test { EnvelopeLinks public vault; @@ -20,25 +20,13 @@ contract TestSigWithdrawEther is Test { } function _signOpen(uint256 idx, address recipient) internal view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, recipient, vault.OPEN_CLAIM_MODE() - ) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), idx, recipient, vault.OPEN_CLAIM_MODE()); (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); return abi.encodePacked(r, s, v); } function _signBound(uint256 idx, address recipient) internal view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - vault.ENVELOPE_SALT(), block.chainid, address(vault), idx, recipient, vault.BOUND_CLAIM_MODE() - ) - ) - ); + bytes32 digest = EnvelopeEIP712Utils.claimDigest(address(vault), idx, recipient, vault.BOUND_CLAIM_MODE()); (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIV, digest); return abi.encodePacked(r, s, v); } diff --git a/test/paymasters/EnvelopePaymaster.t.sol b/test/paymasters/EnvelopePaymaster.t.sol index 154a3e96..144e6867 100644 --- a/test/paymasters/EnvelopePaymaster.t.sol +++ b/test/paymasters/EnvelopePaymaster.t.sol @@ -10,7 +10,7 @@ import {EnvelopePaymaster} from "../../src/paymasters/EnvelopePaymaster.sol"; import {EnvelopeLinks} from "../../src/envelope/EnvelopeLinks.sol"; import {ERC20Mock} from "../envelope/mocks/ERC20Mock.sol"; import {EnvelopeFeeAuthTestUtils} from "../envelope/EnvelopeFeeAuthTestUtils.sol"; -import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; +import {EnvelopeEIP712Utils} from "../envelope/EnvelopeEIP712Utils.sol"; contract EnvelopePaymasterTest is Test { EnvelopeLinks public vault; @@ -75,7 +75,6 @@ contract EnvelopePaymasterTest is Test { uint256 deadline ) internal view returns (bytes memory) { bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( - vault.ENVELOPE_SALT(), address(vault), request, SENDER, @@ -103,18 +102,8 @@ contract EnvelopePaymasterTest is Test { } function _signWithdrawal(uint256 depositIndex, address recipient) internal view returns (bytes memory) { - bytes32 digest = MessageHashUtils.toEthSignedMessageHash( - keccak256( - abi.encodePacked( - vault.ENVELOPE_SALT(), - block.chainid, - address(vault), - depositIndex, - recipient, - vault.OPEN_CLAIM_MODE() - ) - ) - ); + bytes32 digest = + EnvelopeEIP712Utils.claimDigest(address(vault), depositIndex, recipient, vault.OPEN_CLAIM_MODE()); (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIVKEY, digest); return abi.encodePacked(r, s, v); } From 42a551620d7444f0b573eb703b814e4651e4a9e8 Mon Sep 17 00:00:00 2001 From: Alex Sedighi Date: Thu, 21 May 2026 15:48:59 +1200 Subject: [PATCH 4/5] paymaster: raise gasless attempt limit to 3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One attempt was too strict — legitimate retries (wrong gas limit, receiver contract not deployed, token paused then unpaused) would permanently consume the user's gasless quota on the first failure. MAX_GASLESS_ATTEMPTS_PER_LINK = 3 gives users room for honest retries while keeping paymaster sponsorship liability bounded. The test for GaslessAttemptLimitReached now exhausts all 3 allowed attempts before asserting the revert. --- src/envelope/doc/EnvelopePaymaster.md | 4 ++- src/paymasters/EnvelopePaymaster.sol | 30 +++++++++++++++- test/paymasters/EnvelopePaymaster.t.sol | 48 +++++++++++++++++++++---- 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/envelope/doc/EnvelopePaymaster.md b/src/envelope/doc/EnvelopePaymaster.md index bd5477eb..9d509993 100644 --- a/src/envelope/doc/EnvelopePaymaster.md +++ b/src/envelope/doc/EnvelopePaymaster.md @@ -29,7 +29,9 @@ The paymaster supports ZkSync general flow only. 5. It verifies it has enough ETH for `requiredETH`. 6. `BasePaymaster` pays the bootloader. -The paymaster does not keep per-gift state and does not price fees. Fee pricing, prepaid gasless amounts, and backend-sponsored eligibility are recorded in `EnvelopeLinks` at deposit creation. +The paymaster does not price fees. Fee pricing, prepaid gasless amounts, and backend-sponsored eligibility are recorded in `EnvelopeLinks` at deposit creation. + +The paymaster records one validation attempt per link before paying the bootloader. ZkSync runs validation and execution separately, so this attempt remains recorded even when the subsequent vault execution reverts. Up to `MAX_GASLESS_ATTEMPTS_PER_LINK` (currently **3**) attempts are allowed per link. This gives users room for honest retries (e.g. wrong gas limit, receiver contract not yet deployed) while bounding paymaster loss from repeated execution failures. Once the limit is reached, the user can still submit the vault call while paying gas themselves. ## Sponsored Selectors diff --git a/src/paymasters/EnvelopePaymaster.sol b/src/paymasters/EnvelopePaymaster.sol index 8e814bbb..90e3cbc5 100644 --- a/src/paymasters/EnvelopePaymaster.sol +++ b/src/paymasters/EnvelopePaymaster.sol @@ -9,11 +9,18 @@ import {IEnvelopeGaslessValidator} from "../envelope/IEnvelopeGaslessValidator.s /// @dev The EnvelopeLinks remains the source of truth for whether a call is valid and prepaid or sponsored. /// This paymaster only accepts general-flow transactions targeting that vault. contract EnvelopePaymaster is BasePaymaster { + uint256 public constant MAX_GASLESS_ATTEMPTS_PER_LINK = 3; + IEnvelopeGaslessValidator public immutable envelopeLinks; + mapping(uint256 => uint256) public gaslessAttemptsByLink; + error DestinationIsNotEnvelopeLinks(); error EnvelopeGaslessOperationNotApproved(); error PaymasterBalanceTooLow(); + error GaslessAttemptLimitReached(uint256 index); + + event GaslessAttemptRecorded(uint256 indexed index, uint256 indexed attempts); constructor(address admin, address withdrawer, address envelopeLinks_) BasePaymaster(admin, withdrawer) { envelopeLinks = IEnvelopeGaslessValidator(envelopeLinks_); @@ -21,7 +28,6 @@ contract EnvelopePaymaster is BasePaymaster { function _validateAndPayGeneralFlow(address from, address to, uint256 requiredETH, bytes memory transactionData) internal - view override { if (to != address(envelopeLinks)) revert DestinationIsNotEnvelopeLinks(); @@ -35,6 +41,28 @@ contract EnvelopePaymaster is BasePaymaster { if (!approved) revert EnvelopeGaslessOperationNotApproved(); if (address(this).balance < requiredETH) revert PaymasterBalanceTooLow(); + + _recordGaslessAttempt(transactionData); + } + + function _recordGaslessAttempt(bytes memory transactionData) internal { + uint256 index = _decodeGaslessLinkIndex(transactionData); + uint256 attempts = gaslessAttemptsByLink[index]; + if (attempts == MAX_GASLESS_ATTEMPTS_PER_LINK) revert GaslessAttemptLimitReached(index); + + unchecked { + ++attempts; + } + gaslessAttemptsByLink[index] = attempts; + emit GaslessAttemptRecorded(index, attempts); + } + + function _decodeGaslessLinkIndex(bytes memory transactionData) internal pure returns (uint256 index) { + if (transactionData.length < 36) revert EnvelopeGaslessOperationNotApproved(); + + assembly { + index := mload(add(transactionData, 36)) + } } function _validateAndPayApprovalBasedFlow(address, address, address, uint256, bytes memory, uint256) diff --git a/test/paymasters/EnvelopePaymaster.t.sol b/test/paymasters/EnvelopePaymaster.t.sol index 144e6867..2fe55c12 100644 --- a/test/paymasters/EnvelopePaymaster.t.sol +++ b/test/paymasters/EnvelopePaymaster.t.sol @@ -75,13 +75,7 @@ contract EnvelopePaymasterTest is Test { uint256 deadline ) internal view returns (bytes memory) { bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( - address(vault), - request, - SENDER, - serviceFee, - gaslessFee, - gaslessSponsored, - deadline + address(vault), request, SENDER, serviceFee, gaslessFee, gaslessSponsored, deadline ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BACKEND_PRIVKEY, digest); return abi.encodePacked(r, s, v); @@ -108,6 +102,13 @@ contract EnvelopePaymasterTest is Test { return abi.encodePacked(r, s, v); } + function _signBoundWithdrawal(uint256 depositIndex, address recipient) internal view returns (bytes memory) { + bytes32 digest = + EnvelopeEIP712Utils.claimDigest(address(vault), depositIndex, recipient, vault.BOUND_CLAIM_MODE()); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIVKEY, digest); + return abi.encodePacked(r, s, v); + } + function _buildTransaction(address from, address to, bytes memory data, uint256 gasLimit, uint256 maxFeePerGas) internal pure @@ -140,6 +141,39 @@ contract EnvelopePaymasterTest is Test { assertEq(magic, paymaster.validateAndPayForPaymasterTransaction.selector); assertEq(BOOTLOADER_FORMAL_ADDRESS.balance, bootloaderBalBefore + requiredETH); + assertEq(paymaster.gaslessAttemptsByLink(index), 1); + } + + function test_RevertIf_GaslessAttemptLimitReached() public { + uint256 index = _makeGaslessDeposit(1 ether); + bytes memory withdrawalSig = _signWithdrawal(index, RECIPIENT); + bytes memory data = abi.encodeCall(EnvelopeLinks.claim, (index, RECIPIENT, withdrawalSig)); + Transaction memory txn = _buildTransaction(RECIPIENT, address(vault), data, 100_000, 1 gwei); + + // Exhaust all 3 allowed attempts + for (uint256 i = 0; i < paymaster.MAX_GASLESS_ATTEMPTS_PER_LINK(); i++) { + vm.prank(BOOTLOADER_FORMAL_ADDRESS); + paymaster.validateAndPayForPaymasterTransaction(bytes32(0), bytes32(0), txn); + } + assertEq(paymaster.gaslessAttemptsByLink(index), paymaster.MAX_GASLESS_ATTEMPTS_PER_LINK()); + + vm.prank(BOOTLOADER_FORMAL_ADDRESS); + vm.expectRevert(abi.encodeWithSelector(EnvelopePaymaster.GaslessAttemptLimitReached.selector, index)); + paymaster.validateAndPayForPaymasterTransaction(bytes32(0), bytes32(0), txn); + } + + function test_RevertIf_UnboundBoundRecipientGaslessOperationNotApproved() public { + uint256 index = _makeGaslessDeposit(1 ether); + bytes memory withdrawalSig = _signBoundWithdrawal(index, RECIPIENT); + bytes memory data = abi.encodeCall(EnvelopeLinks.claimAsBoundRecipient, (index, RECIPIENT, withdrawalSig)); + Transaction memory txn = _buildTransaction(RECIPIENT, address(vault), data, 100_000, 1 gwei); + + assertFalse(vault.isValidGaslessOperation(RECIPIENT, data)); + + vm.prank(BOOTLOADER_FORMAL_ADDRESS); + vm.expectRevert(EnvelopePaymaster.EnvelopeGaslessOperationNotApproved.selector); + paymaster.validateAndPayForPaymasterTransaction(bytes32(0), bytes32(0), txn); + assertEq(paymaster.gaslessAttemptsByLink(index), 0); } function test_RevertIf_DestinationIsNotEnvelopeLinks() public { From 1b12ee5578f1c96d0763424547b37fd46888da67 Mon Sep 17 00:00:00 2001 From: Alex Sedighi Date: Thu, 21 May 2026 16:55:39 +1200 Subject: [PATCH 5/5] envelope: finalize pre-prod security hardening (H-1, M-1..M-4, L-1, L-2, L-6, L-7) Security fixes: - H-1: Reject zero claimKey only when also recipient-bound (allows recipient-only links) - M-1: Constructor + setMfaAuthorizer reject zero authorizer - M-2: Enforce no ETH fees (accumulatedFees scalar, withdrawFees no-arg) - M-3: claimWithMFA bounds-checks index, rejects non-MFA links - M-4: Batch functions early-return on empty list, revert on uneven division - L-1: Fee replay protection uses EIP712 digest - L-2/L-6/L-7: Consolidate signature recovery via _recoverSigner, add NatSpec All 220 envelope tests pass. --- src/envelope/EnvelopeLinks.sol | 116 ++++++++------ src/paymasters/EnvelopePaymaster.sol | 4 + test/envelope/Coverage.t.sol | 178 +++++---------------- test/envelope/Deposit.t.sol | 2 +- test/envelope/EnvelopeBatching.t.sol | 12 +- test/envelope/EnvelopeEIP712.t.sol | 6 +- test/envelope/EnvelopeEIP712Utils.sol | 5 +- test/envelope/EnvelopeEdgeCases.t.sol | 49 +++++- test/envelope/EnvelopeFeeAuthTestUtils.sol | 2 +- test/envelope/EnvelopeHardening.t.sol | 2 +- test/envelope/EnvelopeLinks.t.sol | 2 +- test/envelope/EnvelopeSecurity.t.sol | 58 +++++-- test/envelope/Gasless.t.sol | 12 +- test/envelope/Integration.t.sol | 2 +- test/envelope/RecipientBound.t.sol | 2 +- test/envelope/SenderWithdraw.t.sol | 8 +- test/envelope/SigWithdraw.t.sol | 2 +- 17 files changed, 223 insertions(+), 239 deletions(-) diff --git a/src/envelope/EnvelopeLinks.sol b/src/envelope/EnvelopeLinks.sol index be4c7cb1..003896c9 100644 --- a/src/envelope/EnvelopeLinks.sol +++ b/src/envelope/EnvelopeLinks.sol @@ -26,6 +26,7 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow error LinkIndexOutOfBounds(); error LinkAlreadyRedeemed(); error RequiresMfaAuthorization(); + error MfaNotRequired(); error WrongMfaSignature(); error WrongSignature(); error WrongRecipient(); @@ -48,7 +49,10 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow error ZeroRecipientAddress(); error LinkNotRecipientBound(); error FeeAuthorizationAlreadyUsed(); - error MfaAuthorizerIsZero(); + error ZeroMfaAuthorizer(); + error ZeroClaimKey(); + error UnevenBatchAmount(); + error FeeTokenTransferAmountMismatch(); // ── Data Structures ────────────────────────────────────────────────────────── @@ -124,7 +128,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow bytes32 public constant BOUND_CLAIM_MODE = 0x2bb5bef2b248d3edba501ad918c3ab524cce2aea54d4c914414e1c4401dc4ff4; // keccak256("only recipient") /// @notice Address authorized to issue MFA signatures gating claimWithMFA calls and fee authorizations. - /// @dev Rotatable by owner. Address(0) disables MFA — claimWithMFA will revert. + /// @dev Rotatable by owner. Setting to address(0) is rejected — MFA and fee-authorized creation are + /// always-on for production. Use rotation to disable a compromised key by replacing it. address public mfaAuthorizer; Link[] internal links; // array of links @@ -132,12 +137,14 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow /// @notice ERC-20 token used for Envelope service and gasless sponsorship fees (for example NODL). IERC20 public immutable feeToken; - /// @notice Accumulated fees per token address (address(0) for ETH; feeToken for link-creation fees). - mapping(address => uint256) public accumulatedFees; + /// @notice Accumulated fees in feeToken from createLinkWithFees/createCustomLinksWithFees. + /// @dev ETH fees are not supported; the protocol intentionally collects fees only in feeToken. + uint256 public accumulatedFees; - /// @notice Tracks consumed fee authorizations to prevent replay. + /// @notice Tracks consumed fee authorizations to prevent replay (keyed by the EIP-712 digest). mapping(bytes32 => bool) public usedFeeAuthorizations; + // events event LinkCreated(uint256 indexed _index, uint8 indexed _contractType, uint256 _amount, address indexed _creator); event LinkRedeemed( @@ -147,13 +154,15 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow event FeesWithdrawn(address indexed tokenAddress, uint256 amount); event MfaAuthorizerUpdated(address indexed oldAuthorizer, address indexed newAuthorizer); - /// @param _mfaAuthorizer address authorized to sign backend fee and MFA approvals (use address(0) to disable). + /// @param _mfaAuthorizer address authorized to sign backend fee and MFA approvals. Must be non-zero; + /// this single key gates both MFA-protected claims and fee authorizations. /// @param _owner initial owner of the contract (receives accumulated fees). /// @param _feeToken ERC-20 token used for fees; address(0) disables non-zero fee authorizations. constructor(address _mfaAuthorizer, address _owner, address _feeToken) Ownable(_owner) EIP712("EnvelopeLinks", "5") { + if (_mfaAuthorizer == address(0)) revert ZeroMfaAuthorizer(); mfaAuthorizer = _mfaAuthorizer; feeToken = IERC20(_feeToken); } @@ -304,6 +313,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow /// @notice Create many same-shape links in one transaction. /// @dev The caller remains the recorded sender for every deposit and keeps reclaim rights. /// ERC-721 is intentionally excluded here because each NFT needs a distinct tokenId. + /// Reverts if the actually-received total is not evenly divisible across all links to + /// prevent silent dust loss when fee-on-transfer tokens are used. function createLinks( address _tokenAddress, uint8 _contractType, @@ -311,9 +322,11 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow uint256 _tokenId, address[] calldata _claimKeys ) external payable nonReentrant returns (uint256[] memory) { + if (_claimKeys.length == 0) return new uint256[](0); uint256 totalAmount = _amount * _claimKeys.length; uint256 actualTotal = _pullUniformBatchAssets(msg.sender, _tokenAddress, _contractType, totalAmount, _tokenId); - uint256 perLinkAmount = _claimKeys.length > 0 ? actualTotal / _claimKeys.length : 0; + if (actualTotal % _claimKeys.length != 0) revert UnevenBatchAmount(); + uint256 perLinkAmount = actualTotal / _claimKeys.length; uint256[] memory linkIndexes = new uint256[](_claimKeys.length); for (uint256 i = 0; i < _claimKeys.length; ++i) { @@ -343,9 +356,11 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow uint256 _tokenId, address[] calldata _claimKeys ) external payable nonReentrant { + if (_claimKeys.length == 0) return; uint256 totalAmount = _amount * _claimKeys.length; uint256 actualTotal = _pullUniformBatchAssets(msg.sender, _tokenAddress, _contractType, totalAmount, _tokenId); - uint256 perLinkAmount = _claimKeys.length > 0 ? actualTotal / _claimKeys.length : 0; + if (actualTotal % _claimKeys.length != 0) revert UnevenBatchAmount(); + uint256 perLinkAmount = actualTotal / _claimKeys.length; for (uint256 i = 0; i < _claimKeys.length; ++i) { _storeLink( @@ -461,6 +476,7 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow /** * @notice Withdraw tokens with backend MFA approval. + * @dev Reverts if the target link does not require MFA; use plain {claim} for non-MFA links. * @param _index deposit index * @param _recipientAddress address to receive the full deposit amount * @param _signature withdrawal signature from the link's claimKey @@ -474,12 +490,17 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow bytes memory _MFASignature, uint256 _deadline ) external nonReentrant returns (bool) { + if (_index >= links.length) revert LinkIndexOutOfBounds(); + if (!links[_index].status.requiresMFA) revert MfaNotRequired(); _verifyMfaSignature(_index, _recipientAddress, _deadline, _MFASignature); return _executeClaim(_index, _recipientAddress, OPEN_CLAIM_MODE, _signature, true); } /** - * @notice Withdraw tokens. Must be called by the recipient. + * @notice Withdraw tokens from a recipient-bound link directly by the recipient. + * @dev Bound links can also be claimed via plain {claim} when the caller has a claimKey signature + * over OPEN_CLAIM_MODE and the recipient matches. This entry uses BOUND_CLAIM_MODE so a + * bound-mode signature cannot be reused as an open-mode signature and vice versa. */ function claimAsBoundRecipient(uint256 _index, address _recipientAddress, bytes memory _signature) external @@ -543,30 +564,27 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow /** * @notice Update the MFA authorizer address. Only callable by owner. - * @param _newAuthorizer new MFA signer address (address(0) disables MFA). + * @dev Reverts on address(0) — the protocol requires an always-set authorizer for MFA claims + * and fee-authorized creation. To replace a compromised key, rotate to a new non-zero address. + * @param _newAuthorizer new MFA signer address. */ function setMfaAuthorizer(address _newAuthorizer) external onlyOwner { + if (_newAuthorizer == address(0)) revert ZeroMfaAuthorizer(); emit MfaAuthorizerUpdated(mfaAuthorizer, _newAuthorizer); mfaAuthorizer = _newAuthorizer; } /** - * @notice Withdraw accumulated fees for a given token. Only callable by owner. - * @param _tokenAddress token to withdraw fees for (address(0) for ETH) + * @notice Withdraw accumulated feeToken fees to the caller (owner). ETH fees are not supported. */ - function withdrawFees(address _tokenAddress) external onlyOwner nonReentrant { - uint256 amount = accumulatedFees[_tokenAddress]; + function withdrawFees() external onlyOwner nonReentrant { + uint256 amount = accumulatedFees; if (amount == 0) revert NoFeesToWithdraw(); - accumulatedFees[_tokenAddress] = 0; + accumulatedFees = 0; - if (_tokenAddress == address(0)) { - (bool success,) = msg.sender.call{value: amount}(""); - if (!success) revert EthTransferFailed(); - } else { - IERC20(_tokenAddress).safeTransfer(msg.sender, amount); - } + feeToken.safeTransfer(msg.sender, amount); - emit FeesWithdrawn(_tokenAddress, amount); + emit FeesWithdrawn(address(feeToken), amount); } // ══════════════════════════════════════════════════════════════════════════════ @@ -594,6 +612,7 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow if (selector == this.claimAsBoundRecipient.selector) { (uint256 index, address recipient, bytes memory signature) = abi.decode(callData[4:], (uint256, address, bytes)); + if (!_isRecipientBoundLink(index)) return false; return _isValidGaslessClaim(caller, index, recipient, BOUND_CLAIM_MODE, signature, false); } @@ -668,15 +687,12 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow uint256 _deadline, bytes memory _MFASignature ) internal view { - if (mfaAuthorizer == address(0)) revert MfaAuthorizerIsZero(); // deadline == 0 means no expiry if (_deadline != 0 && block.timestamp > _deadline) revert MfaSignatureExpired(); - bytes32 digest = _hashTypedDataV4( - keccak256(abi.encode(MFA_APPROVAL_TYPEHASH, _index, _recipientAddress, _deadline)) - ); - address authorizationSigner = getSigner(digest, _MFASignature); - if (authorizationSigner != mfaAuthorizer) revert WrongMfaSignature(); + bytes32 digest = + _hashTypedDataV4(keccak256(abi.encode(MFA_APPROVAL_TYPEHASH, _index, _recipientAddress, _deadline))); + if (_recoverSigner(digest, _MFASignature) != mfaAuthorizer) revert WrongMfaSignature(); } function _isMfaSignatureValid(uint256 _index, address _recipientAddress, uint256 _deadline, bytes memory _signature) @@ -684,12 +700,10 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow view returns (bool) { - if (mfaAuthorizer == address(0)) return false; if (_deadline != 0 && block.timestamp > _deadline) return false; - bytes32 digest = _hashTypedDataV4( - keccak256(abi.encode(MFA_APPROVAL_TYPEHASH, _index, _recipientAddress, _deadline)) - ); + bytes32 digest = + _hashTypedDataV4(keccak256(abi.encode(MFA_APPROVAL_TYPEHASH, _index, _recipientAddress, _deadline))); return _recoverSigner(digest, _signature) == mfaAuthorizer; } @@ -705,13 +719,14 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow bytes32 digest = _feeAuthorizationDigest(_request, _feeAuthorization, msg.sender); - // Replay protection: mark this authorization as consumed. - bytes32 authHash = keccak256(_feeAuthorization.signature); - if (usedFeeAuthorizations[authHash]) revert FeeAuthorizationAlreadyUsed(); - usedFeeAuthorizations[authHash] = true; + // Replay protection keyed by the EIP-712 digest: each (intent, feePayer, deadline) tuple may be + // consumed exactly once, regardless of the on-the-wire signature encoding. + if (usedFeeAuthorizations[digest]) revert FeeAuthorizationAlreadyUsed(); + usedFeeAuthorizations[digest] = true; - address authorizationSigner = getSigner(digest, _feeAuthorization.signature); - if (authorizationSigner != mfaAuthorizer) revert WrongFeeAuthorizationSignature(); + if (_recoverSigner(digest, _feeAuthorization.signature) != mfaAuthorizer) { + revert WrongFeeAuthorizationSignature(); + } } function _feeAuthorizationDigest( @@ -746,8 +761,11 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow uint256 totalFee = _serviceFee + _gaslessFee; if (totalFee > 0) { address tokenAddress = address(feeToken); + uint256 balanceBefore = feeToken.balanceOf(address(this)); feeToken.safeTransferFrom(_feePayer, address(this), totalFee); - accumulatedFees[tokenAddress] += totalFee; + uint256 actualReceived = feeToken.balanceOf(address(this)) - balanceBefore; + if (actualReceived != totalFee) revert FeeTokenTransferAmountMismatch(); + accumulatedFees += totalFee; emit FeeCollected(_index, tokenAddress, _serviceFee, _gaslessFee); } } @@ -766,6 +784,10 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow uint256 _gaslessFee, bool _gaslessSponsored ) internal returns (uint256) { + // A link must be claimable: either via a claim-key signature, or by a bound recipient. + // Rejecting `claimKey == 0 && recipient == 0` prevents accidentally creating an + // unbound link that anyone could drain with an empty signature. + if (claimKey == address(0) && _recipient == address(0)) revert ZeroClaimKey(); uint256 index = links.length; links.push(); @@ -805,6 +827,10 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow return _isValidClaim(_index, _recipientAddress, _extraData, _signature, _authorized); } + function _isRecipientBoundLink(uint256 _index) internal view returns (bool) { + return _index < links.length && links[_index].parties.recipient != address(0); + } + function _isValidClaim( uint256 _index, address _recipientAddress, @@ -819,9 +845,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow if (deposit.parties.recipient != address(0) && _recipientAddress != deposit.parties.recipient) return false; if (deposit.status.claimKey != address(0)) { - bytes32 _claimHash = _hashTypedDataV4( - keccak256(abi.encode(CLAIM_TYPEHASH, _index, _recipientAddress, _extraData)) - ); + bytes32 _claimHash = + _hashTypedDataV4(keccak256(abi.encode(CLAIM_TYPEHASH, _index, _recipientAddress, _extraData))); if (_recoverSigner(_claimHash, _signature) != deposit.status.claimKey) return false; } @@ -1016,6 +1041,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow if (_contractType == 0) { if (_amount != _ethAmount) revert WrongEthAmount(); + } else if (_ethAmount != 0) { + revert EthNotAcceptedForNonEthLink(); } else if (_contractType == 1) { uint256 balanceBefore = IERC20(_tokenAddress).balanceOf(address(this)); IERC20(_tokenAddress).safeTransferFrom(_from, address(this), _amount); @@ -1044,9 +1071,8 @@ contract EnvelopeLinks is IERC721Receiver, IERC1155Receiver, ReentrancyGuard, Ow address claimSigner; if (_signature.length > 0) { - bytes32 _claimHash = _hashTypedDataV4( - keccak256(abi.encode(CLAIM_TYPEHASH, _index, _recipientAddress, _extraData)) - ); + bytes32 _claimHash = + _hashTypedDataV4(keccak256(abi.encode(CLAIM_TYPEHASH, _index, _recipientAddress, _extraData))); claimSigner = getSigner(_claimHash, _signature); } if (link.status.requiresMFA && !_authorized) revert RequiresMfaAuthorization(); diff --git a/src/paymasters/EnvelopePaymaster.sol b/src/paymasters/EnvelopePaymaster.sol index 90e3cbc5..ea7c45db 100644 --- a/src/paymasters/EnvelopePaymaster.sol +++ b/src/paymasters/EnvelopePaymaster.sol @@ -57,6 +57,10 @@ contract EnvelopePaymaster is BasePaymaster { emit GaslessAttemptRecorded(index, attempts); } + /// @dev Reads the first uint256 calldata argument out of `transactionData` (the link index). + /// Safe because this is only called after `isValidGaslessOperation` matched one of the + /// claim/reclaim selectors, all of which have `uint256 _index` as their first parameter. + /// Offset 36 = 32 (bytes-memory length prefix) + 4 (function selector). function _decodeGaslessLinkIndex(bytes memory transactionData) internal pure returns (uint256 index) { if (transactionData.length < 36) revert EnvelopeGaslessOperationNotApproved(); diff --git a/test/envelope/Coverage.t.sol b/test/envelope/Coverage.t.sol index aad15154..557bf36d 100644 --- a/test/envelope/Coverage.t.sol +++ b/test/envelope/Coverage.t.sol @@ -217,24 +217,12 @@ contract EnvelopeCoverageTest is Test { } // ══════════════════════════════════════════════════════════════════════════════ - // withdrawFees — ETH accumulated fees + // withdrawFees — ERC-20 (feeToken) only; ETH fees are not supported // ══════════════════════════════════════════════════════════════════════════════ - function test_withdrawFees_eth() public { - // Seed accumulatedFees[address(0)] with ETH balance (slot 7 for accumulatedFees mapping) - bytes32 slot = keccak256(abi.encode(address(0), uint256(7))); - vm.store(address(vault), slot, bytes32(uint256(0.5 ether))); - vm.deal(address(vault), 0.5 ether); - - uint256 ownerBalBefore = address(this).balance; - vault.withdrawFees(address(0)); - assertEq(address(this).balance, ownerBalBefore + 0.5 ether); - assertEq(vault.accumulatedFees(address(0)), 0); - } - function test_RevertIf_withdrawFees_noFees() public { vm.expectRevert(EnvelopeLinks.NoFeesToWithdraw.selector); - vault.withdrawFees(address(feeToken)); + vault.withdrawFees(); } function test_withdrawFees_erc20() public { @@ -252,26 +240,22 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0.1 ether, 0.05 ether, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0.1 ether, - gaslessFee: 0.05 ether, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0.1 ether, gaslessFee: 0.05 ether, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); vault.createLinkWithFees{value: 1 ether}(req, auth); uint256 ownerBalBefore = feeToken.balanceOf(address(this)); - vault.withdrawFees(address(feeToken)); + vault.withdrawFees(); assertEq(feeToken.balanceOf(address(this)), ownerBalBefore + 0.15 ether); - assertEq(vault.accumulatedFees(address(feeToken)), 0); + assertEq(vault.accumulatedFees(), 0); } function test_RevertIf_withdrawFees_nonOwner() public { vm.prank(OTHER); vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, OTHER)); - vault.withdrawFees(address(feeToken)); + vault.withdrawFees(); } // ══════════════════════════════════════════════════════════════════════════════ @@ -303,7 +287,11 @@ contract EnvelopeCoverageTest is Test { } function test_isValidGaslessOperation_unknownSelector() public view { - assertFalse(vault.isValidGaslessOperation(RECIPIENT, hex"deadbeef0000000000000000000000000000000000000000000000000000000000000000")); + assertFalse( + vault.isValidGaslessOperation( + RECIPIENT, hex"deadbeef0000000000000000000000000000000000000000000000000000000000000000" + ) + ); } function test_isValidGaslessOperation_reclaim_indexOutOfBounds() public view { @@ -342,11 +330,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0, 0.01 ether, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0, - gaslessFee: 0.01 ether, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0, gaslessFee: 0.01 ether, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); vault.createLinkWithFees{value: 1 ether}(req, auth); @@ -401,11 +385,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0, 0.01 ether, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0, - gaslessFee: 0.01 ether, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0, gaslessFee: 0.01 ether, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); uint256 idx = vault.createLinkWithFees{value: 1 ether}(req, auth); @@ -462,7 +442,8 @@ contract EnvelopeCoverageTest is Test { bool[] memory mfas = new bool[](2); vm.prank(SENDER); - uint256[] memory indexes = vault.createCustomLinks{value: 0.5 ether}(tokens, types, amounts, tokenIds, keys, mfas); + uint256[] memory indexes = + vault.createCustomLinks{value: 0.5 ether}(tokens, types, amounts, tokenIds, keys, mfas); assertEq(indexes.length, 2); assertEq(vault.getLinkAsset(indexes[0]).amount, 0.5 ether); @@ -538,7 +519,7 @@ contract EnvelopeCoverageTest is Test { assertEq(vault.getLinkAsset(indexes[0]).amount, 1 ether); assertEq(vault.getLinkAsset(indexes[1]).amount, 50); assertEq(vault.getLinkParties(indexes[1]).recipient, RECIPIENT); - assertEq(vault.accumulatedFees(address(feeToken)), 0.04 ether); + assertEq(vault.accumulatedFees(), 0.04 ether); } function test_RevertIf_createCustomLinksWithFees_lengthMismatch() public { @@ -565,11 +546,7 @@ contract EnvelopeCoverageTest is Test { }); EnvelopeLinks.FeeAuthorization[] memory auths = new EnvelopeLinks.FeeAuthorization[](1); auths[0] = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0, - gaslessFee: 0, - gaslessSponsored: false, - deadline: 0, - signature: "" + serviceFee: 0, gaslessFee: 0, gaslessSponsored: false, deadline: 0, signature: "" }); vm.prank(SENDER); @@ -592,11 +569,7 @@ contract EnvelopeCoverageTest is Test { }); EnvelopeLinks.FeeAuthorization[] memory auths = new EnvelopeLinks.FeeAuthorization[](1); auths[0] = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0, - gaslessFee: 0, - gaslessSponsored: false, - deadline: 0, - signature: "" + serviceFee: 0, gaslessFee: 0, gaslessSponsored: false, deadline: 0, signature: "" }); vm.prank(SENDER); @@ -748,11 +721,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0.01 ether, 0, false, deadline); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0.01 ether, - gaslessFee: 0, - gaslessSponsored: false, - deadline: deadline, - signature: authSig + serviceFee: 0.01 ether, gaslessFee: 0, gaslessSponsored: false, deadline: deadline, signature: authSig }); vm.warp(deadline + 1); @@ -815,17 +784,13 @@ contract EnvelopeCoverageTest is Test { } // ══════════════════════════════════════════════════════════════════════════════ - // Claim with empty signature (claimKey == address(0)) + // H-1: createLink with zero claimKey must revert // ══════════════════════════════════════════════════════════════════════════════ - function test_claim_noClaimKey() public { - // Create a link with claimKey = address(0) — anyone can claim without signature + function test_RevertIf_createWithZeroClaimKey() public { vm.prank(SENDER); - uint256 idx = vault.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, address(0)); - - uint256 balBefore = RECIPIENT.balance; - vault.claim(idx, RECIPIENT, ""); - assertEq(RECIPIENT.balance, balBefore + 1 ether); + vm.expectRevert(EnvelopeLinks.ZeroClaimKey.selector); + vault.createLink{value: 1 ether}(address(0), 0, 1 ether, 0, address(0)); } // ══════════════════════════════════════════════════════════════════════════════ @@ -946,11 +911,7 @@ contract EnvelopeCoverageTest is Test { bytes memory authSig = _signFeeAuthForVault(address(vaultNoFeeToken), req, SENDER, 0.01 ether, 0, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0.01 ether, - gaslessFee: 0, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0.01 ether, gaslessFee: 0, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); @@ -1022,11 +983,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0, 0.01 ether, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0, - gaslessFee: 0.01 ether, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0, gaslessFee: 0.01 ether, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); return vault.createLinkWithFees{value: amount}(req, auth); @@ -1046,11 +1003,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0, 0.01 ether, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0, - gaslessFee: 0.01 ether, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0, gaslessFee: 0.01 ether, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); return vault.createLinkWithFees{value: amount}(req, auth); @@ -1070,41 +1023,12 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0, 0, true, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0, - gaslessFee: 0, - gaslessSponsored: true, - deadline: 0, - signature: authSig + serviceFee: 0, gaslessFee: 0, gaslessSponsored: true, deadline: 0, signature: authSig }); vm.prank(SENDER); return vault.createLinkWithFees{value: amount}(req, auth); } - // ══════════════════════════════════════════════════════════════════════════════ - // withdrawFees — ETH path where owner contract rejects the transfer - // Scenario: Owner is a multisig/governance contract that cannot receive ETH - // ══════════════════════════════════════════════════════════════════════════════ - - function test_RevertIf_withdrawFees_ethRejected() public { - // Deploy a vault owned by a contract that rejects ETH - EthRejecter rejecter = new EthRejecter(); - EnvelopeLinks rejVault = new EnvelopeLinks(BACKEND_AUTHORIZER, address(rejecter), address(feeToken)); - - // Create a link with ETH service fees so that ETH accumulates in the vault - // Since fees are ERC-20 (feeToken), we need to get ETH into accumulatedFees. - // withdrawFees(address(0)) withdraws ETH accumulated fees. - // Seed directly: we can call withdrawFees with ETH balance. - vm.deal(address(rejVault), 1 ether); - // Write to the accumulatedFees[address(0)] storage slot - // accumulatedFees is at storage slot 7 in the contract layout - bytes32 slot = keccak256(abi.encode(address(0), uint256(7))); - vm.store(address(rejVault), slot, bytes32(uint256(1 ether))); - - vm.prank(address(rejecter)); - vm.expectRevert(EnvelopeLinks.EthTransferFailed.selector); - rejVault.withdrawFees(address(0)); - } - // ══════════════════════════════════════════════════════════════════════════════ // Claim ERC-721 and ERC-1155 via createCustomLinksWithFees // Scenario: Backend-authorized fee links for NFTs — full lifecycle @@ -1124,11 +1048,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0.05 ether, 0, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0.05 ether, - gaslessFee: 0, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0.05 ether, gaslessFee: 0, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); @@ -1155,11 +1075,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0.02 ether, 0, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0.02 ether, - gaslessFee: 0, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0.02 ether, gaslessFee: 0, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); @@ -1277,7 +1193,8 @@ contract EnvelopeCoverageTest is Test { (uint8 v, bytes32 r, bytes32 s) = vm.sign(LINK_PRIVKEY, digest); bytes memory wrongMfaSig = abi.encodePacked(r, s, v); - bytes memory data = abi.encodeCall(EnvelopeLinks.claimWithMFA, (idx, RECIPIENT, claimSig, wrongMfaSig, deadline)); + bytes memory data = + abi.encodeCall(EnvelopeLinks.claimWithMFA, (idx, RECIPIENT, claimSig, wrongMfaSig, deadline)); assertFalse(vault.isValidGaslessOperation(RECIPIENT, data)); } @@ -1344,11 +1261,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0, 0.01 ether, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0, - gaslessFee: 0.01 ether, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0, gaslessFee: 0.01 ether, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); uint256 idx = vault.createLinkWithFees{value: 1 ether}(req, auth); @@ -1422,11 +1335,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0, 0, true, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0, - gaslessFee: 0, - gaslessSponsored: true, - deadline: 0, - signature: authSig + serviceFee: 0, gaslessFee: 0, gaslessSponsored: true, deadline: 0, signature: authSig }); vm.prank(SENDER); uint256 idx = vault.createLinkWithFees{value: 1 ether}(req, auth); @@ -1459,11 +1368,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0.01 ether, 0, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0.01 ether, - gaslessFee: 0, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0.01 ether, gaslessFee: 0, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); @@ -1489,11 +1394,7 @@ contract EnvelopeCoverageTest is Test { }); bytes memory authSig = _signFeeAuth(req, SENDER, 0.01 ether, 0, false, 0); EnvelopeLinks.FeeAuthorization memory auth = EnvelopeLinks.FeeAuthorization({ - serviceFee: 0.01 ether, - gaslessFee: 0, - gaslessSponsored: false, - deadline: 0, - signature: authSig + serviceFee: 0.01 ether, gaslessFee: 0, gaslessSponsored: false, deadline: 0, signature: authSig }); vm.prank(SENDER); @@ -1668,9 +1569,8 @@ contract EnvelopeCoverageTest is Test { mfas[1] = false; vm.prank(SENDER); - uint256[] memory indexes = vault.createCustomLinks{value: 0.5 ether}( - tokenAddresses, types, amounts, tokenIds, keys, mfas - ); + uint256[] memory indexes = + vault.createCustomLinks{value: 0.5 ether}(tokenAddresses, types, amounts, tokenIds, keys, mfas); assertEq(indexes.length, 2); assertEq(vault.getLinkAsset(indexes[0]).contractType, 0); @@ -1686,9 +1586,7 @@ contract EnvelopeCoverageTest is Test { function test_onERC1155BatchReceived_internalTransfer() public { // The onERC1155BatchReceived success path is only reachable when operator == vault address. // We can call it directly to verify the selector is returned. - bytes4 result = vault.onERC1155BatchReceived( - address(vault), SENDER, new uint256[](1), new uint256[](1), "" - ); + bytes4 result = vault.onERC1155BatchReceived(address(vault), SENDER, new uint256[](1), new uint256[](1), ""); assertEq(result, vault.onERC1155BatchReceived.selector); } } diff --git a/test/envelope/Deposit.t.sol b/test/envelope/Deposit.t.sol index c8b9f23d..a6a2e07d 100644 --- a/test/envelope/Deposit.t.sol +++ b/test/envelope/Deposit.t.sol @@ -25,7 +25,7 @@ contract EnvelopeLinksDepositTest is Test, ERC1155Holder, ERC721Holder { function setUp() public { console.log("Setting up test"); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); testToken = new ERC20Mock(); testToken721 = new ERC721Mock(); testToken1155 = new ERC1155Mock(); diff --git a/test/envelope/EnvelopeBatching.t.sol b/test/envelope/EnvelopeBatching.t.sol index 8bfbb930..7da81fcf 100644 --- a/test/envelope/EnvelopeBatching.t.sol +++ b/test/envelope/EnvelopeBatching.t.sol @@ -30,7 +30,7 @@ contract EnvelopeBatchingTest is Test, ERC1155Holder, ERC721Holder { linkPubKey = vm.addr(LINK_PRIVKEY); backendAuthorizer = vm.addr(BACKEND_PRIVKEY); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); testToken = new ERC20Mock(); feeToken = new ERC20Mock(); feeVault = new EnvelopeLinks(backendAuthorizer, address(this), address(feeToken)); @@ -182,7 +182,7 @@ contract EnvelopeBatchingTest is Test, ERC1155Holder, ERC721Holder { assertEq(secondStatus.requiresMFA, true); assertEq(secondParties.recipient, RECIPIENT); assertEq(feeToken.balanceOf(address(feeVault)), 0.1 ether); - assertEq(feeVault.accumulatedFees(address(feeToken)), 0.1 ether); + assertEq(feeVault.accumulatedFees(), 0.1 ether); bytes memory withdrawalSig = _signWithdrawal(feeVault, depositIndexes[0], RECIPIENT, feeVault.OPEN_CLAIM_MODE()); bytes memory callData = abi.encodeCall(EnvelopeLinks.claim, (depositIndexes[0], RECIPIENT, withdrawalSig)); @@ -411,13 +411,7 @@ contract EnvelopeBatchingTest is Test, ERC1155Holder, ERC721Holder { uint256 deadline ) internal view returns (bytes memory) { bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( - address(targetVault), - request, - feePayer, - serviceFee, - gaslessFee, - gaslessSponsored, - deadline + address(targetVault), request, feePayer, serviceFee, gaslessFee, gaslessSponsored, deadline ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BACKEND_PRIVKEY, digest); return abi.encodePacked(r, s, v); diff --git a/test/envelope/EnvelopeEIP712.t.sol b/test/envelope/EnvelopeEIP712.t.sol index b4b396c7..f10cae61 100644 --- a/test/envelope/EnvelopeEIP712.t.sol +++ b/test/envelope/EnvelopeEIP712.t.sol @@ -218,11 +218,7 @@ contract EnvelopeEIP712Test is Test { bytes memory sig = abi.encodePacked(r, s, v); EnvelopeLinks.FeeAuthorization memory feeAuth = EnvelopeLinks.FeeAuthorization({ - serviceFee: serviceFee, - gaslessFee: 0, - gaslessSponsored: false, - deadline: deadline, - signature: sig + serviceFee: serviceFee, gaslessFee: 0, gaslessSponsored: false, deadline: deadline, signature: sig }); // Works on vault1 diff --git a/test/envelope/EnvelopeEIP712Utils.sol b/test/envelope/EnvelopeEIP712Utils.sol index 9a496642..e2a5eafb 100644 --- a/test/envelope/EnvelopeEIP712Utils.sol +++ b/test/envelope/EnvelopeEIP712Utils.sol @@ -30,8 +30,9 @@ library EnvelopeEIP712Utils { view returns (bytes32) { - bytes32 structHash = - keccak256(abi.encode(EnvelopeLinks(payable(vaultAddr)).MFA_APPROVAL_TYPEHASH(), index, recipient, deadline)); + bytes32 structHash = keccak256( + abi.encode(EnvelopeLinks(payable(vaultAddr)).MFA_APPROVAL_TYPEHASH(), index, recipient, deadline) + ); return _hashTypedData(vaultAddr, structHash); } diff --git a/test/envelope/EnvelopeEdgeCases.t.sol b/test/envelope/EnvelopeEdgeCases.t.sol index fd325fef..7f491df7 100644 --- a/test/envelope/EnvelopeEdgeCases.t.sol +++ b/test/envelope/EnvelopeEdgeCases.t.sol @@ -61,7 +61,7 @@ contract EnvelopeEdgeCasesTest is Test, ERC721Holder, ERC1155Holder { function setUp() public { LINK_PUBKEY20 = vm.addr(LINK_PRIV); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); erc20 = new ERC20Mock(); erc721 = new ERC721Mock(); erc1155 = new ERC1155Mock(); @@ -103,6 +103,53 @@ contract EnvelopeEdgeCasesTest is Test, ERC721Holder, ERC1155Holder { vault.createLink(address(erc721), 2, 2, 1, LINK_PUBKEY20); } + function test_RevertWhen_SingleErc20DepositReceivesEth() public { + erc20.mint(address(this), 100); + erc20.approve(address(vault), 100); + + vm.expectRevert(EnvelopeLinks.EthNotAcceptedForNonEthLink.selector); + vault.createLink{value: 1 wei}(address(erc20), 1, 100, 0, LINK_PUBKEY20); + } + + function test_RevertWhen_SingleErc721DepositReceivesEth() public { + erc721.mint(address(this), 1); + erc721.approve(address(vault), 1); + + vm.expectRevert(EnvelopeLinks.EthNotAcceptedForNonEthLink.selector); + vault.createLink{value: 1 wei}(address(erc721), 2, 1, 1, LINK_PUBKEY20); + } + + function test_RevertWhen_SingleErc1155DepositReceivesEth() public { + erc1155.mint(address(this), 1, 100, ""); + erc1155.setApprovalForAll(address(vault), true); + + vm.expectRevert(EnvelopeLinks.EthNotAcceptedForNonEthLink.selector); + vault.createLink{value: 1 wei}(address(erc1155), 3, 100, 1, LINK_PUBKEY20); + } + + function test_RevertWhen_CreateLinkWithFeesNonEthDepositReceivesEth() public { + erc20.mint(address(this), 100); + erc20.approve(address(vault), 100); + + EnvelopeLinks.LinkRequest memory request = EnvelopeLinks.LinkRequest({ + tokenAddress: address(erc20), + contractType: 1, + amount: 100, + tokenId: 0, + claimKey: LINK_PUBKEY20, + onBehalfOf: address(this), + withMFA: false, + recipient: address(0), + reclaimableAfter: 0 + }); + EnvelopeLinks.FeeAuthorization memory authorization = EnvelopeLinks.FeeAuthorization({ + serviceFee: 0, gaslessFee: 0, gaslessSponsored: false, deadline: 0, signature: "" + }); + + vm.expectRevert(EnvelopeLinks.EthNotAcceptedForNonEthLink.selector); + vault.createLinkWithFees{value: 1 wei}(request, authorization); + } + // ── EnvelopeLinks withdraw input validation ───────────────────────────────── function test_RevertWhen_WithdrawIndexOutOfBounds() public { diff --git a/test/envelope/EnvelopeFeeAuthTestUtils.sol b/test/envelope/EnvelopeFeeAuthTestUtils.sol index 5a3059f5..04a65ff6 100644 --- a/test/envelope/EnvelopeFeeAuthTestUtils.sol +++ b/test/envelope/EnvelopeFeeAuthTestUtils.sol @@ -18,4 +18,4 @@ library EnvelopeFeeAuthTestUtils { vaultAddr, feePayer, request, serviceFee, gaslessFee, gaslessSponsored, deadline ); } -} \ No newline at end of file +} diff --git a/test/envelope/EnvelopeHardening.t.sol b/test/envelope/EnvelopeHardening.t.sol index ac2d29fc..9f1cb1f2 100644 --- a/test/envelope/EnvelopeHardening.t.sol +++ b/test/envelope/EnvelopeHardening.t.sol @@ -24,7 +24,7 @@ contract EnvelopeHardeningTest is Test, ERC721Holder, ERC1155Holder { address constant PUBKEY20 = address(0xaBC5211D86a01c2dD50797ba7B5b32e3C1167F9f); function setUp() public { - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); erc721 = new ERC721Mock(); erc1155 = new ERC1155Mock(); } diff --git a/test/envelope/EnvelopeLinks.t.sol b/test/envelope/EnvelopeLinks.t.sol index d695bed0..fd7f225e 100644 --- a/test/envelope/EnvelopeLinks.t.sol +++ b/test/envelope/EnvelopeLinks.t.sol @@ -30,7 +30,7 @@ contract EnvelopeLinksTest is Test { testToken = new ERC20Mock(); testToken721 = new ERC721Mock(); testToken1155 = new ERC1155Mock(); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); // Mint tokens for test accounts testToken.mint(address(this), 1000); diff --git a/test/envelope/EnvelopeSecurity.t.sol b/test/envelope/EnvelopeSecurity.t.sol index 8e1cd5eb..162dfd34 100644 --- a/test/envelope/EnvelopeSecurity.t.sol +++ b/test/envelope/EnvelopeSecurity.t.sol @@ -37,7 +37,7 @@ contract EnvelopeSecurityTest is Test { feeToken = new ERC20Mock(); fotToken = new FeeOnTransferERC20Mock(); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); mfaVault = new EnvelopeLinks(mfaSigner, address(this), address(feeToken)); } @@ -90,6 +90,40 @@ contract EnvelopeSecurityTest is Test { vault.createRaffleLinks(address(fotToken), 1, amounts, linkPubKey); } + function test_H1_feeTokenMustTransferExactAmount() public { + EnvelopeLinks fotFeeVault = new EnvelopeLinks(mfaSigner, address(this), address(fotToken)); + fotToken.mint(address(this), 10000); + fotToken.approve(address(fotFeeVault), 10000); + + EnvelopeLinks.LinkRequest memory request = EnvelopeLinks.LinkRequest({ + tokenAddress: address(0), + contractType: 0, + amount: 0.1 ether, + tokenId: 0, + claimKey: linkPubKey, + onBehalfOf: address(this), + withMFA: false, + recipient: address(0), + reclaimableAfter: 0 + }); + + uint256 serviceFee = 1000; + bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( + address(fotFeeVault), request, address(this), serviceFee, 0, false, 0 + ); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(MFA_PRIV, digest); + EnvelopeLinks.FeeAuthorization memory feeAuth = EnvelopeLinks.FeeAuthorization({ + serviceFee: serviceFee, + gaslessFee: 0, + gaslessSponsored: false, + deadline: 0, + signature: abi.encodePacked(r, s, v) + }); + + vm.expectRevert(EnvelopeLinks.FeeTokenTransferAmountMismatch.selector); + fotFeeVault.createLinkWithFees{value: 0.1 ether}(request, feeAuth); + } + // ══════════════════════════════════════════════════════════════════════════════ // H-2: Mutable mfaAuthorizer (key rotation) // ══════════════════════════════════════════════════════════════════════════════ @@ -133,24 +167,14 @@ contract EnvelopeSecurityTest is Test { // M-1: Guard against mfaAuthorizer == address(0) // ══════════════════════════════════════════════════════════════════════════════ - function test_M1_claimWithMfaRevertsWhenAuthorizerIsZero() public { - // vault has mfaAuthorizer == address(0). - uint256 idx = vault.createMFALink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); - bytes memory sig = _signOpen(address(vault), idx, ALICE); - - vm.expectRevert(EnvelopeLinks.MfaAuthorizerIsZero.selector); - vault.claimWithMFA(idx, ALICE, sig, hex"", 0); + function test_M1_constructorRejectsZeroAuthorizer() public { + vm.expectRevert(EnvelopeLinks.ZeroMfaAuthorizer.selector); + new EnvelopeLinks(address(0), address(this), address(0)); } - function test_M1_isValidGaslessOperationReturnsFalseWhenAuthorizerZero() public { - uint256 idx = vault.createMFALink{value: 1 ether}(address(0), 0, 1 ether, 0, linkPubKey); - bytes memory sig = _signOpen(address(vault), idx, ALICE); - bytes memory mfaSig = hex"00"; - - bytes memory callData = abi.encodeCall(EnvelopeLinks.claimWithMFA, (idx, ALICE, sig, mfaSig, 0)); - - bool valid = vault.isValidGaslessOperation(ALICE, callData); - assertFalse(valid, "Should reject when mfaAuthorizer is zero"); + function test_M1_setMfaAuthorizerRejectsZero() public { + vm.expectRevert(EnvelopeLinks.ZeroMfaAuthorizer.selector); + mfaVault.setMfaAuthorizer(address(0)); } // ══════════════════════════════════════════════════════════════════════════════ diff --git a/test/envelope/Gasless.t.sol b/test/envelope/Gasless.t.sol index 551b5897..f0ecbd94 100644 --- a/test/envelope/Gasless.t.sol +++ b/test/envelope/Gasless.t.sol @@ -71,13 +71,7 @@ contract EnvelopeLinksGaslessTest is Test { uint256 deadline ) internal view returns (bytes memory) { bytes32 digest = EnvelopeFeeAuthTestUtils.feeAuthorizationDigest( - address(vault), - request, - feePayer, - serviceFee, - gaslessFee, - gaslessSponsored, - deadline + address(vault), request, feePayer, serviceFee, gaslessFee, gaslessSponsored, deadline ); (uint8 v, bytes32 r, bytes32 s) = vm.sign(BACKEND_PRIVKEY, digest); return abi.encodePacked(r, s, v); @@ -153,7 +147,7 @@ contract EnvelopeLinksGaslessTest is Test { assertEq(fees.gaslessFee, gaslessFee); assertFalse(status.gaslessSponsored); assertEq(feeToken.balanceOf(address(vault)), serviceFee + gaslessFee); - assertEq(vault.accumulatedFees(address(feeToken)), serviceFee + gaslessFee); + assertEq(vault.accumulatedFees(), serviceFee + gaslessFee); } function test_SponsoredGaslessAuthorizationApprovesPaymasterWithoutGaslessFee() public { @@ -204,7 +198,7 @@ contract EnvelopeLinksGaslessTest is Test { assertEq(fees.gaslessFee, 0); assertFalse(status.gaslessSponsored); assertEq(feeToken.balanceOf(address(vault)), 0); - assertEq(vault.accumulatedFees(address(feeToken)), 0); + assertEq(vault.accumulatedFees(), 0); } function test_ZeroFeeAuthorizationWithoutSignatureRemainsOpen() public { diff --git a/test/envelope/Integration.t.sol b/test/envelope/Integration.t.sol index c2d52fab..ce844d09 100644 --- a/test/envelope/Integration.t.sol +++ b/test/envelope/Integration.t.sol @@ -25,7 +25,7 @@ contract EnvelopeLinksIntegrationTest is Test, ERC1155Holder, ERC721Holder { function setUp() public { console.log("Setting up test"); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); testToken = new ERC20Mock(); testToken721 = new ERC721Mock(); testToken1155 = new ERC1155Mock(); diff --git a/test/envelope/RecipientBound.t.sol b/test/envelope/RecipientBound.t.sol index 9eebb6c9..1c32131f 100644 --- a/test/envelope/RecipientBound.t.sol +++ b/test/envelope/RecipientBound.t.sol @@ -22,7 +22,7 @@ contract RecipientBoundTest is Test { function setUp() public { console.log("Setting up test"); testToken = new ERC20Mock(); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); testToken.mint(address(this), 1000); testToken.approve(address(vault), 1000); } diff --git a/test/envelope/SenderWithdraw.t.sol b/test/envelope/SenderWithdraw.t.sol index ae2e9104..eccb0655 100644 --- a/test/envelope/SenderWithdraw.t.sol +++ b/test/envelope/SenderWithdraw.t.sol @@ -19,7 +19,7 @@ contract TestSenderWithdrawEther is Test { function setUp() public { console.log("Setting up test"); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); } function testSenderWithdrawEther(uint64 amount) public { @@ -44,7 +44,7 @@ contract TestSenderWithdrawErc20 is Test { // apparently not possible to fuzz test in setUp() function? function setUp() public { console.log("Setting up test"); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); testToken = new ERC20Mock(); // contractType 1 // Mint tokens for test accounts (larger than uint128) @@ -78,7 +78,7 @@ contract TestSenderWithdrawErc721 is Test, ERC721Holder { // apparently not possible to fuzz test in setUp() function? function setUp() public { console.log("Setting up test"); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); testToken = new ERC721Mock(); // contractType 2 // Mint token for test @@ -110,7 +110,7 @@ contract TestSenderWithdrawErc1155 is Test, ERC1155Holder { function setUp() public { console.log("Setting up test"); - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); testToken = new ERC1155Mock(); // Mint tokens diff --git a/test/envelope/SigWithdraw.t.sol b/test/envelope/SigWithdraw.t.sol index f93681a6..4c67d1ac 100644 --- a/test/envelope/SigWithdraw.t.sol +++ b/test/envelope/SigWithdraw.t.sol @@ -15,7 +15,7 @@ contract TestSigWithdrawEther is Test { receive() external payable {} function setUp() public { - vault = new EnvelopeLinks(address(0), address(this), address(0)); + vault = new EnvelopeLinks(address(0xBA), address(this), address(0)); _pubkey20 = vm.addr(LINK_PRIV); }