diff --git a/cards/src/core/CardSpend.sol b/cards/src/core/CardSpend.sol index dad9a8a..8dcdfac 100644 --- a/cards/src/core/CardSpend.sol +++ b/cards/src/core/CardSpend.sol @@ -17,7 +17,8 @@ struct CardSpend { uint256 destinationHint; /// @dev Tempo Earn vault that funds `fromVault`, holding `token`. Zero when `fromVault` is zero. address vault; - /// @dev Part of `amount` redeemed from `vault`. The rest is paid in `token`. + /// @dev Part of `amount` redeemed from `vault`. The rest is paid in `token`. Only a public spend mixes the two: a + /// private spend carries one token, so `fromVault` is zero or all of `amount`. uint256 fromVault; /// @dev Most EarnShares `fromVault` may burn. Zero accepts the live price. uint256 maxShares; diff --git a/cards/src/core/CardSpender.sol b/cards/src/core/CardSpender.sol index 4549e06..c6d3ecb 100644 --- a/cards/src/core/CardSpender.sol +++ b/cards/src/core/CardSpender.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.36; import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; import { IERC20Like } from "tempo-earn/interfaces/external/IERC20Like.sol"; +import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol"; import { ADDRESS_REGISTRY_ADDRESS, IAddressRegistry } from "zone-router/interfaces/IAddressRegistry.sol"; import { IERC20Allowance } from "zone-router/interfaces/IERC20Allowance.sol"; import { ISpender } from "zone-router/interfaces/ISpender.sol"; @@ -55,6 +56,7 @@ contract CardSpender is ISpender { error ResidualBalance(address token); error ShareCostExceeded(uint256 required, uint256 maxShares); error UnresolvedRecipient(); + error UnsupportedToken(address token); error VaultNotAllowed(address vault); error WrongInputToken(address token); error WrongProgram(address program); @@ -224,6 +226,9 @@ contract CardSpender is ISpender { function _check(CardSpend memory s, address payer, bool isPrivate) private view returns (address payee) { if (s.program != address(this)) revert WrongProgram(s.program); if (s.amount == 0 || s.fromVault > s.amount) revert InvalidAmount(); + // Only TIP-20s settle: no transfer callbacks, fees or rebases to upset the exact-credit checks. A vault's + // asset must equal `s.token`, so this covers redemptions too. + if (!StdPrecompiles.TIP20_FACTORY.isTIP20(s.token)) revert UnsupportedToken(s.token); payee = _payee(s.destination); // The program's own balance is the baseline it measures input against, so it never pays itself. if (payee == address(0) || payee == address(this)) revert DestinationNotAuthorized(s.destination); @@ -254,9 +259,15 @@ contract CardSpender is ISpender { uint256 required = v.previewWithdraw(s.fromVault); if (s.maxShares != 0 && required > s.maxShares) revert ShareCostExceeded(required, s.maxShares); address share = v.earnShare(); + uint256 shareBase = IERC20Like(share).balanceOf(address(this)); SafeTransferLib.safeTransferFrom(share, msg.sender, address(this), required); - sharesUsed = _redeem(v, share, s, payee, required); - if (sharesUsed < required) _refund(share, required - sharesUsed); + _redeem(v, share, s, payee, required); + // The burn is measured, not taken from the vault's report, and may not reach the program's seed. + uint256 shareHeld = IERC20Like(share).balanceOf(address(this)); + if (shareHeld < shareBase) revert ShareCostExceeded(required + shareBase - shareHeld, required); + uint256 unused = shareHeld - shareBase; + sharesUsed = required - unused; + if (unused != 0) _refund(share, unused); } /// @dev A direct private spend, from the `maxInput` the router sent. The vault prices shares itself and diff --git a/cards/src/providers/bridge/SpenderTempoSpend.sol b/cards/src/providers/bridge/SpenderTempoSpend.sol index 4a725ea..8bce6ae 100644 --- a/cards/src/providers/bridge/SpenderTempoSpend.sol +++ b/cards/src/providers/bridge/SpenderTempoSpend.sol @@ -7,6 +7,7 @@ import { SpenderBase } from "bridge-spender/src/spenders/SpenderBase.sol"; import { IERC20 } from "openzeppelin-contracts/token/ERC20/IERC20.sol"; import { LibClone } from "solady/utils/LibClone.sol"; import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; +import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol"; import { ADDRESS_REGISTRY_ADDRESS, IAddressRegistry } from "zone-router/interfaces/IAddressRegistry.sol"; import { ITempoEarnVault } from "../../core/interfaces/ITempoEarnVault.sol"; @@ -52,9 +53,12 @@ contract SpenderTempoSpend is SpenderBase { error DestinationNotAuthorized(address destination); error ExpectedBooleanByte(); error IncorrectAssetForVault(); + error InvalidVault(); error ShareCostExceeded(uint256 required, uint256 maxShares); error PipelineDisabled(); + error RefundNotCredited(); error UnresolvedRecipient(); + error UnsupportedToken(address token); error VaultNotAllowed(); error WrongProgram(bytes32 spenderId); @@ -116,6 +120,7 @@ contract SpenderTempoSpend is SpenderBase { require(data.length == 23, DataIncorrectLength()); require(data[2] == 0x00 || data[2] == 0x01, ExpectedBooleanByte()); address vault = address(bytes20(data[3:23])); + require(vault != address(0), InvalidVault()); bool allowed = data[2] == 0x01; _storage().allowedVaults[vault] = allowed; // The clone's standing EarnShare approval follows the allowlist: set for an allowed vault, so its @@ -202,10 +207,14 @@ contract SpenderTempoSpend is SpenderBase { // Implied by `_isCanonical` under the vendored factory, which registers an ID once and deploys its clone // only in `createSpender`; kept because the factory is upgradeable and could re-point an ID. require(factory.getSpenderAddress(s.spenderId) == address(this), WrongProgram(s.spenderId)); + // The literal destination, not the master it resolves to: a master may hand out its virtual addresses as + // separate deposit accounts, so allowing the master must not allow them. require(factory.isAuthorizedDestination(s.spenderId, s.destination), DestinationNotAuthorized(s.destination)); require(s.amount > 0 && s.fromVault <= s.amount, InvalidAmount()); address payee = _payee(s.destination); require(payee != address(0) && payee != address(this), DestinationNotAuthorized(s.destination)); + // Only TIP-20s settle: no transfer callbacks, fees or rebases to upset the exact-credit checks. + require(StdPrecompiles.TIP20_FACTORY.isTIP20(s.token), UnsupportedToken(s.token)); uint256 fromStable = s.amount - s.fromVault; uint256 before = IERC20(s.token).balanceOf(payee); @@ -235,6 +244,7 @@ contract SpenderTempoSpend is SpenderBase { require(s.maxShares == 0 || required <= s.maxShares, ShareCostExceeded(required, s.maxShares)); address share = v.earnShare(); + uint256 shareBase = IERC20(share).balanceOf(address(this)); SafeTransferLib.safeTransferFrom(share, msg.sender, address(this), required); // A standing maximum approval, set when governance allowed the vault and topped up here when // another handler (such as TF, which clears its own) has lowered it. Clearing it after each spend @@ -251,14 +261,27 @@ contract SpenderTempoSpend is SpenderBase { } } if (payee == s.destination) { - burned = v.withdrawExact(s.fromVault, s.destination, required); + v.withdrawExact(s.fromVault, s.destination, required); } else { uint256 held = IERC20(s.token).balanceOf(address(this)); - burned = v.withdrawExact(s.fromVault, address(this), required); + v.withdrawExact(s.fromVault, address(this), required); require(IERC20(s.token).balanceOf(address(this)) == held + s.fromVault, DestinationNotCredited()); SafeTransferLib.safeTransfer(s.token, s.destination, s.fromVault); } - if (burned < required) SafeTransferLib.safeTransfer(share, msg.sender, required - burned); + // The burn is measured, not taken from the vault's report, and may not reach shares the clone held before. + uint256 shareHeld = IERC20(share).balanceOf(address(this)); + require(shareHeld >= shareBase, ShareCostExceeded(required + shareBase - shareHeld, required)); + uint256 unused = shareHeld - shareBase; + burned = required - unused; + if (unused != 0) _refund(share, unused); + } + + /// @dev Returns unused shares to the payer. Receive policies can redirect a successful transfer to a guard + /// (TIP-1028), so the payer's balance must rise by exactly `amount`. + function _refund(address share, uint256 amount) private { + uint256 before = IERC20(share).balanceOf(msg.sender); + SafeTransferLib.safeTransfer(share, msg.sender, amount); + require(IERC20(share).balanceOf(msg.sender) == before + amount, RefundNotCredited()); } /// @dev Who a transfer to `to` credits: its TIP-1022 master if it is a virtual address, else `to`. diff --git a/cards/test/bridge/unit/BridgeProgram.t.sol b/cards/test/bridge/unit/BridgeProgram.t.sol index 1457585..ca3db22 100644 --- a/cards/test/bridge/unit/BridgeProgram.t.sol +++ b/cards/test/bridge/unit/BridgeProgram.t.sol @@ -9,6 +9,7 @@ import { ZONE_FACTORY_ADDRESS as ROUTER_ZONE_FACTORY, ZoneInfo as RouterZoneInfo } from "tempo-earn/interfaces/external/tempo/IZone.sol"; +import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol"; import { ZoneRouter } from "zone-router/ZoneRouter.sol"; import { ZoneRouterFactory } from "zone-router/ZoneRouterFactory.sol"; @@ -378,6 +379,12 @@ contract BridgeProgramTest is AccessKeys { /// reentrancy lock, and the spend still settles. function test_rejectsReentrancy() public { ReentrantToken token = new ReentrantToken(); + // Stands in for a TIP-20, so the program's own lock is what stops the callback. + vm.mockCall( + StdPrecompiles.TIP20_FACTORY_ADDRESS, + abi.encodeWithSignature("isTIP20(address)", address(token)), + abi.encode(true) + ); token.mint(router, AMOUNT); token.arm(address(program), abi.encodeCall(CardSpender.spendFrom, (address(token), 1, ""))); CardSpend memory spend = _stableSpend(AMOUNT); diff --git a/cards/test/bridge/unit/BridgeProviderMock.t.sol b/cards/test/bridge/unit/BridgeProviderMock.t.sol index 3e12d24..5cf5a7f 100644 --- a/cards/test/bridge/unit/BridgeProviderMock.t.sol +++ b/cards/test/bridge/unit/BridgeProviderMock.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.36; import { Test } from "forge-std/Test.sol"; import { IERC165 } from "openzeppelin-contracts/utils/introspection/IERC165.sol"; +import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol"; import { CardSpend } from "../../../src/core/CardSpend.sol"; import { CardSpender } from "../../../src/core/CardSpender.sol"; @@ -185,6 +186,17 @@ contract BridgeProviderMockTest is Test { function setUp() public { usd = new SpenderMockToken(); share = new SpenderMockToken(); + // The mock tokens stand in for TIP-20s, which the program requires. + vm.mockCall( + StdPrecompiles.TIP20_FACTORY_ADDRESS, + abi.encodeWithSignature("isTIP20(address)", address(usd)), + abi.encode(true) + ); + vm.mockCall( + StdPrecompiles.TIP20_FACTORY_ADDRESS, + abi.encodeWithSignature("isTIP20(address)", address(share)), + abi.encode(true) + ); vault = new MockVault(address(share), address(usd)); clone = new MockClone(); clone.setVaultAnswer(abi.encode(abi.encode(true)), false); diff --git a/cards/test/bridge/unit/SpenderTempoSpend.t.sol b/cards/test/bridge/unit/SpenderTempoSpend.t.sol index 449a2d9..4909d70 100644 --- a/cards/test/bridge/unit/SpenderTempoSpend.t.sol +++ b/cards/test/bridge/unit/SpenderTempoSpend.t.sol @@ -5,6 +5,7 @@ import { SpenderBase } from "bridge-spender/src/spenders/SpenderBase.sol"; import { IAccessControl } from "openzeppelin-contracts/access/IAccessControl.sol"; import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; import { FeeConfig } from "tempo-earn/interfaces/core/EarnFeeTypes.sol"; +import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol"; import { ITempoEarnVault } from "../../../src/core/interfaces/ITempoEarnVault.sol"; import { @@ -108,9 +109,25 @@ contract SpenderTempoSpendTest is TempoEnv { _expectRevert(alice, _stable(0), abi.encodeWithSelector(SpenderBase.InvalidAmount.selector)); } - /// @notice A token that delivers less than it was asked to cannot settle a spend. + /// @notice Only TIP-20s settle, so a token with fees, callbacks or rebases is refused up front. + function test_spendRejectsNonTip20Token() public { + FeeOnTransferToken token = new FeeOnTransferToken(); + token.mint(alice, 100e6); + vm.prank(alice); + token.approve(spender, type(uint256).max); + Spend memory s = _stable(10e6); + s.token = address(token); + _expectRevert(alice, s, abi.encodeWithSelector(SpenderTempoSpend.UnsupportedToken.selector, address(token))); + } + + /// @notice A token that delivers less than it was asked to cannot settle a spend, even past the TIP-20 check. function test_spendRejectsFeeOnTransferToken() public { FeeOnTransferToken token = new FeeOnTransferToken(); + vm.mockCall( + StdPrecompiles.TIP20_FACTORY_ADDRESS, + abi.encodeWithSignature("isTIP20(address)", address(token)), + abi.encode(true) + ); token.mint(alice, 100e6); vm.prank(alice); token.approve(spender, type(uint256).max); @@ -489,6 +506,13 @@ contract SpenderTempoSpendTest is TempoEnv { bridgeFactory.governorCall(SPENDER_ID, abi.encodePacked(PREFIX_TEMPO_SPEND, bytes2("UV"), uint8(1))); } + function test_governorCallRejectsZeroVault() public { + vm.prank(bridgeGovernor); + vm.expectRevert(SpenderTempoSpend.InvalidVault.selector); + bridgeFactory.governorCall(SPENDER_ID, abi.encodePacked(PREFIX_TEMPO_SPEND, bytes2("UV"), uint8(1), address(0))); + assertFalse(_vaultAllowed(address(0))); + } + function test_governorCallRejectsNonBooleanFlag() public { vm.prank(bridgeGovernor); vm.expectRevert(SpenderTempoSpend.ExpectedBooleanByte.selector); @@ -541,6 +565,11 @@ contract SpenderTempoSpendTest is TempoEnv { ADDRESS_REGISTRY_ADDRESS, abi.encodeCall(IAddressRegistry.resolveRecipient, (virtualTo)), abi.encode(master) ); VirtualAwareToken vusd = new VirtualAwareToken("VUSD"); + vm.mockCall( + StdPrecompiles.TIP20_FACTORY_ADDRESS, + abi.encodeWithSignature("isTIP20(address)", address(vusd)), + abi.encode(true) + ); LiteralReceiverVault vault = new LiteralReceiverVault(address(vusd)); VirtualAwareToken share = VirtualAwareToken(vault.earnShare()); vusd.mint(address(vault), 100e6); @@ -567,6 +596,42 @@ contract SpenderTempoSpendTest is TempoEnv { assertEq(share.balanceOf(spender), 0); } + /// @notice Allowing a master does not allow its virtual addresses: a master may hand them out as separate + /// deposit accounts, so only the literal destination on the allowlist is paid. + function test_spendToAVirtualDestinationOfAnAllowedMasterReverts() public { + address virtualTo = address(uint160(0x12345678fdfDfdFdfDFdfdfdFDfD000000000002)); + address master = makeAddr("virtual.allowed-master"); + vm.mockCall( + ADDRESS_REGISTRY_ADDRESS, abi.encodeCall(IAddressRegistry.resolveRecipient, (virtualTo)), abi.encode(master) + ); + vm.prank(bridgeGovernor); + bridgeFactory.updateSpenderDestination(SPENDER_ID, master, true); + Spend memory s = _stable(1e6); + s.destination = virtualTo; + _expectRevert(alice, s, abi.encodeWithSelector(SpenderTempoSpend.DestinationNotAuthorized.selector, virtualTo)); + } + + /// @notice A vault that burns more shares than it reports cannot make the clone refund shares it held before: + /// the refund and the reported burn are measured from the clone's balance. + function test_spendMeasuresTheBurnNotTheVaultReport() public { + UnderReportingEarnVault vault = new UnderReportingEarnVault(address(usd)); + _mintUsd(address(vault), 100e6); + _setVault(address(vault), true); + MockToken share = vault.SHARE(); + share.mint(alice, 100e6); + share.mint(spender, 5e6); + vm.prank(alice); + share.approve(spender, type(uint256).max); + + Spend memory s = _earn(10e6, 10e6, 0); + s.vault = address(vault); + vm.expectEmit(true, true, true, true, spender); + emit SpenderTempoSpend.TempoSpendExecuted(s.ref, alice, settlement, address(usd), 10e6, 0, 10e6, 20e6); + _spendAs(alice, s); + assertEq(share.balanceOf(alice), 80e6, "payer charged the quote and refunded nothing"); + assertEq(share.balanceOf(spender), 5e6, "the clone's own shares untouched"); + } + // ------------------------------------------------------------------ // Helpers // ------------------------------------------------------------------ @@ -620,3 +685,38 @@ contract SpenderTempoSpendTest is TempoEnv { _cloneCall(_psCall(s)); } } + +/// @notice Burns the full quote but reports half of it burnt. +contract UnderReportingEarnVault is ITempoEarnVault { + address public immutable override asset; + MockToken public immutable SHARE; + + constructor(address asset_) { + asset = asset_; + SHARE = new MockToken("Under-reported EarnShare", "uES"); + } + + function earnShare() external view override returns (address) { + return address(SHARE); + } + + function previewWithdraw(uint256 assets) public pure override returns (uint256) { + return assets * 2; + } + + function previewRedeem(uint256 earnShares) external pure override returns (uint256) { + return earnShares / 2; + } + + function withdrawExact(uint256 assets, address receiver, uint256 maxEarnShares) + external + override + returns (uint256) + { + uint256 cost = previewWithdraw(assets); + require(cost <= maxEarnShares, "max shares"); + SHARE.transferFrom(msg.sender, address(0xdead), cost); + SafeTransferLib.safeTransfer(asset, receiver, assets); + return cost / 2; + } +} diff --git a/cards/test/core/CardSpender.t.sol b/cards/test/core/CardSpender.t.sol index 4a7d293..0dd976d 100644 --- a/cards/test/core/CardSpender.t.sol +++ b/cards/test/core/CardSpender.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.36; import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; +import { StdPrecompiles } from "tempo-std/StdPrecompiles.sol"; import { FeeConfig } from "tempo-earn/interfaces/core/EarnFeeTypes.sol"; @@ -239,6 +240,12 @@ abstract contract CardSpenderBehavior is CoreEnv { /// @dev A hand-back that reports success without moving the change leaves it behind, which is refused. function test_revert_residualBalance() public { StuckToken token = new StuckToken(); + // Stands in for a TIP-20, so the program's own residual check is what refuses it. + vm.mockCall( + StdPrecompiles.TIP20_FACTORY_ADDRESS, + abi.encodeWithSignature("isTIP20(address)", address(token)), + abi.encode(true) + ); mock.setToken(address(token), true); token.mint(router, 10e6); CardSpend memory s = _stableSpend(pooledWallet, 0, 1e6, "residual"); @@ -350,6 +357,12 @@ abstract contract CardSpenderBehavior is CoreEnv { /// point, and the spend still pays. function test_revert_reentrancy() public { ReentrantToken token = new ReentrantToken(); + // Stands in for a TIP-20, so the program's own lock is what stops the callback. + vm.mockCall( + StdPrecompiles.TIP20_FACTORY_ADDRESS, + abi.encodeWithSignature("isTIP20(address)", address(token)), + abi.encode(true) + ); mock.setToken(address(token), true); token.mint(alice, 10e6); token.mint(router, 10e6); @@ -466,6 +479,29 @@ contract CardSpenderDirectTest is CardSpenderBehavior { assertEq(program.PAYOUT_TARGET(), address(0)); } + /// @dev A vault that burns more shares than it reports cannot make the program refund from its own balance: + /// the refund is what the program measurably still holds of the payer's shares. + function test_publicSpend_refundIsMeasuredNotReported() public { + UnderReportingVault vault = new UnderReportingVault(address(usd)); + mock.setVault(address(vault), true); + usd.mint(address(vault), 100e6); + MintableShare share = MintableShare(vault.earnShare()); + share.mint(alice, 100e6); + share.mint(address(program), 5e6); + CardSpend memory s = _earnSpend(publicWallet, 0, 10e6, 10e6, "under.report"); + s.vault = address(vault); + s.maxShares = 0; + vm.prank(alice); + share.approve(address(program), type(uint256).max); + vm.expectEmit(address(program)); + emit CardSpender.CardSpent("under.report", alice, publicWallet, address(usd), 10e6, 10e6, 20e6, false); + vm.prank(alice); + program.spend(s); + assertEq(usd.balanceOf(publicWallet), 10e6); + assertEq(share.balanceOf(alice), 80e6, "payer charged the quote and refunded nothing"); + assertEq(share.balanceOf(address(program)), 5e6, "the program's own shares untouched"); + } + /// @dev Anyone may set the program's standing EarnShare approval to an allowed vault before its first /// spend from it, after which a spend approves nothing; a vault the provider does not allow is refused. function test_warmVault() public { @@ -679,6 +715,12 @@ contract CardSpenderVirtualDestinationTest is CoreEnv { ADDRESS_REGISTRY_ADDRESS, abi.encodeCall(IAddressRegistry.resolveRecipient, (VIRTUAL)), abi.encode(master) ); vusd = new VirtualAwareToken("VUSD"); + // Stands in for a TIP-20 that resolves virtual recipients. + vm.mockCall( + StdPrecompiles.TIP20_FACTORY_ADDRESS, + abi.encodeWithSignature("isTIP20(address)", address(vusd)), + abi.encode(true) + ); vault = new LiteralReceiverVault(address(vusd)); vusd.mint(address(vault), 1000e6); vusd.mint(alice, 1000e6); @@ -772,6 +814,49 @@ contract CardSpenderVirtualDestinationTest is CoreEnv { } /// @dev An ERC-20 whose transfers from one account to another report success without moving anything. +/// @dev Burns the full quote but reports half of it burnt. +contract UnderReportingVault is ITempoEarnVault { + address public immutable override asset; + MintableShare internal immutable SHARE; + + constructor(address asset_) { + asset = asset_; + SHARE = new MintableShare(); + } + + function earnShare() external view override returns (address) { + return address(SHARE); + } + + function previewWithdraw(uint256 assets) public pure override returns (uint256) { + return assets * 2; + } + + function previewRedeem(uint256 earnShares) external pure override returns (uint256) { + return earnShares / 2; + } + + function withdrawExact(uint256 assets, address receiver, uint256 maxEarnShares) + external + override + returns (uint256) + { + uint256 cost = previewWithdraw(assets); + require(cost <= maxEarnShares, "max shares"); + SHARE.transferFrom(msg.sender, address(0xdead), cost); + SafeTransferLib.safeTransfer(asset, receiver, assets); + return cost / 2; + } +} + +contract MintableShare is ERC20 { + constructor() ERC20("Under-reported EarnShare", "uES") { } + + function mint(address to, uint256 amount) external { + _mint(to, amount); + } +} + contract StuckToken is ERC20 { address public stuckFrom; address public stuckTo; diff --git a/cards/test/core/RefundReceivePolicy.t.sol b/cards/test/core/RefundReceivePolicy.t.sol index 9263e06..aca0965 100644 --- a/cards/test/core/RefundReceivePolicy.t.sol +++ b/cards/test/core/RefundReceivePolicy.t.sol @@ -21,6 +21,8 @@ import { ITIP403Registry } from "tempo-std/interfaces/ITIP403Registry.sol"; import { CardSpend } from "../../src/core/CardSpend.sol"; import { CardSpender } from "../../src/core/CardSpender.sol"; +import { SpenderTempoSpend } from "../../src/providers/bridge/SpenderTempoSpend.sol"; +import { Spend } from "../../src/providers/bridge/interfaces/IBridgeSpender.sol"; import { TempoEnv } from "../bridge/harness/TempoEnv.sol"; import { MockProvider } from "./harness/Mocks.sol"; @@ -137,6 +139,56 @@ contract RefundReceivePolicyTest is TempoEnv { _successful(program); } + function test_directCloneBlockedRefundDefaultAuthorityRevertsAtomically() public { + _cloneBlocked(address(0)); + } + + function test_directCloneBlockedRefundPayerAuthorityRevertsAtomically() public { + _cloneBlocked(payer); + } + + function test_directCloneNoReceivePolicyRefundCreditsPayer() public { + Spend memory s = _cloneSpend(); + vm.prank(payer); + earnShare.approve(spender, s.maxShares); + uint256 shares = earnShare.balanceOf(payer); + vm.prank(payer); + _cloneCall(_psCall(s)); + assertEq(shares - earnShare.balanceOf(payer), 10e6, "refund must reach payer"); + assertEq(earnShare.balanceOf(address(GUARD)), 0); + assertEq(earnShare.balanceOf(spender), 0); + } + + /// @dev A payer may spend through the clone directly; its refund must reach the payer, not the guard. + function _cloneBlocked(address authority) internal { + vm.prank(payer); + POLICIES.setReceivePolicy(senderPolicy, allTokens, authority); + Spend memory s = _cloneSpend(); + vm.prank(payer); + earnShare.approve(spender, s.maxShares); + Balances memory before = _balances(program); + vm.prank(payer); + vm.expectRevert(SpenderTempoSpend.RefundNotCredited.selector); + _cloneCall(_psCall(s)); + assertEq(abi.encode(_balances(program)), abi.encode(before), "refund failure must roll back every balance"); + assertEq(earnShare.allowance(payer, spender), s.maxShares, "payer approval must roll back"); + } + + function _cloneSpend() internal view returns (Spend memory) { + uint256 quote = earnVault.previewWithdraw(SPEND_AMOUNT); + assertEq(quote, 11e6); + return Spend({ + spenderId: SPENDER_ID, + token: address(usd), + amount: SPEND_AMOUNT, + destination: settlement, + vault: address(earnVault), + fromVault: SPEND_AMOUNT, + maxShares: quote, + ref: bytes32("refund.clone") + }); + } + function _allowRefund(CardSpender selected) internal { vm.startPrank(payer); POLICIES.modifyPolicyWhitelist(senderPolicy, address(selected), true); diff --git a/docs/cards/bridge/bridge-integration.md b/docs/cards/bridge/bridge-integration.md index 14efaca..41c5568 100644 --- a/docs/cards/bridge/bridge-integration.md +++ b/docs/cards/bridge/bridge-integration.md @@ -115,6 +115,10 @@ outbox.requestWithdrawal(inputToken, router, input, memo, callbackGas, `router` is the program's Spend-mode `ZoneRouter` for that Zone (see [`deploy.md`](deploy.md)). +Send both calls in the same transaction, and approve exactly what the withdrawal takes. The key's scope +allows each call alone, so an approval left standing could be withdrawn in a later period without +charging the key's limit again, or by another key with `requestWithdrawal` on the same account. + Bridge sponsors the transaction, and the outbox then takes the withdrawal fee from Bridge's fee payer in the input token: stablecoin, or EarnShare for an Earn spend. The fee payer holds and approves those tokens. Unsponsored, the cardholder approves `input + withdrawalFee` and pays it. diff --git a/docs/cards/bridge/design.md b/docs/cards/bridge/design.md index 2a287a8..cabd312 100644 --- a/docs/cards/bridge/design.md +++ b/docs/cards/bridge/design.md @@ -96,6 +96,9 @@ and expiry of the key in the protocol. The cardholder can revoke the key at any - **Tempo account.** Scope: `token.approve(recipient = program)`, `earnShare.approve(recipient = program)` for Earn, and `program.spend`. - **Zone account.** Scope: `zoneToken.approve(recipient = outbox)` and `outbox.requestWithdrawal`. + The scope does not tie the two together, so Bridge always sends them in one transaction, approving + exactly what that withdrawal takes. An approval left standing would let the key withdraw it later + without charging its limit again, and let any other key with `requestWithdrawal` on the account use it. - **Limits.** Each key has a periodic limit for each token and an expiry. Bridge uses one key for each program. Limits count gross debits. @@ -299,10 +302,17 @@ struct SpendReturn { - `transferToDestination` and `transferToDestinationStrict` always revert, so no standing allowance can be pulled. Wallets that cannot use access keys stay on the EA and TF handlers. - Governance action "UV" (factory only) allows or disallows a vault, and sets or clears the clone's - share approval to that vault. + share approval to that vault. It refuses the zero address (`InvalidVault`). +- Only TIP-20 tokens settle (`UnsupportedToken`). +- Unused shares go back to the payer, measured from the clone's own balance rather than the vault's + report, and the payer's balance must rise by exactly that amount (`RefundNotCredited`). A payer who + calls the clone directly is held to the same check as the program. - `availableBalance` reports the payer's balance of `token` plus the value of its shares in an allowed vault that holds `token`: an upper bound, since a spend pays at the `previewWithdraw` cost. -- A virtual destination (TIP-1022) is credited through its master: the handler checks the master's +- A virtual destination (TIP-1022) is credited through its master. Unlike TIP-403, which authorizes the + master, the handler authorizes only the literal destination on the program's allowlist: a master may + hand out its virtual addresses as separate deposit accounts, so allowing a master does not allow them, + and allowing a virtual address pays its master. The handler checks the master's balance, and redeems Earn shares for one to the clone and transfers them on, since Earn's engines check the literal receiver. - Storage is at an ERC-7201 slot (`tempo.spend.SpenderTempoSpend`), so it cannot collide with the diff --git a/docs/cards/design.md b/docs/cards/design.md index 803f9c6..1e1e01a 100644 --- a/docs/cards/design.md +++ b/docs/cards/design.md @@ -100,6 +100,8 @@ program holds as its baseline and leaves it there, so a spend never creates the - `CardSpend.program` is this program (`WrongProgram`); `amount` is above zero and `fromVault` at most `amount` (`InvalidAmount`). +- `token` is a TIP-20 (`UnsupportedToken`), so no transfer callback, fee or rebase can upset the + exact-credit checks. An Earn spend's vault holds `token`, so its asset is a TIP-20 too. - A program that pays directly asks its provider: it has not paused the program (`ProgramPaused`), it authorizes the destination for this payer, token and path (`DestinationNotAuthorized`), and an Earn spend's vault is allowed and holds `token` (`VaultNotAllowed`, `IncorrectAssetForVault`). A program @@ -109,8 +111,10 @@ program holds as its baseline and leaves it there, so a spend never creates the - The destination's balance of `token` rises by exactly `amount` (`DestinationNotCredited`). - A private destination may be neither the calling router nor an alias resolving to it (`DestinationNotAuthorized`): the router returns only unused input, not the redeemed asset. -- A public Earn refund increases the payer's share balance by exactly the unused amount - (`RefundNotCredited`). A receive policy that redirects the refund reverts the entire spend, including +- A public Earn refund is what the program measurably still holds of the shares it pulled, not what the + vault reports burning, so a vault that under-reports cannot make it refund its own balance + (`ShareCostExceeded` if the vault took more). The refund increases the payer's share balance by + exactly the unused amount (`RefundNotCredited`). A receive policy that redirects the refund reverts the entire spend, including the redemption and provider payment; allowing the program to refund shares permits a retry. - A virtual destination (TIP-1022) is credited through its master, so the program checks the master's balance, and redeems Earn shares for one to itself first and transfers them on, since Earn's engines diff --git a/docs/threat-models/card-spend.md b/docs/threat-models/card-spend.md index 3815c1b..56d0ec7 100644 --- a/docs/threat-models/card-spend.md +++ b/docs/threat-models/card-spend.md @@ -284,11 +284,34 @@ The design accepts these risks. They are decisions, not gaps. - **A leaked Zone key.** It can withdraw a cardholder's Zone balance, up to the key's limit, to any address in an open Zone or any admitted account in a closed-loop Zone, and it chooses where the change goes. Bridge uses one key per program, so one leak reaches every cardholder of that program. +- **Standing outbox approvals.** A Zone key's `approve` and `requestWithdrawal` scopes are separate, and + only `approve` counts against its limit. A key that approves in one period and withdraws in the next + can take up to twice its periodic limit, and another key with `requestWithdrawal` on the same account + can use an approval it did not make. Bridge sends both calls in one transaction for the exact amount; + removing the need for the approval needs an outbox or scope change outside this repository. - **A leaked public key.** It can spend a cardholder's funds, up to the key's limit, only to the program's destinations: its scope names the program's `spend`. - **No on-chain idempotency.** The provider removes duplicate spends off chain, by its own reference. - **Approval before settlement.** The provider approves a private spend on the Zone receipt. If L1 - settlement fails, the provider's reserve pays. + settlement fails, the funds return to the cardholder's Zone balance and the provider's reserve pays. + Known ways an approved spend can fail to settle: + - **Liquidity.** `availableBalance` values shares and reserves nothing. A spend can exceed the vault's + withdrawable liquidity, or lose it to a redemption the cardholder queues ahead of it in the same Zone, + makes on the L1 before it settles, or to another spend against the same buffer, by the same + cardholder or another. + - **Ordering.** On a Zone that permits arbitrary callback targets, an earlier withdrawal in the same + `processWithdrawals` can run an L1 operation first. On a Veda engine, an earlier operation in the same + transaction can pin and then move the Accountant rate (`RateChangedWithinTransaction`). + - **Fees and gas.** A higher L1 base fee raises the bounce-back fee above the change's margin + (`ChangeBelowDepositFee`, `DepositTooSmall`), and a venue's gas can rise past the callback's budget. + - **Provider governance in flight.** Bridge's pauser pauses the factory; governance disables or replaces + the "TS" handler, removes the destination or disallows the vault. + - **Zone administration in flight.** The admin disables the return token or pauses deposits, raises the + deposit fee or `bouncebackGas`, removes the router's gateway or depositor permissions, or de-admits + the program's `refundRecipient`; or the return's encryption key expires. + - **Policies and venues.** A TIP-403 administrator blocks a participant in the settlement; an Earn venue + stops servicing withdrawals; the value per share falls past `maxShares` or the input; or the vault's + engine or fees change between approval and settlement. - **Events are not proof of payment.** Anyone can emit a spend event with any `ref`. The provider reconciles by transaction and log index, binding a private spend's `CardSpent` and `InputSpent` to the portal's `WithdrawalProcessed` and the router's `Spent` in the same transaction, and a public spend to