circlefin / stablecoin-evm Goto Github PK

Circle tokens use EIP-712 signatures, but only have a DOMAIN_SEPARATOR() getter. This getter is not enough for applications to request EIP-712 signatures from users because the full domain parameters are required. The only available solution is for applications to hardcode known domain objects, which significantly limits adoption of EIP-712, and can become a problem if Circle tokens ever need to update their domain.

ERC-5267 defines a standard eip712Domain() function for contracts to publish their domain parameters so that applications can retrieve them. Please consider adding this function to Circle tokens.

For reference here is OpenZeppelin's implementation: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v5.0.0/contracts/utils/cryptography/EIP712.sol#L114-L137

Repeatedly in the code (Controller.sol lines 56, 66; MintController.sol lines 60, 71, 80, 92), the public keyword appears after the custom modifiers. According to the Solidity docs on Function declaration, however, “The visibility modifier for a function should come before other modifiers”.

Hi,

I am Nicolas, the manager at SimpleSwap.io, an instant cryptocurrency exchange.

All import statements use single quotes, double quotes are recommended in the Solidity Style Guide.

/**

• @dev Upgrade the backing implementation of the proxy and call a function
• on the new implementation.
  */
  function upgradeToAndCall(address newImplementation, bytes data) payable external ifAdmin {
  _upgradeTo(newImplementation);
  require(address(this).call.value(msg.value)(data));
  }

as i just checked openzepplin's latest code,
there's bug in last line, it should be newImplementation instead of address(this)
you code will call

function _willFallback() internal {
require(msg.sender != _admin(), "Cannot call fallback function from the proxy admin");
super._willFallback();
}

which will throw since msg.sender is admin

My account was severely compromised and hacked how can I release and retrieve my funds

There is an inconsistency in function parameter naming. In Controller they all start with an underscore, while in MintController only those of constructor and setMinterManager do. Consider using underscores in all parameter names.

I'm not sure if this issue is specific to Sepolia or not but I am getting this error during the "Initializing proxy contract" step: Fail with error 'Cannot call fallback function from the proxy admin' https://sepolia.etherscan.io/tx/0xc933ae34f9255647baf67215b3344357065f5b1a47f831ebe7919eceb39a8fea

On a related note, since Ropsten is now deprecated and Goerli is borderline unusable due to high gas fees and will be deprecated soon as well, would it be a good idea to include config for Sepolia in the truffle-config.js?

Hello Team,

Encountered an error while compiling or processing the source file FiatTokenV2_2_flattened.sol due to the presence of multiple SPDX license identifiers. The file contains two SPDX license identifiers, namely "Apache-2.0" and "MIT", resulting in the following error message:

.deps/github/circlefin/stablecoin-evm/contracts/v2/FiatTokenV2_2_flattened.sol: ParserError: Multiple SPDX license identifiers found in source file. Use "AND" or "OR" to combine multiple licenses. Please see https://spdx.org for more information.

As per the SPDX specification, a source file should contain only one SPDX license identifier. If multiple licenses are applicable, they should be combined using "AND" or "OR" as appropriate. To address this, the SPDX license identifier "SPDX-License-Identifier: Apache-2.0 OR MIT" has been used.

Steps to Reproduce:

• Attempt to compile or process the source file FiatTokenV2_2_flattened.sol.
• Observe the error message indicating the presence of multiple SPDX license identifiers.

Additional IMP issue:
Due to this issue, we are unable to interact with the functions of the FiatTokenV2_2 smart contract. This slows down the contract's verification process as well.

Refer to the attached screenshot:

Hi,

I was searching for a "verified" FiatToken contract on etherscan, and couldn't find any.

Your website points to 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48, which seems to be the Proxy contract.

I also searched through the transactions made by the deployer, but couldn't find a deployment of FiatToken.

Having implemented EIP-712 signing and use of transferWithAuthorization instead of transfer, I was expecting the contract to emit an event. But I can't see any emitted on Etherscan or find any reference to it in the code.

Is it really true that it does not emit an event? @petejkim
If so can we please get that on the feature list for the next update.

There is a double space in line 52 of MintController.sol (after the equal sign)

What is the purpose of the “Approve USDC spend limit” call? Does it make this call once per account per recipient account?

Originally posted by @almccann in #313

removeController does not check if the controller is set before removal, allowing for subsequent calls to the function, which will confusingly emit repeated ControllerRemoved events with the same address. Consider whether this check should be included in order to avoid this behavior.

When monitoring Transfer and AuthorizationUsed (transferWithAuthorization) events, it is made more difficult by the latter not including/exposing the from, to and value parameters in the same way as the former.

Transfer:

"returnValues": {
    "0": "0xD9d3768621774c3C357b9B5fcB23B5735eA2c7aE",
    "1": "0x53574F64F817290e622842462cc2B8708c1a3808",
    "2": "80000",
    "from": "0xD9d3768621774c3C357b9B5fcB23B5735eA2c7aE",
    "to": "0x53574F64F817290e622842462cc2B8708c1a3808",
    "value": "80000"
}

AuthorizationUsed:

"returnValues": {
    "0": "0xD9d3768621774c3C357b9B5fcB23B5735eA2c7aE",
    "1": "0x1f05761787aabb9d5e1b89e4572163db5ddd76c2e18bd8ccd78d46e6e18bcf09",
    "authorizer": "0xD9d3768621774c3C357b9B5fcB23B5735eA2c7aE",
    "nonce": "0x1f05761787aabb9d5e1b89e4572163db5ddd76c2e18bd8ccd78d46e6e18bcf09",
}

How do I get the from, to and value parameters from an AuthorizationUsed event?

Are there any considerations of having FiatTokenV3 be ERC865-compatible? I think it would provide a significant boost to UX if members of the CENTRE ecosystem could allow users to pay fees in USDC in a chain-native manner when applicable.

I'd be happy to lead the effort if possible.

Consider moving MinterManagementInterface to an individual file and importing it from MinterController.sol, thus allowing it to be cleanly imported from other files in the project.

Hi - can someone explain why USDC only has 6 decimal places of precision whereas most ERC 20 tokens have 18? What is the reasoning?

While trying to set up the project with the Hardhat framework using the repository circlefin/stablecoin-evm, encountering the following error related to "foundry-forge": "Error HH413: File forge-std/Test.sol => lib/forge-std/src/Test.sol, imported from test/scripts/deploy/TestUtils.sol, not found", Please check the attached screenshot for reference.

Steps to reproduce:

• Installed "Node 16.14.0", "Yarn 1.22.19", and "Foundry" (forge 0.2.0 (f6208d8 2024-04-04T00:18:54.847734725Z)).
• Created ".env" file.
• Created "blacklist.remote.json" file.
• Entered the command "yarn compile".

Please advise on how to resolve this issue. Thank you.

The incrementMinterAllowance function can be called with a parameter value of allowanceIncrement = 0. This would lead to gas expenditure with no state changes or useful computation.

Consider checking that allowanceIncrement > 0 to avoid unnecessary gas costs.

The function setMinterManager does not verify if the input parameter _newMinterManager implements MinterManagementInterface now. So the variable minterManager maybe set to the wrong value by accident.

    function setMinterManager(address _newMinterManager) public onlyOwner {
        emit MinterManagerSet(address(minterManager), _newMinterManager);
        minterManager = MinterManagementInterface(_newMinterManager);
    }

I suggest the following new codes:

    function setMinterManager(address _newMinterManager) public onlyOwner {
        require(
            !MinterManagementInterface(_newMinterManager).isMinter(address(0)),
            "invalid minter mangaer"
        );
        emit MinterManagerSet(address(minterManager), _newMinterManager);
        minterManager = MinterManagementInterface(_newMinterManager);
    }

Or check it by ERC-165.

Functions incrementMinterAllowance and internal_setMinterAllowance in MintController are not correctly indented.

The line wrapping is inconsistent in the codebase. Comments appear to be wrapped at around 86 characters but some parts of the code extend beyond that limit (e.g. line 49 of MintController.sol, which goes to 128). Consider adhering to a single limit and applying it throughout. The recommended width is 80 columns.

Hi,

I am Nicolas, the manager at SimpleSwap.io, an instant cryptocurrency exchange.

We present a simple and easy-to-use platform that works without registration and limits and allows users to make swaps in just two clicks.

We successfully support USDC on our service for a long time and we have been exchanging USDC for more than 200 coins to our customers. I tried to find contact with your team but without results.

I'm sorry that I invaded here, but maybe you can tell me where I can turn. I would like to make contact so we can have an interaction.

Dear team,

I have a question here that does USDC allow user transfer USDC to other address through meta transactions (without pay gas)? If yes, could you please give me an example?

Thanks in advanced.

Controller explicitly defines an empty constructor, which, according to the Solidity docs, is exactly what the contract would assume if this piece of code was absent. Consider removing the empty constructor for clarity.

I have tried both of these for pools:

//address WETHUSDCPool = 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640;
from https://etherscan.io/address/0x88e6a0c2ddd26feeb64f039a2c41296fcb3f5640#readContract -- this looks like the right pool from searching

  address WETHUSDCPool = 0x8ad599c3A0ff1De082011EFDDc58f1908eb6e6D8; //?? linked from uniswap
from https://info.uniswap.org/#/pools/0x8ad599c3a0ff1de082011efddc58f1908eb6e6d -- this one I get from UNISWAP

I always get an error consulting the Uniswap V3 Oracle Library with these pools, it reverts which it can do if there are no reserves.

What is the correct WETH-USDC Pool? Thank you!!

Hi

I'm trying to send USDC as the same way that I'm doing with BNB and TUSD. But the transactions fails, anybody here can help me? I'm using the same ABI for all of them.

https://etherscan.io/tx/0xc2cdc9d93d5ca1926f78e2d7c46d31f181b3a5bb79397bd659df247ec7776ba0

https://etherscan.io/tx/0xbe8a3cdca33370153de9bba5abbfcbb9a2ff5a9f8a18b302b79abf316457f713

https://etherscan.io/tx/0xec33f5fba3357cf90546475277fb906da0d89f032377c9f47bfefe43d90244e0

https://etherscan.io/tx/0x3706978515b0949b99594ab413caec73f07dae1d8784ad4795e8fb7ea2658d2f

async function send(recipientAddress, amount, privateKey) {
const data = contract.methods.transfer(recipientAddress, web3.utils.toWei(amount)).encodeABI();
const wallet = new ethers.Wallet(privateKey, provider);
const transaction = {
gasPrice: web3.eth.gasPrice,
gasLimit: 90000,
to: contractAddress,
data,
};

const transactionHash = await wallet.sendTransaction(transaction);
transactionHash.tx = transactionHash.hash
return transactionHash;

}

##@

0xc198631d5ef54d140f58260faa0f3e8032cdae90796bd9bd3dfeea93226e6b06

Consider restricting the visibility of the controllers and minterManager variables to internal or private and providing the required getter functions as a good encapsulation practice.

No way to decrement an allowance without reconfiguring minterThe MintController contract provides a mechanism to increment a minter's allowance that only works if the minter is active, thus preventing the accidental reactivation of inactive minters, and, as described in the inline documentation, the usage of a signed transaction to reactivate a minter. This mechanism is implemented in the incrementMinterAllowance function, which in turn calls minterManager.isMinter to perform the required check.

The contract, however, provides no analogous mechanism for decreasing a minter's allowance, meaning that this can only be achieved by “resetting” the minter through the configureMinter function.

what does the google sheet in Verification provide?

    centre-tokens/verification/GoogleSheets/index.js

i requested permission but im guessing its important.

do you have any template for this sheet?

thanks

In line 61 of MintController, the minterManager variable is implicitly converted to the address type. Starting from Solidity version v0.5.0, implicit address conversions issue an error. Consider explicitly casting minterManager to the address type in case the decision is later made to use a newer version of Solidity.

I am currently looking for it...my accounts have been severely compromised and I have to do a security check

Originally posted by @Mehduzza420 in #348 (comment)

The proxy contract (the erc20 address) do not show the admin and implementation values in read contract tab of etherscan:

https://etherscan.io/token/0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48#readContract

This is because of the ifAsmin modifier:

function admin() external view ifAdmin returns (address) {
  return _admin();
}

modifier ifAdmin() {
  if (msg.sender == _admin()) {
    _;
  } else {
    _fallback();
  }
}

Fixes #2567

PR Checklist

• Tests
• Documentation
• Changelog entry

Originally posted by @Amxx in OpenZeppelin/openzeppelin-contracts#2568

Several functions (configureController and removeController in Controller, and setMinterManager in MintController) return hardcoded boolean values. Consider documenting this if it arises from the need of conforming to an interface.

In several places in the code, addresses are passed as parameters to functions. In many of these instances, the functions do not validate that the passed address is not the null address 0. In the Controller contract, this happens in functions configureController (which only checks _worker) and removeController.

In the MintController contract, on the other hand, it might be expected that the constructor sets the minterManager variable to 0, but the setMinterManager function does not prevent setting it to 0.

We have an architecture where a smart contract is deployed to a CREATE2 address when the address receives USDC.

This works correctly when using transfer to send USDC to the CREATE2 address.
It also works when using transferWithAuthorization to send USDC to a normal EOA.

But using transferWithAuthorization to send USDC to the CREATE2 address the transaction fails.
Are there differences between the two when using CREATE2, is this behavior expected?

See this transaction on Etherscan for an example.

Using a library like eth-revert-reason to get the revert reason does not give any further information:
Error: Unable to decode revert reason.

I'm fresh to this project. It seems the deploy guide is not very clear. I failed to deploy according to the document. It seems that my PROXY_ADMIN_ADDRESS is wrong. The below text is my settings in .env except private key:

TOKEN_NAME=USDC
TOKEN_SYMBOL=USDC
TOKEN_CURRENCY=USD
TOKEN_DECIMALS=6
PROXY_ADMIN_ADDRESS="0xD4CE02705041F04135f1949Bc835c1Fe0885513c"
OWNER_ADDRESS="0xD4CE02705041F04135f1949Bc835c1Fe0885513c"
MASTER_MINTER_OWNER_ADDRESS="0xD4CE02705041F04135f1949Bc835c1Fe0885513c"
GAS_MULTIPLIER=110
BLACKLIST_FILE_NAME=blacklist.remote.json

My deploy command is:

forge script -vv --broadcast scripts/deploy/deploy-fiat-token.s.sol --rpc-url <MY_RPC_URL>

Got error:

[⠒] Compiling...
No files changed, compilation skipped
Traces:
  [348611] → new SignatureChecker@0x518Fc3A0F2Ef69A267669BA09E7b56188E6C04FE
    └─ ← [Return] 1741 bytes of code

  [8161444] → new DeployFiatToken@0x5b73C5498c1E3b4dbA84de0F1833c4a029d90519
    └─ ← [Return] 40642 bytes of code

  [270378] DeployFiatToken::setUp()
    ├─ [0] VM::envString("TOKEN_NAME") [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envString("TOKEN_SYMBOL") [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envString("TOKEN_CURRENCY") [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envUint("TOKEN_DECIMALS") [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envOr("FIAT_TOKEN_IMPLEMENTATION_ADDRESS", 0x0000000000000000000000000000000000000000) [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envAddress("PROXY_ADMIN_ADDRESS") [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envAddress("MASTER_MINTER_OWNER_ADDRESS") [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envAddress("OWNER_ADDRESS") [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envOr("PAUSER_ADDRESS", 0xD4CE02705041F04135f1949Bc835c1Fe0885513c) [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envOr("BLACKLISTER_ADDRESS", 0xD4CE02705041F04135f1949Bc835c1Fe0885513c) [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envOr("LOST_AND_FOUND_ADDRESS", 0xD4CE02705041F04135f1949Bc835c1Fe0885513c) [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] VM::envUint("DEPLOYER_PRIVATE_KEY") [staticcall]
    │   └─ ← [Return] <env var value>
    ├─ [0] console::log("TOKEN_NAME: '%s'", "USDC") [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("TOKEN_SYMBOL: '%s'", "USDC") [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("TOKEN_CURRENCY: '%s'", "USD") [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("TOKEN_DECIMALS: '%s'", 6) [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("FIAT_TOKEN_IMPLEMENTATION_ADDRESS: '%s'", 0x0000000000000000000000000000000000000000) [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("PROXY_ADMIN_ADDRESS: '%s'", 0xD4CE02705041F04135f1949Bc835c1Fe0885513c) [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("MASTER_MINTER_OWNER_ADDRESS: '%s'", 0xD4CE02705041F04135f1949Bc835c1Fe0885513c) [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("OWNER_ADDRESS: '%s'", 0xD4CE02705041F04135f1949Bc835c1Fe0885513c) [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("PAUSER_ADDRESS: '%s'", 0xD4CE02705041F04135f1949Bc835c1Fe0885513c) [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("BLACKLISTER_ADDRESS: '%s'", 0xD4CE02705041F04135f1949Bc835c1Fe0885513c) [staticcall]
    │   └─ ← [Stop]
    ├─ [0] console::log("LOST_AND_FOUND_ADDRESS: '%s'", 0xD4CE02705041F04135f1949Bc835c1Fe0885513c) [staticcall]
    │   └─ ← [Stop]
    └─ ← [Stop]

  [6551512] DeployFiatToken::run()
    ├─ [0] VM::startBroadcast(<pk>)
    │   └─ ← [Return]
    ├─ [4724944] → new FiatTokenV2_2@0x6F9d086cEFC1dB0836FEd3a53A703b6Dac89e87F
    │   └─ ← [Return] 23464 bytes of code
    ├─ [76219] FiatTokenV2_2::initialize("", "", "", 0, 0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000001)
    │   └─ ← [Stop]
    ├─ [65980] FiatTokenV2_2::initializeV2("")
    │   └─ ← [Stop]
    ├─ [23205] FiatTokenV2_2::initializeV2_1(0x0000000000000000000000000000000000000001)
    │   └─ ← [Stop]
    ├─ [4275] FiatTokenV2_2::initializeV2_2([], "")
    │   └─ ← [Stop]
    ├─ [404525] → new FiatTokenProxy@0xC6004B9aa8AD300D917DE8aB6879D91CA93dAdF4
    │   └─ ← [Return] 1798 bytes of code
    ├─ [1114770] → new MasterMinter@0xC52422EF3e3A84e58F93A530e304Ed69363b5E8F
    │   └─ ← [Return] 5346 bytes of code
    ├─ [2113] MasterMinter::transferOwnership(0xD4CE02705041F04135f1949Bc835c1Fe0885513c)
    │   ├─ emit OwnershipTransferred(previousOwner: 0xD4CE02705041F04135f1949Bc835c1Fe0885513c, newOwner: 0xD4CE02705041F04135f1949Bc835c1Fe0885513c)
    │   └─ ← [Stop]
    ├─ [2020] FiatTokenProxy::changeAdmin(0xD4CE02705041F04135f1949Bc835c1Fe0885513c)
    │   ├─ emit AdminChanged(previousAdmin: 0xD4CE02705041F04135f1949Bc835c1Fe0885513c, newAdmin: 0xD4CE02705041F04135f1949Bc835c1Fe0885513c)
    │   └─ ← [Stop]
    ├─ [445] FiatTokenProxy::initialize("USDC", "USDC", "USD", 6, MasterMinter: [0xC52422EF3e3A84e58F93A530e304Ed69363b5E8F], 0xD4CE02705041F04135f1949Bc835c1Fe0885513c, 0xD4CE02705041F04135f1949Bc835c1Fe0885513c, 0xD4CE02705041F04135f1949Bc835c1Fe0885513c)
    │   └─ ← [Revert] revert: Cannot call fallback function from the proxy admin
    └─ ← [Revert] revert: Cannot call fallback function from the proxy admin



== Logs ==
  TOKEN_NAME: 'USDC'
  TOKEN_SYMBOL: 'USDC'
  TOKEN_CURRENCY: 'USD'
  TOKEN_DECIMALS: '6'
  FIAT_TOKEN_IMPLEMENTATION_ADDRESS: '0x0000000000000000000000000000000000000000'
  PROXY_ADMIN_ADDRESS: '0xD4CE02705041F04135f1949Bc835c1Fe0885513c'
  MASTER_MINTER_OWNER_ADDRESS: '0xD4CE02705041F04135f1949Bc835c1Fe0885513c'
  OWNER_ADDRESS: '0xD4CE02705041F04135f1949Bc835c1Fe0885513c'
  PAUSER_ADDRESS: '0xD4CE02705041F04135f1949Bc835c1Fe0885513c'
  BLACKLISTER_ADDRESS: '0xD4CE02705041F04135f1949Bc835c1Fe0885513c'
  LOST_AND_FOUND_ADDRESS: '0xD4CE02705041F04135f1949Bc835c1Fe0885513c'
Error:
script failed: revert: Cannot call fallback function from the proxy admin

The Centre Token minting contracts are in general well documented, and the documentation adheres to the NatSpec format. There is, however, room for improvement. First, MasterMinter.sol has no documentation at all. Second, while all contracts have a short description of each function, all comments make use of only the @dev tag among the many available in NatSpec.

Yet another instance where documentation can be improved is the following comment in Controller.sol: “set the controller of a particular _worker”. This might lead the user to think that each worker has a single controller, while the system in fact allows it to have more than one.

Finally, the comment “allows control of configure/remove minter…” in MintController seems to refer to the configureMinter and removeMinter functions, but these are not referenced using their full names.

Consider adding complete docstrings for all contracts, struct fields, state variables, mapping keys and functions, and having comments more clearly and faithfully represent the functionality of the contracts.

What is the purpose of the “Approve USDC spend limit” call? Does it make this call once per account per recipient account?