Skip to content

Commit

Permalink
cleaned up async nonce processing, fixed a bug w/ token allowance sys…
Browse files Browse the repository at this point in the history
…tem and intent example (permit69 is disabled during searcher stage)
  • Loading branch information
thogard785 committed Aug 11, 2023
1 parent 7b5cf39 commit 2d58f19
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 176 deletions.
1 change: 0 additions & 1 deletion src/contracts/atlas/Atlas.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
3 changes: 3 additions & 0 deletions src/contracts/atlas/ExecutionEnvironment.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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:]
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/contracts/atlas/Factory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion src/contracts/atlas/ProtocolIntegration.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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] =
Expand All @@ -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});
}

Expand Down
71 changes: 42 additions & 29 deletions src/contracts/atlas/ProtocolVerification.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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") {}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down
108 changes: 0 additions & 108 deletions src/contracts/atlas/UserVerification.sol

This file was deleted.

25 changes: 19 additions & 6 deletions src/contracts/intents-example/SwapIntent.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand Down
Loading

0 comments on commit 2d58f19

Please sign in to comment.