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

Burn Request Fix #18

Draft
wants to merge 6 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
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
import "@openzeppelin/contracts/access/Ownable2Step.sol";

contract ExampleTransferRules is ITransferRules, Ownable2Step {
Identity public immutable identityContract;

Check warning on line 10 in contracts/CompliantConfidentialERC20/ExampleTransferRules.sol

View workflow job for this annotation

GitHub Actions / ci

Immutable variables name are set to be in capitalized SNAKE_CASE
uint8 private minimumAge;
uint64 public constant transferLimit = (20000 * 10 ** 6);
euint64 public TRANSFER_LIMIT = TFHE.asEuint64(transferLimit); // 20,000 tokens with 6 decimals

Choose a reason for hiding this comment

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

what is it for ?

Choose a reason for hiding this comment

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

transferLimit becomes unused



Check failure on line 14 in contracts/CompliantConfidentialERC20/ExampleTransferRules.sol

View workflow job for this annotation

GitHub Actions / ci

Delete ⏎
mapping(address => bool) public userBlocklist;

event BlacklistUpdated(address indexed user, bool isBlacklisted);
Expand Down
96 changes: 73 additions & 23 deletions contracts/ConfidentialERC20/ConfidentialERC20.sol
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// SPDX-License-Identifier: BSD-3-Clause-Clear
// OpenZeppelin Contracts (last updated v5.0.0) (token/ERC20/ERC20.sol)


Check failure on line 3 in contracts/ConfidentialERC20/ConfidentialERC20.sol

View workflow job for this annotation

GitHub Actions / ci

Delete ⏎
pragma solidity ^0.8.24;

import { IConfidentialERC20 } from "./Interfaces/IConfidentialERC20.sol";
import { IERC20Metadata } from "./Utils/IERC20Metadata.sol";
import "@openzeppelin/contracts/access/Ownable.sol";

import { IERC20Errors } from "./Utils/IERC6093.sol";
import "fhevm/lib/TFHE.sol";
import "fhevm/gateway/GatewayCaller.sol";
Expand All @@ -30,6 +30,10 @@
abstract contract ConfidentialERC20 is Ownable, IConfidentialERC20, IERC20Metadata, IERC20Errors, GatewayCaller {
mapping(address account => euint64) public _balances;

// To safely handle burn requests we must ensure we lock up an amount of token that is in-flight so that the
// we can guarantee there will be sufficient balance to be burnt at the point that _burnCallback runs.
mapping(address account => uint64) public _lockedBalances;

mapping(address account => mapping(address spender => euint64)) internal _allowances;

uint64 public _totalSupply;
Expand All @@ -43,14 +47,16 @@
* All two of these values are immutable: they can only be set once during
* construction.
*/
constructor(string memory name_, string memory symbol_) Ownable(msg.sender) {
constructor(string memory name_, string memory symbol_) {
_name = name_;
_symbol = symbol_;
}
struct BurnRq {
address account;
uint64 amount;
bool exists;

Choose a reason for hiding this comment

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

I'm not convinced this is useful ?
It only ever has true as a value, "non existant" Burn Requests have address(0) as account and amount 0

}

mapping(uint256 => BurnRq) public burnRqs;
/**
* @dev Returns the name of the token.
Expand Down Expand Up @@ -108,9 +114,8 @@
*/
function transfer(address to, euint64 value) public virtual returns (bool) {
require(TFHE.isSenderAllowed(value));
address owner = _msgSender();
ebool isTransferable = TFHE.le(value, _balances[msg.sender]);

Choose a reason for hiding this comment

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

why remove this ?

_transfer(owner, to, value, isTransferable);
address owner = msg.sender;
_transfer(owner, to, value, TFHE.asEbool(true));
return true;
}

Expand Down Expand Up @@ -138,10 +143,11 @@
*/
function approve(address spender, euint64 value) public virtual returns (bool) {
require(TFHE.isSenderAllowed(value));
address owner = _msgSender();
address owner = msg.sender;
_approve(owner, spender, value);
return true;
}

function approve(address spender, einput encryptedAmount, bytes calldata inputProof) public virtual returns (bool) {
approve(spender, TFHE.asEuint64(encryptedAmount, inputProof));
return true;
Expand All @@ -162,7 +168,7 @@
*/
function transferFrom(address from, address to, euint64 value) public virtual returns (bool) {
require(TFHE.isSenderAllowed(value));
address spender = _msgSender();
address spender = msg.sender;
ebool isTransferable = _decreaseAllowance(from, spender, value);
_transfer(from, to, value, isTransferable);
return true;
Expand All @@ -178,6 +184,17 @@
return true;
}

/**
* @dev Checks the available balance of an account exceeds amount
*/
function _hasSufficientBalance(address owner, euint64 amount) internal virtual returns (ebool) {
return TFHE.le(TFHE.add(amount, _lockedBalances[owner]), _balances[owner]);
}

function _hasSufficientBalance(address owner, uint64 amount) internal virtual returns (ebool) {
return _hasSufficientBalance(owner, TFHE.asEuint64(amount));
}

/**
* @dev Moves a `value` amount of tokens from `from` to `to`.
*
Expand All @@ -195,7 +212,12 @@
if (to == address(0)) {
revert ERC20InvalidReceiver(address(0));
}
euint64 transferValue = TFHE.select(isTransferable, value, TFHE.asEuint64(0));
// Enforce sufficient balance constraint universally
euint64 transferValue = TFHE.select(
TFHE.and(_hasSufficientBalance(from, value), isTransferable),
value,
TFHE.asEuint64(0)
);
euint64 newBalanceTo = TFHE.add(_balances[to], transferValue);
_balances[to] = newBalanceTo;
TFHE.allow(newBalanceTo, address(this));
Expand All @@ -204,6 +226,7 @@
_balances[from] = newBalanceFrom;
TFHE.allow(newBalanceFrom, address(this));
TFHE.allow(newBalanceFrom, from);
emit Transfer(from, to, transferValue);
}

/**
Expand Down Expand Up @@ -232,6 +255,25 @@
_totalSupply += value;
}

event BurnRequested(uint256 requestId, address account, uint64 amount);

Choose a reason for hiding this comment

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

events should be at the top of the file


event BurnRequestCancelled(uint256 requestID);

error BurnRequestDoesNotExist(uint256 requestId);

function _cancelBurn(uint256 requestId) internal virtual {

Choose a reason for hiding this comment

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

you must check the calling account is allowed to do this

BurnRq memory burnRequest = burnRqs[requestId];
if (!burnRequest.exists) {
revert BurnRequestDoesNotExist(requestId);
}
address account = burnRequest.account;
uint64 amount = burnRequest.amount;
// Unlock the burn amount unconditionally
_lockedBalances[account] = _lockedBalances[account] - amount;
delete burnRqs[requestId];
emit BurnRequestCancelled(requestId);
}

/**
* @dev Destroys a `value` amount of tokens from `account`, lowering the total supply.
* Relies on the `_update` mechanism.
Expand All @@ -244,7 +286,9 @@
if (account == address(0)) {
revert ERC20InvalidReceiver(address(0));
}
ebool enoughBalance = TFHE.le(amount, _balances[account]);
ebool enoughBalance = _hasSufficientBalance(account, amount);
// Unconditionally lock the burn amount after calculating sufficient balance
_lockedBalances[account] = _lockedBalances[account] + amount;
TFHE.allow(enoughBalance, address(this));
uint256[] memory cts = new uint256[](1);
cts[0] = Gateway.toUint256(enoughBalance);
Expand All @@ -256,22 +300,30 @@
block.timestamp + 100,
false
);

burnRqs[requestID] = BurnRq(account, amount);
burnRqs[requestID] = BurnRq(account, amount, true);
emit BurnRequested(requestID, account, amount);
}

function _burnCallback(uint256 requestID, bool decryptedInput) public virtual onlyGateway {
event InsufficientBalanceToBurn(address account, uint64 burnAmount);

function _burnCallback(uint256 requestID, bool hasEnoughBalance) public virtual onlyGateway {

Choose a reason for hiding this comment

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

the underlyning token transfer is missing

BurnRq memory burnRequest = burnRqs[requestID];
if (!burnRequest.exists) {
revert BurnRequestDoesNotExist(requestID);
}
address account = burnRequest.account;
uint64 amount = burnRequest.amount;
if (!decryptedInput) {
revert("Decryption failed");
// Unlock the burn amount unconditionally
_lockedBalances[account] = _lockedBalances[account] - amount;
delete burnRqs[requestID];
if (!hasEnoughBalance) {
emit InsufficientBalanceToBurn(account, amount);
return;
}
_totalSupply = _totalSupply - amount;
_balances[account] = TFHE.sub(_balances[account], amount);
TFHE.allow(_balances[account], address(this));
TFHE.allow(_balances[account], account);
delete burnRqs[requestID];
}
/**
* @dev Sets `value` as the allowance of `spender` over the `owner` s tokens.
Expand Down Expand Up @@ -324,6 +376,7 @@
TFHE.allow(value, address(this));
TFHE.allow(value, owner);
TFHE.allow(value, spender);
emit Approval(owner, spender, value);
}

/**
Expand All @@ -334,14 +387,11 @@
*/
function _decreaseAllowance(address owner, address spender, euint64 amount) internal virtual returns (ebool) {
euint64 currentAllowance = _allowances[owner][spender];

ebool allowedTransfer = TFHE.le(amount, currentAllowance);

ebool canTransfer = TFHE.le(amount, _balances[owner]);
ebool isTransferable = TFHE.and(canTransfer, allowedTransfer);
ebool isTransferable = TFHE.le(amount, currentAllowance);

Choose a reason for hiding this comment

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

shouldn't the eamount already containt this check and passed when this func is called ?

_approve(owner, spender, TFHE.select(isTransferable, TFHE.sub(currentAllowance, amount), currentAllowance));
return isTransferable;
}

/**
* @dev Increases `owner` s allowance for `spender` based on spent `value`.
*
Expand All @@ -350,7 +400,7 @@
*/
function _increaseAllowance(address spender, euint64 addedValue) internal virtual returns (ebool) {

Choose a reason for hiding this comment

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

why does this function even exist

require(TFHE.isSenderAllowed(addedValue));
address owner = _msgSender();
address owner = msg.sender;
ebool isTransferable = TFHE.le(addedValue, _balances[owner]);
euint64 newAllowance = TFHE.add(_allowances[owner][spender], addedValue);
TFHE.allow(newAllowance, address(this));
Expand All @@ -373,7 +423,7 @@

function decreaseAllowance(address spender, euint64 subtractedValue) public virtual returns (ebool) {
require(TFHE.isSenderAllowed(subtractedValue));
return _decreaseAllowance(_msgSender(), spender, subtractedValue);
return _decreaseAllowance(msg.sender, spender, subtractedValue);
}

function decreaseAllowance(
Expand Down
30 changes: 8 additions & 22 deletions contracts/ConfidentialERC20/ConfidentialToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,26 @@

import "./ConfidentialERC20.sol";
import "fhevm/lib/TFHE.sol";

import "@openzeppelin/contracts/access/Ownable.sol";
/**
* @dev Example Implementation of the {ConfidentialERC20} contract, providing minting and additional functionality.
*/
contract ConfidentialToken is ConfidentialERC20 {
address private _owner;

Check failure on line 11 in contracts/ConfidentialERC20/ConfidentialToken.sol

View workflow job for this annotation

GitHub Actions / ci

Delete ⏎⏎

/**
* @dev Sets the initial values for {name} and {symbol}, and assigns ownership to the deployer.
*/
constructor(string memory name_, string memory symbol_) ConfidentialERC20(name_, symbol_) {
_owner = msg.sender;
constructor(string memory name_, string memory symbol_) ConfidentialERC20(name_, symbol_)Ownable(msg.sender) {

Check failure on line 16 in contracts/ConfidentialERC20/ConfidentialToken.sol

View workflow job for this annotation

GitHub Actions / ci

Replace Ownable(msg.sender)·{⏎⏎···· with ·Ownable(msg.sender)·{

Choose a reason for hiding this comment

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

why reintroduce the owner


}

/**
* @dev Mint new tokens.
*
*/
function mint(address to, uint64 amount) public {
require(msg.sender == _owner, "Only owner");
function mint(address to, uint64 amount) public onlyOwner() {

Check failure on line 24 in contracts/ConfidentialERC20/ConfidentialToken.sol

View workflow job for this annotation

GitHub Actions / ci

Replace ()·{⏎ with ·{

_mint(to, amount);
}

Expand All @@ -31,23 +31,9 @@
*
*/
function burn(address from, uint64 amount) public {
require(msg.sender == _owner, "Only owner");
burn(from, amount);
_requestBurn(from, amount);
}

Check failure on line 35 in contracts/ConfidentialERC20/ConfidentialToken.sol

View workflow job for this annotation

GitHub Actions / ci

Delete ⏎⏎⏎

/**
* @dev Change the owner
*
*/
function transferOwnership(address newOwner) public override {
require(msg.sender == _owner, " Only owner");
_owner = newOwner;
}

/**
* @dev Get the owner of the contract.
*/
function owner() public view override returns (address) {
return _owner;
}

}
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// SPDX-License-Identifier: BSD-3-Clause-Clear
// OpenZeppelin Contracts (last updated v5.0.0) (token/ERC20/IERC20.sol)
import "fhevm/lib/TFHE.sol";

pragma solidity ^0.8.24;

import "fhevm/lib/TFHE.sol";
/**
* @dev Interface of the ERC-20 standard as defined in the ERC.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// SPDX-License-Identifier: BSD-3-Clause-Clear
pragma solidity ^0.8.24;

import "fhevm/lib/TFHE.sol";
import { IConfidentialERC20 } from "./IConfidentialERC20.sol";

/**
* @dev Interface of the Confidential ERC-20 wrapper.
*/

interface IConfidentialERC20Wrapper is IConfidentialERC20 {

Check failure on line 11 in contracts/ConfidentialERC20/Interfaces/IConfidentialERC20Wrapper.sol

View workflow job for this annotation

GitHub Actions / ci

Delete ····
/**

Check failure on line 12 in contracts/ConfidentialERC20/Interfaces/IConfidentialERC20Wrapper.sol

View workflow job for this annotation

GitHub Actions / ci

Delete ····
* @dev Returns the base ERC-20 token address.

Check failure on line 13 in contracts/ConfidentialERC20/Interfaces/IConfidentialERC20Wrapper.sol

View workflow job for this annotation

GitHub Actions / ci

Delete ····
*/

Check failure on line 14 in contracts/ConfidentialERC20/Interfaces/IConfidentialERC20Wrapper.sol

View workflow job for this annotation

GitHub Actions / ci

Delete ····
function baseERC20() external view returns (address);

/**
* @dev Returns the decimals of the base ERC-20 token.
*/
function decimals() external view returns (uint8);

/**
* @dev Wraps a `amount` of base ERC-20 tokens into the wrapper.
*/
function wrap(uint64 amount) external;

/**
* @dev Unwraps a `amount` of wrapped tokens into the base ERC-20 token.
*/
function unwrap(uint256 amount) external;

function setUnwrapStatus(address account, bool status) external;



event Wrap(address indexed account, uint64 amount);
event Unwrap(address indexed account, uint64 amount);

}
3 changes: 2 additions & 1 deletion contracts/ConfidentialERC20/Utils/IERC20Metadata.sol
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// SPDX-License-Identifier: BSD-3-Clause-Clear
// OpenZeppelin Contracts (last updated v5.0.0) (token/ERC20/extensions/IERC20Metadata.sol)


pragma solidity ^0.8.24;

import { IConfidentialERC20 } from "../Interfaces/IConfidentialERC20.sol";
import "fhevm/lib/TFHE.sol";

/**
* @dev Interface for the optional metadata functions from the ERC-20 standard.
Expand Down
2 changes: 1 addition & 1 deletion contracts/ConfidentialERC20/Utils/IERC6093.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: BSD-3-Clause-Clear
// OpenZeppelin Contracts (last updated v5.0.0) (interfaces/draft-IERC6093.sol)

pragma solidity ^0.8.24;

Check warning on line 3 in contracts/ConfidentialERC20/Utils/IERC6093.sol

View workflow job for this annotation

GitHub Actions / ci

Found more than One contract per file. 3 contracts found!
import "fhevm/lib/TFHE.sol";
/**
* @dev Standard ERC-20 Errors
Expand Down
Loading
Loading