Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colin/liquid exchange #5

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions contracts/arbiter_token.sol
Original file line number Diff line number Diff line change
@@ -1,12 +1,25 @@
pragma solidity ^0.8.10;
import "../../openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forge install transmissions11/solmate

Better module!


contract ArbiterToken is ERC20 {
address public admin;

constructor(string memory name, string memory symbol)
ERC20(name, symbol) {

admin = msg.sender; // Set the contract deployer as the initial admin
}

// Our admin lock
modifier onlyAdmin() {
require(msg.sender == admin, "Only admin can call this function");
_;
}

function mint(address receiver, uint256 amount) public {
function mint(address receiver, uint256 amount) public onlyAdmin {
_mint(receiver, amount);
}

function mintMax(address receiver) public onlyAdmin {
_mint(receiver, type(uint256).max);
}
}
19 changes: 0 additions & 19 deletions contracts/inf_liq_market.sol

This file was deleted.

75 changes: 75 additions & 0 deletions contracts/liquid_exchange.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// SPDX-License-Identifier: MIT
// compiler version must be greater than or equal to 0.8.17 and less than 0.9.0
pragma solidity ^0.8.17;
// import "solmate/utils/FixedPointMathLib.sol"; // This import is correct given Arbiter's foundry.toml
import "../../portfolio/lib/solmate/src/utils/FixedPointMathLib.sol"; // This import goes directly to the contract
import "../../openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
Comment on lines +5 to +6

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In foundry.toml, you can make a 'remappings' entry that will make these imports easier:

remappings = [
 'solmate/=lib/portfolio/lib/solmate/src'
]

Then in contract:
import "solmate/utils/FixedPointMath.sol";


/**
* @dev Implementation of the test interface for Arbiter writing contracts.
*/
contract LiquidExchange {
using FixedPointMathLib for int256;
using FixedPointMathLib for uint256;
address public admin;
ERC20 public arbiter_token_x;
address public arbiter_token_x_address;
ERC20 public arbiter_token_y;
address public arbiter_token_y_address;
uint256 public price;
uint256 public constant WAD = 10**18;

// Each LiquidExchange contract will be deployed with a pair of token addresses and an initial price
constructor(address _arbiter_token_x, address _arbiter_token_y, uint256 _price) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Styling nit:

Solidity is usually camel case, but I don't mind.
I DO mind leading underscores. Leading underscores are reserved for internal variables and functions. Trailing underscores are good for arguments.

admin = msg.sender; // Set the contract deployer as the initial admin
arbiter_token_x = ERC20(_arbiter_token_x);
arbiter_token_x_address = _arbiter_token_x;
Comment on lines +25 to +26

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need two! Just need the address, and you can cast between the types like this:

address token = 0x01
ERC20 token0 = ERC20(token);
address token1 = address(token0);

arbiter_token_y = ERC20(_arbiter_token_y);
arbiter_token_y_address = _arbiter_token_x;
price = _price;
}

// Our admin lock
modifier onlyAdmin() {
require(msg.sender == admin, "Only admin can call this function");
_;
}

event PriceChange(uint256 price);
event Swap(address token_in, address token_out, uint256 amount_in, uint256 amount_out, address to);

// Admin only function to set the price of x in terms of y
function setPrice(uint256 _price) public onlyAdmin {
price = _price;
emit PriceChange(price);
}

// Returns the price of x in terms of y (i.e., y is the numeraire)
function getPrice() public view returns (uint256) {
return price;
}
Comment on lines +48 to +50

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While getPrice is fine, since price is a public variable, it creates a getter price() already


// TODO: This function is NOT completed yet. It is just a placeholder for now.
function swap(address _token_in_address, uint256 _amount_in) public {
uint256 amount_out;
address token_out_address;
if (_token_in_address == arbiter_token_x_address) {
// amount_out = FixedPointMathLib.mulWadDown(_amount_in, price);
amount_out = _amount_in * price;
// Set allowances
// arbiter_token_x.increaseAllowance(msg.sender, _amount_in);
// arbiter_token_y.increaseAllowance(admin, amount_out); //might not be necessary if we increase the allowance for the manager outside of this
Comment on lines +59 to +61

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So:
You call this contract, which means its asking for YOUR allowance for THIS contract to spend tokens. This means you have to send a transaction to the token to approve this contract as a spender.

// Transfer amount in and amount out
arbiter_token_x.transferFrom(msg.sender, admin, _amount_in);
arbiter_token_y.transferFrom(admin, msg.sender, amount_out);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The from argument in transferFrom should ALWAYS be msg.sender.

You should use transfer here, and have this contract hold the tokens.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this does work for admin.

} else if (_token_in_address == arbiter_token_y_address) {
amount_out = FixedPointMathLib.divWadDown(_amount_in, price);
// Transfer amount in and amount out
arbiter_token_y.transferFrom(msg.sender, admin, _amount_in);
arbiter_token_x.transferFrom(admin, msg.sender, amount_out);
} else {
revert("Invalid token");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a custom error:

error InvalidToken();

revert InvalidToken();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on this? @Autoparallel, I am at a spot where i want to decode the events on this market.

}
emit Swap(_token_in_address, token_out_address, _amount_in, amount_out, msg.sender);
}
}