diff --git a/src/contracts/atlas/Atlas.sol b/src/contracts/atlas/Atlas.sol index d1524708e..555c04ab0 100644 --- a/src/contracts/atlas/Atlas.sol +++ b/src/contracts/atlas/Atlas.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.16; import {Factory} from "./Factory.sol"; -import {UserVerifier} from "./UserVerification.sol"; import "../types/CallTypes.sol"; import "../types/VerificationTypes.sol"; diff --git a/src/contracts/atlas/ExecutionEnvironment.sol b/src/contracts/atlas/ExecutionEnvironment.sol index ece2157e7..3caf1f7dc 100644 --- a/src/contracts/atlas/ExecutionEnvironment.sol +++ b/src/contracts/atlas/ExecutionEnvironment.sol @@ -71,6 +71,7 @@ contract ExecutionEnvironment is Test { address control = _control(); require(msg.sender == atlas && userCall.from == _user(), "ERR-CE00 InvalidSenderStaging"); + require(userCall.to != address(this), "ERR-EV008 InvalidTo"); bytes memory stagingData = abi.encodeWithSelector( IProtocolControl.stagingCall.selector, userCall.to, userCall.from, bytes4(userCall.data), userCall.data[4:] @@ -102,6 +103,7 @@ contract ExecutionEnvironment is Test { require(msg.sender == atlas && userCall.from == user, "ERR-CE00 InvalidSenderUser"); require(address(this).balance >= userCall.value, "ERR-CE01 ValueExceedsBalance"); + require(userCall.to != address(this), "ERR-EV008 InvalidTo"); bool success; @@ -172,6 +174,7 @@ contract ExecutionEnvironment is Test { // address(this) = ExecutionEnvironment require(msg.sender == atlas, "ERR-04 InvalidCaller"); require(address(this).balance == searcherCall.metaTx.value, "ERR-CE05 IncorrectValue"); + require(searcherCall.metaTx.to != address(this), "ERR-EV008 InvalidTo"); // Track token balances to measure if the bid amount is paid. uint256[] memory tokenBalances = new uint[](searcherCall.bids.length); diff --git a/src/contracts/atlas/Factory.sol b/src/contracts/atlas/Factory.sol index 7afbf4387..76a205425 100644 --- a/src/contracts/atlas/Factory.sol +++ b/src/contracts/atlas/Factory.sol @@ -7,14 +7,14 @@ import {IExecutionEnvironment} from "../interfaces/IExecutionEnvironment.sol"; import {Escrow} from "./Escrow.sol"; import {Mimic} from "./Mimic.sol"; import {ExecutionEnvironment} from "./ExecutionEnvironment.sol"; -import {TokenTransfers} from "./TokenTransfers.sol"; +import {Permit69} from "./Permit69.sol"; import "../types/CallTypes.sol"; import {EscrowKey} from "../types/LockTypes.sol"; import "forge-std/Test.sol"; -contract Factory is Test, Escrow, TokenTransfers { +contract Factory is Test, Escrow, Permit69 { //address immutable public atlas; bytes32 public immutable salt; address public immutable execution; diff --git a/src/contracts/atlas/TokenTransfers.sol b/src/contracts/atlas/Permit69.sol similarity index 84% rename from src/contracts/atlas/TokenTransfers.sol rename to src/contracts/atlas/Permit69.sol index 8af694c57..98d0d25f1 100644 --- a/src/contracts/atlas/TokenTransfers.sol +++ b/src/contracts/atlas/Permit69.sol @@ -6,7 +6,16 @@ import {SafeTransferLib, ERC20} from "solmate/utils/SafeTransferLib.sol"; import "../types/LockTypes.sol"; import {ProtocolCall} from "../types/CallTypes.sol"; -abstract contract TokenTransfers { +// NOTE: IPermit69 only works inside of the Atlas environment - specifically +// inside of the custom ExecutionEnvironments that each user deploys when +// interacting with Atlas in a manner controlled by the DeFi protocol. + +// The name comes from the reciprocal nature of the token transfers. Both +// the user and the ProtocolControl can transfer tokens from the User +// and the ProtocolControl contracts... but only if they each have granted +// token approval to the Atlas main contract, and only during specific phases +// of the Atlas execution process. +abstract contract Permit69 { using SafeTransferLib for ERC20; uint16 internal constant _EXECUTION_PHASE_OFFSET = uint16(type(BaseLock).max); diff --git a/src/contracts/atlas/ProtocolIntegration.sol b/src/contracts/atlas/ProtocolIntegration.sol index 6b7bef9b0..faef9b042 100644 --- a/src/contracts/atlas/ProtocolIntegration.sol +++ b/src/contracts/atlas/ProtocolIntegration.sol @@ -23,12 +23,14 @@ contract ProtocolIntegration { mapping(bytes32 => bytes32) public protocols; - // Integrates a new protocol + // Permissionlessly integrates a new protocol function initializeGovernance(address protocolControl) external { address owner = IProtocolControl(protocolControl).getProtocolSignatory(); require(msg.sender == owner, "ERR-V50 OnlyGovernance"); + require(signatories[owner].governance == address(0), "ERR-V49 OwnerActive"); + uint16 callConfig = CallBits.buildCallConfig(protocolControl); governance[protocolControl] = @@ -42,6 +44,8 @@ contract ProtocolIntegration { require(msg.sender == govData.governance, "ERR-V50 OnlyGovernance"); + require(signatories[signatory].governance == address(0), "ERR-V49 SignatoryActive"); + signatories[signatory] = ApproverSigningData({governance: protocolControl, enabled: true, nonce: 0}); } diff --git a/src/contracts/atlas/ProtocolVerification.sol b/src/contracts/atlas/ProtocolVerification.sol index 7746c4bbb..4367e8587 100644 --- a/src/contracts/atlas/ProtocolVerification.sol +++ b/src/contracts/atlas/ProtocolVerification.sol @@ -31,6 +31,15 @@ contract ProtocolVerifier is EIP712, ProtocolIntegration { ); mapping(address => uint256) public userNonces; + + struct NonceTracker { + uint64 asyncFloor; + uint64 asyncCeiling; + uint64 blockingLast; + } + + // keccak256(from, callConfig, nonce) => to + mapping(bytes32 => address) public asyncNonceFills; constructor() EIP712("ProtoCallHandler", "0.0.1") {} @@ -85,15 +94,18 @@ contract ProtocolVerifier is EIP712, ProtocolIntegration { // To avoid exposure to social engineering vulnerabilities, disgruntled // former employees, or beneficiary uncertainty during intra-DAO conflict, // governance should refrain from using a proxy contract for ProtocolControl. - bytes32 controlCodeHash = protocolCall.to.codehash; - if (controlCodeHash == bytes32(0) || protocols[key] != controlCodeHash) { + if (protocolCall.to.codehash == bytes32(0) || protocols[key] != protocolCall.to.codehash) { return (false); } // Verify that ProtocolControl hasn't been updated. // NOTE: Performing this check here allows the searchers' checks // to be against the verification proof's controlCodeHash to save gas. - if (controlCodeHash != verification.proof.controlCodeHash) { + if (protocolCall.to.codehash != verification.proof.controlCodeHash) { + return (false); + } + + if (verification.proof.nonce > type(uint64).max - 1) { return (false); } @@ -103,28 +115,27 @@ contract ProtocolVerifier is EIP712, ProtocolIntegration { // which builders or validators may be able to profit via censorship. // Protocols are encouraged to rely on the deadline parameter // to prevent replay attacks. + if (protocolCall.callConfig.needsSequencedNonces()) { if (verification.proof.nonce != signatory.nonce + 1) { return (false); } - unchecked { - ++signatories[verification.proof.from].nonce; - } + unchecked {++signatories[verification.proof.from].nonce;} + // If not sequenced, check to see if this nonce is highest and store // it if so. This ensures nonce + 1 will always be available. } else { - if (verification.proof.nonce > signatory.nonce + 1) { - unchecked { - signatories[verification.proof.from].nonce = uint64(verification.proof.nonce) + 1; - } - } else if (verification.proof.nonce == signatory.nonce + 1) { - unchecked { - ++signatories[verification.proof.from].nonce; - } - } else { - return false; + // NOTE: including the callConfig in the asyncNonceKey should prevent + // issues occuring when a protocol may switch configs between blocking + // and async, since callConfig can double as another seed. + bytes32 asyncNonceKey = keccak256(abi.encode(verification.proof.from, protocolCall.callConfig, verification.proof.nonce + 1)); + + if (asyncNonceFills[asyncNonceKey] != address(0)) { + return (false); } + + asyncNonceFills[asyncNonceKey] = protocolCall.to; } return (true); @@ -174,6 +185,10 @@ contract ProtocolVerifier is EIP712, ProtocolIntegration { return (false); } + if (userCall.metaTx.nonce > type(uint64).max - 1) { + return (false); + } + uint256 userNonce = userNonces[userCall.metaTx.from]; // If the protocol indicated that they only accept sequenced userNonces @@ -186,24 +201,22 @@ contract ProtocolVerifier is EIP712, ProtocolIntegration { if (userCall.metaTx.nonce != userNonce + 1) { return (false); } - unchecked { - ++userNonces[userCall.metaTx.from]; - } + + unchecked {++userNonces[userCall.metaTx.from];} // If not sequenced, check to see if this nonce is highest and store // it if so. This ensures nonce + 1 will always be available. } else { - if (userCall.metaTx.nonce > userNonce + 1) { - unchecked { - userNonces[userCall.metaTx.from] = userCall.metaTx.nonce + 1; - } - } else if (userCall.metaTx.nonce == userNonce + 1) { - unchecked { - ++userNonces[userCall.metaTx.from]; - } - } else { - return false; + // NOTE: including the callConfig in the asyncNonceKey should prevent + // issues occuring when a protocol may switch configs between blocking + // and async, since callConfig can double as another seed. + bytes32 asyncNonceKey = keccak256(abi.encode(userCall.metaTx.from, protocolCall.callConfig, userCall.metaTx.nonce + 1)); + + if (asyncNonceFills[asyncNonceKey] != address(0)) { + return (false); } + + asyncNonceFills[asyncNonceKey] = protocolCall.to; } return (true); diff --git a/src/contracts/atlas/UserVerification.sol b/src/contracts/atlas/UserVerification.sol deleted file mode 100644 index 1f7a4c315..000000000 --- a/src/contracts/atlas/UserVerification.sol +++ /dev/null @@ -1,108 +0,0 @@ -//SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.16; - -import "openzeppelin-contracts/contracts/utils/cryptography/ECDSA.sol"; -import "openzeppelin-contracts/contracts/utils/cryptography/EIP712.sol"; - -import {CallBits} from "../libraries/CallBits.sol"; - -import "../types/CallTypes.sol"; -import "../types/GovernanceTypes.sol"; - -import {UserCall, UserMetaTx} from "../types/CallTypes.sol"; - -// This contract exists so that protocol frontends can sign and confirm the -// calldata for users. Users already trust the frontends to build and verify -// their calldata. This allows users to know that any CallData sourced via -// an external relay (such as FastLane) has been verified by the already-trusted -// frontend -contract UserVerifier is EIP712 { - using ECDSA for bytes32; - using CallBits for uint16; - - bytes32 public constant USER_TYPE_HASH = keccak256( - "UserMetaTx(address from,address to,uint256 deadline,uint256 gas,uint256 nonce,uint256 maxFeePerGas,uint256 value,bytes32 data)" - ); - - mapping(address => uint256) public userNonces; - - constructor() EIP712("UserVerifier", "0.0.1") {} - - // Verify the user's meta transaction - function _verifyUser(ProtocolCall calldata protocolCall, UserCall calldata userCall) - internal - returns (bool) - { - // Verify the signature before storing any data to avoid - // spoof transactions clogging up protocol userNonces - require(_verifySignature(userCall), "ERR-UV01 InvalidSignature"); - - if (userCall.metaTx.to != protocolCall.to) { - return (false); - } - - uint256 userNonce = userNonces[userCall.metaTx.from]; - - // If the protocol indicated that they only accept sequenced userNonces - // (IE for FCFS execution), check and make sure the order is correct - // NOTE: allowing only sequenced userNonces could create a scenario in - // which builders or validators may be able to profit via censorship. - // Protocols are encouraged to rely on the deadline parameter - // to prevent replay attacks. - if (protocolCall.callConfig.needsSequencedNonces()) { - if (userCall.metaTx.nonce != userNonce + 1) { - return (false); - } - unchecked { - ++userNonces[userCall.metaTx.from]; - } - - // If not sequenced, check to see if this nonce is highest and store - // it if so. This ensures nonce + 1 will always be available. - } else { - if (userCall.metaTx.nonce > userNonce + 1) { - unchecked { - userNonces[userCall.metaTx.from] = userCall.metaTx.nonce + 1; - } - } else if (userCall.metaTx.nonce == userNonce + 1) { - unchecked { - ++userNonces[userCall.metaTx.from]; - } - } else { - return false; - } - } - - return (true); - } - - function _getProofHash(UserMetaTx memory metaTx) internal pure returns (bytes32 proofHash) { - proofHash = keccak256( - abi.encode( - USER_TYPE_HASH, - metaTx.from, - metaTx.to, - metaTx.deadline, - metaTx.gas, - metaTx.nonce, - metaTx.maxFeePerGas, - metaTx.value, - keccak256(metaTx.data) - ) - ); - } - - function _verifySignature(UserCall calldata userCall) internal view returns (bool) { - address signer = _hashTypedDataV4(_getProofHash(userCall.metaTx)).recover(userCall.signature); - - return signer == userCall.metaTx.from; - } - - function getUserCallPayload(UserCall memory userCall) public view returns (bytes32 payload) { - payload = _hashTypedDataV4(_getProofHash(userCall.metaTx)); - } - - function nextUserNonce(address user) external view returns (uint256 nextNonce) { - nextNonce = userNonces[user] + 1; - } -} diff --git a/src/contracts/intents-example/SwapIntent.sol b/src/contracts/intents-example/SwapIntent.sol index 1a9ea82e9..402e12126 100644 --- a/src/contracts/intents-example/SwapIntent.sol +++ b/src/contracts/intents-example/SwapIntent.sol @@ -75,7 +75,18 @@ contract SwapIntentController is ProtocolControl { require(address(this) != control, "ERR-PI004 MustBeDelegated"); - require(ERC20(swapIntent.tokenUserSells).balanceOf(_user()) >= swapIntent.amountUserSells, "ERR-PI005 InsufficientBalance"); + uint256 sellTokenBalance = ERC20(swapIntent.tokenUserSells).balanceOf(address(this)); + + // Transfer the tokens that the user is selling into the ExecutionEnvironment + if (sellTokenBalance > swapIntent.amountUserSells) { + ERC20(swapIntent.tokenUserSells).safeTransfer(_user(), sellTokenBalance - swapIntent.amountUserSells); + + } else if (sellTokenBalance > 0) { + _transferUserERC20(swapIntent.tokenUserSells, address(this), swapIntent.amountUserSells - sellTokenBalance); + + } else { + _transferUserERC20(swapIntent.tokenUserSells, address(this), swapIntent.amountUserSells); + } } function _stagingCall(address to, address, bytes4 userSelector, bytes calldata userData) @@ -92,10 +103,8 @@ contract SwapIntentController is ProtocolControl { // There should never be a balance on this ExecutionEnvironment, but check // so that the auction accounting isn't imbalanced by unexpected inventory. - uint256 sellTokenBalance = ERC20(swapIntent.tokenUserSells).balanceOf(address(this)); - if (sellTokenBalance > 0) { - ERC20(swapIntent.tokenUserSells).safeTransfer(_user(), sellTokenBalance); - } + require(swapIntent.tokenUserSells != swapIntent.surplusToken, "ERR-PI008 SellIsSurplus"); + // TODO: If user is Selling Eth, convert it to WETH rather than rejecting. uint256 buyTokenBalance = ERC20(swapIntent.tokenUserBuys).balanceOf(address(this)); if (buyTokenBalance > 0) { @@ -123,7 +132,11 @@ contract SwapIntentController is ProtocolControl { SwapIntent memory swapIntent = abi.decode(stagingReturnData, (SwapIntent)); // Optimistically transfer the searcher contract the tokens that the user is selling - _transferUserERC20(swapIntent.tokenUserSells, searcherTo, swapIntent.amountUserSells); + ERC20(swapIntent.tokenUserSells).safeTransfer(searcherTo, swapIntent.amountUserSells); + + // TODO: Permit69 is currently disabled during searcher phase, but there is currently + // no understood attack vector possible. Consider enabling to save gas on a transfer? + //_transferUserERC20(swapIntent.tokenUserSells, searcherTo, swapIntent.amountUserSells); return true; } diff --git a/src/contracts/interfaces/IPermit69.sol b/src/contracts/interfaces/IPermit69.sol new file mode 100644 index 000000000..9999c5f2f --- /dev/null +++ b/src/contracts/interfaces/IPermit69.sol @@ -0,0 +1,35 @@ +//SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.16; + +import "../types/CallTypes.sol"; +import "../types/VerificationTypes.sol"; + +interface IPermit69 { + // NOTE: IPermit69 only works inside of the Atlas environment - specifically + // inside of the custom ExecutionEnvironments that each user deploys when + // interacting with Atlas in a manner controlled by the DeFi protocol. + + // The name comes from the reciprocal nature of the token transfers. Both + // the user and the ProtocolControl can transfer tokens from the User + // and the ProtocolControl contracts... but only if they each have granted + // token approval to the Atlas main contract, and only during specific phases + // of the Atlas execution process. + + function transferUserERC20( + address token, + address destination, + uint256 amount, + address user, + address protocolControl, + uint16 callConfig + ) external; + + function transferProtocolERC20( + address token, + address destination, + uint256 amount, + address user, + address protocolControl, + uint16 callConfig + ) external; +} diff --git a/src/contracts/interfaces/ITokenTransfers.sol b/src/contracts/interfaces/ITokenTransfers.sol deleted file mode 100644 index eb33ac72e..000000000 --- a/src/contracts/interfaces/ITokenTransfers.sol +++ /dev/null @@ -1,25 +0,0 @@ -//SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.16; - -import "../types/CallTypes.sol"; -import "../types/VerificationTypes.sol"; - -interface ITokenTransfers { - function transferUserERC20( - address token, - address destination, - uint256 amount, - address user, - address protocolControl, - uint16 callConfig - ) external; - - function transferProtocolERC20( - address token, - address destination, - uint256 amount, - address user, - address protocolControl, - uint16 callConfig - ) external; -} diff --git a/src/contracts/protocol/ExecutionBase.sol b/src/contracts/protocol/ExecutionBase.sol index 128727e2f..de88e69f0 100644 --- a/src/contracts/protocol/ExecutionBase.sol +++ b/src/contracts/protocol/ExecutionBase.sol @@ -1,7 +1,7 @@ //SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.16; -import {ITokenTransfers} from "../interfaces/ITokenTransfers.sol"; +import {IPermit69} from "../interfaces/IPermit69.sol"; contract ExecutionBase { @@ -40,7 +40,7 @@ contract ExecutionBase { address destination, uint256 amount ) internal { - ITokenTransfers(msg.sender).transferUserERC20( + IPermit69(msg.sender).transferUserERC20( token, destination, amount, _user(), _control(), _config() ); } @@ -50,7 +50,7 @@ contract ExecutionBase { address destination, uint256 amount ) internal { - ITokenTransfers(msg.sender).transferProtocolERC20( + IPermit69(msg.sender).transferProtocolERC20( token, destination, amount, _user(), _control(), _config() ); }