Generally speaking, we encourage standards to omit functions that can only be called by the owner/deployer of a contract (like setLimits and presumably setLockbox) because the owner of a contract can be expected to understand the specifics of a particular implementation. There’s no need for interoperability.
1 Like
@SamWilsn thanks that you took a time to look at this.
@arjunbhuptani The more I work with xERC20 the more I agree with Sam. IMHO the standard should enforce only the critical interoperability parts.
I think it should be broken into core (xERC20 basic interoperability standard) and related extensions (lockbox ERC20, lockbox native) if desired.
Example, where I am in doubt the xERC20 implementation have to include following (in case there is no related lockbox on the chain):
All EIP-XX tokens MUST implement the following interface.
...
event LockboxSet(address _lockbox);
function setLockbox(address _lockbox) external;
...
1 Like
Additionally we had to separate the xERC20 minting by bridges vs by lockbox:
/**
* @notice Mints xERC20 tokens for a user by the `lockbox`
* @dev Can only be called by a lockbox
* @param _user The address of the user who needs tokens minted
* @param _amount The amount of tokens being minted
*/
function mintByLockbox(address _user, uint256 _amount) external virtual onlyLockbox {}
Would we comply with the standard when lockbox is not minting via mint() function?
1 Like
Example of the extension - we needed to implement this accounting for the xERC20 on the home chain.
/**
* @notice Overall circulating supply of this token on all chains
* This is an optimistic figure for the behavior of the bridges and/or other chains
*
* @dev Considering a correct behavior of bridges and other chains, this figure should
* cover whole circulating supply. This is because other chains can bridge out to tertiary
* chains only a portion of the supply bridged to them from this contract.
*/
function totalXChainSupply() public view virtual returns (uint256) {
return totalSupply() + totalOtherChainsSupply();
}
which is used for further checks like this:
function _mintWithCaller(address _caller, address _user, uint256 _amount) internal virtual override {
uint256 _totalLockedERC20 = IXERC20HomeTokenLockbox(lockbox).totalLockedForXERC20();
// A bridge tries to mint more leading to increased local supply over what is locked
if ((_amount + totalSupply()) > _totalLockedERC20) revert IXERC20_NotEnoughInLockbox();
super._mintWithCaller(_caller, _user, _amount);
// A bridge minted xERC20, so it SHOULD decrease the supply on the other chain
_totalOtherChainsSupply = _totalOtherChainsSupply - _amount;
...
Overall, here is the proposal to adjust the MUST have interface:
interface IXERC20 {
/**
* @notice Reverts when a bridge with too low of a limit tries to call mint/burn
*/
error IXERC20_NotHighEnoughLimits();
/**
* @notice Returns the max limit of a minter
*
* @param _bridge The bridge we are viewing the limits of
* @return _limit The limit the bridge has
*/
function mintingMaxLimitOf(address _bridge) external view returns (uint256 _limit);
/**
* @notice Returns the max limit of a bridge
*
* @param _bridge the bridge we are viewing the limits of
* @return _limit The limit the bridge has
*/
function burningMaxLimitOf(address _bridge) external view returns (uint256 _limit);
/**
* @notice Returns the current limit of a minter
*
* @param _bridge The bridge we are viewing the limits of
* @return _limit The limit the minter has
*/
function mintingCurrentLimitOf(address _bridge) external view returns (uint256 _limit);
/**
* @notice Returns the current limit of a bridge
*
* @param _bridge the bridge we are viewing the limits of
* @return _limit The limit the bridge has
*/
function burningCurrentLimitOf(address _bridge) external view returns (uint256 _limit);
/**
* @notice Mints tokens for a user
* @dev Can only be called by a bridge
* @param _user The address of the user who needs tokens minted
* @param _amount The amount of tokens being minted
*/
function mint(address _user, uint256 _amount) external;
/**
* @notice Burns tokens for a user
* @dev Can only be called by a bridge
* @param _user The address of the user who needs tokens burned
* @param _amount The amount of tokens being burned
*/
function burn(address _user, uint256 _amount) external;
}
Hi authors of ERC-7281, this is Victor, an EIP editor and current operator of AllERCDevs.
I like to invite you to our next AllERCDevs meeting (online) to present for 10min of your ERCs if you are interested!
AllERCDevs is a bi-weekly meeting for ERC authors, builders and editors to meet and help the drafting and adoption of an ERC. The next one is 2024-04-30 UTC 2300, let us know if this time works for you, I can put this ERC in the agenda, or you can add a response directly at S2E4 AllERCDevs Agenda 2024-04-30 Tuesday UTC2300 (APEC friendly time) · Issue #22 · ercref/AllERCDevs · GitHub
1 Like
(co-author of ERC-5725)
First off, I really want to see a standard like this become adopted as it’s a very needed standard imho, so thank you for putting some thought into this.
I think the writeup of the proposal is great when it comes to the abstract, motivation, rationale and compatibility.
Interface Improvement Suggestion - Mint/Burn Limits
But, I think the interface needs more thought when it comes to the limits. Imho, it’s not clear how to properly set and manage the limits. There appears to be some incoherence in the current interface.
1. Clarify how the mint and burn limits work
The quote below says that BOTH mint and burn must “reduce the current available limit”
My questions would be…
- Reduce which limit? Then mint or burn? Imho, it needs to be more explicit.
- If these operations reduce the limit, then how does the limit increase again? I would assume that mint reduces the limit, and burn increases the “currentLimit”
Implementations MUST additionally satisfy the following requirements:
mint MUST check that the caller's current available limit is greater than or equal to _amount
mint MUST increase the supply of the underlying ERC-20 token by _amount and reduce the current available limit
burn MUST check that the caller's current available limit is greater than or equal to _amount
burn MUST decrease the supply of the underlying ERC-20 token by _amount and reduce the current available limit
2. Bridge structs incoherence
In BridgeParameters, timestamp, and ratePerSecond are not mentioned in the specification for how to handle and use.
struct Bridge {
BridgeParameters minterParams;
BridgeParameters burnerParams;
}
struct BridgeParameters {
uint256 timestamp;
uint256 ratePerSecond;
uint256 maxLimit;
uint256 currentLimit;
}
3. Setting Limits Clarity
When it comes to role based access functions, I would agree with @SamWilsn here as this probably doesn’t assist with interoperability.
I would considering adding an example interface IXERC20Acess.sol which includes setLimits and setLockBox.
BUT… regardless, I believe this function needs some more clarification. It talks about setting the _mintingLimit and _burningLimit, but is this the currentLimit or the maxLimit?
How does this function affect the other variables in BridgeParameters like ratePerSecond?
/**
* @notice Updates the limits of any bridge
* @dev Can only be called by the owner
* @param _mintingLimit The updated minting limit we are setting to the bridge
* @param _burningLimit The updated burning limit we are setting to the bridge
* @param _bridge The address of the bridge we are setting the limits too
*/
function setLimits(address _bridge, uint256 _mintingLimit, uint256 _burningLimit) external;
Ultimately, I feel like there needs to be more clarification on how to set and manage limits. Things need to be sorted out between these definitions: mintingMaxLimit, mintingCurrentLimit, burningMaxLimit, burningCurrentLimit, and how each of those are affected on mint and burn operations as well as access based adjustments.
Additions
- I’m not 100% up to date on bridge implementations, but I would imagine that a bridge implementation would benefit with a reference to a
chainId, as knowing which chain the tokens are being burned for, could provide valuable risk protection.
@DeFiFoFum IMHO I do not think the standard should describe the management of limits at all - or as you mentioned in the form of an example.
E.g. our Bridge struct is different:
struct Bridge {
//
// SLOT 00a - non configurable
//
/// @notice The last snapshot of the mint/burn capacity indicator value
uint128 snapshotCapacityIndicator;
/// @notice The last snapshot time of the mint/burn capacity indicator
uint40 snapshotTime;
//
// SLOT 00b - configurable
//
/// @notice Total duration it takes for the allowance rebalance after full mint action
/// Applied when below the capacityBalanceTarget.
/// Typically 1 day = 86400 sec, max ~ 194 days
uint24 mintRecoveryDuration;
/// @notice Total duration it takes for the allowance rebalance after full burn action
/// Typically 1 day = 86400 sec, max ~ 194 days
/// Applied when above the capacityBalanceTarget.
uint24 burnRecoveryDuration;
//
// SLOT 01 - configurable
//
/// @notice Configurable total capacity of the mint/burn capacity pool.
/// A hard cap that can be minted / burned.
uint128 totalCapacity;
/// @notice Configurable attractor for the capacity indicator to gravitate to.
/// Typically to be totalCapacity / 2, but can be set to favor either minting or burning
uint128 capacityBalanceTarget;
}
So we have a different recovery speeds for minting and for burning and also DOS protection.
In any case the bridge shall be aware of its limits as defined by the standard. This I believe is the key point of this standard.
IMO it is up to the implementers on how the internal mechanisms for those limits are implemented.
Things need to be sorted out between these definitions:
mintingMaxLimit,mintingCurrentLimit,burningMaxLimit,burningCurrentLimit, and how each of those are affected onmintandburnoperations as well as access based adjustments.
I understand your confusion originates from the wording used in the setLimits(), which could be implemented in many ways anyway.
Thanks for the input @radek
I’m starting to come around to the standard, and I see your point about the BridgeParameters being configurable based on implementation.
With that being said, if the BridgeParameters are changeable, then they should be removed from the IXER20 Specification and included in a sub section showing an example of how one might implement the bridge limit parameters.
That part really threw me off when I was reading through the EIP and I didn’t see any reference material in the EIP describing that limits are generally managed over a set time period.
Based on the implementation I now understand what these values represent, but as a person reading the spec with fresh eyes, I had to look around at implementation code and other examples to fully grasp what they represent.
Things need to be sorted out between these definitions: mintingMaxLimit , mintingCurrentLimit , burningMaxLimit , burningCurrentLimit , and how each of those are affected on mint and burn operations as well as access based adjustments.