This is basically a technical design. I don’t mind anyone implementing it
Thanks for writing this up!
A couple points:
-
Your example is only about one type of DEX: the order book DEX. From my understanding, order book DEXs do not take custody of users’ funds while their orders are being matched (e.g. Dexalot). Doing so would be a big step backwards. In your example, the DEX canister takes custody. What if a user’s order never gets fulfilled? When will the DEX refund the user’s deposit?
-
IMO having a separate deposit and withdraw step for using a DEX is poor UX, since this would be on top of the deposit and withdraw step that’s needed for the underlying wallet.
-
Let’s take the more common DEX model: the Uniswap-style AMM. (That was what my previous code snippet was trying to model.) In this case, a user can instantly swap one token for another from a liquidity pool, all in a single atomic transaction. If any part of that fails (e.g. insufficient liquidity), then the entire transaction should fail and the user should have lost nothing. That is what seems difficult to implement on the IC. Splitting that one logical transaction into multiple doesn’t seem to have the security (or UX) requirements that DeFi users are used to and will tolerate.
-
Syncing balances between the token canister and the DEX canister isn’t great. What if the DEX canister is out of cycles or, worse, offline when the token canister tries to call it? How do you re-sync?
Thanks for sharing.
Here are my initial thoughts:
-
The standard is quite big… I feel like a smaller standard would be more user-friendly, easier to reason about, and less prone to bugs.
-
I’m not sure why you kept
approve
andtransferFrom
if the token canister cannotify
recipients. In my mind, you either need one or the other. Not really quite sure what this explanation means:
Approve is often accompanied by the possibility of simultaneous operation of two tokens. Approve can avoid the repeated payment problem which transaction brought about, has a good supplementary use scenario for transfer.
- I took a look at the Motoko implementation, but can’t find which part of that code actually implements sagas. Could you link me?
Due to Raw Calls - What is necessary? · Issue #2703 · dfinity/motoko · GitHub, Motoko’s implementation is incomplete, and approveAndCall & transferAndCall cannot implement call as well as rust.
Yes, it is bigger than ERC20. I have the same idea as you:it would be better if it could be smaller
,but after careful consideration, we still feel that adding these interfaces is necessary.
Let’s compare it to ERC20
ERC20
service: {
name: () -> (text) query;
symbol: () -> (text) query;
decimals: () -> (nat8) query;
totalSupply: () -> (nat) query;
balanceOf: (owner: principal) -> (nat) query;
allowance: (owner: principal, spender: principal) -> (nat) query;
approve: (spender: principal, value: nat) -> (bool);
transferFrom: (sender: principal, receiver: principal, value: nat) -> (bool);
transfer: (receiver: principal, value: nat, args:opt vec nat8) -> (bool);
}
These are the interfaces added to the DFT standard
The reason why these interfaces are designed is in the interface notes
service : {
// Fee is necessary because the reverse Gas model prevents DDos
fee : () -> (Fee) query;
setFee : (Fee) -> (bool);
setFeeTo : (feeHolder: text) -> (bool);
//Readonly interface(Will not cause token security issues)
// return the metadata of a token (fee setting,decimals,name,totalSupply,symbol)
meta: () -> (MetaData) query;
// Extend information about token, I explain it at [Information self-describing] at https://dft.delandlabs.com/#what-other-functions-does-the-dfinity-fungible-token-standard-need-to-implement
// It just get/set token's socail media information, will not cause token security issues
extend: () -> (vec KeyValuePair) query;
setExtend : (vec KeyValuePair) -> (bool);
//Same as the extend information
logo : () -> (vec nat8) query;
setLogo : (logo : vec nat8) -> (bool);
// Owner is necessary, the only operation who can update token's information
// This is only used to control the permission to update the token information (to call setFee, setLogo, setExtend)
owner : () -> (principal);
setOwner : (owner: principal) -> (bool);
//Readonly interface(Will not cause token security issues)
tokenInfo : () -> (TokenInfo) query;
//Readonly interface(Will not cause token security issues)
allowancesOf : (holder: text) -> (vec record { TokenHolder; nat }) query;
//Readonly interface(Will not cause token security issues)
lastTransactions : (size: nat64) -> (TxRecordsResult) query;
//Readonly interface(Will not cause token security issues)
transactionById : (transactionId: text) -> (TxRecordResult) query;
//Readonly interface(Will not cause token security issues)
transactionByIndex : (nat) -> (TxRecordResult) query;
}
Feel free to discuss it , any your comments will help us to improve it
- The standard is quite big… I feel like a smaller standard would be more user-friendly, easier to reason about, and less prone to bugs.
notify
means to let the receiver be notified, which triggers the receiver to execute its own logic. The important role of notify
is to replace ERC20 event emit
Approve
means authorization. In the DEX scenario, the authorization of two different tokens is required. When the next step of transfer notify cannot be executed internally, but the transfer has been successful, it is necessary to consider how to process the refund, but approve does not have these troubles
The two are used to meet different scenarios, that is why we chose to retain them.
- I’m not sure why you kept
approve
andtransferFrom
if the token canister cannotify
recipients. In my mind, you either need one or the other. Not really quite sure what this explanation means:
Hmm, my understanding is that notify
is the name of the callback for transferAndCall
, and that transferAndCall
is a replacement for approve
and transferFrom
.
approve
and transferFrom
: token canister gets called by DEX (or other) canister
transferAndCall
: token canister calls DEX (or other) canister
Regardless, the main issue with notify
is still cross-canister atomicity. If notify
fails, the balance update made in the token canister needs to be rolled back in the same atomic transaction. I don’t think we should expect or rely on the DEX canister to refund the user.
I also wanted to flag that token standard implementations should use certified variables for query functions (e.g. what’s my balance? what transactions do I have?) for maximum security.
This is what the ICP Ledger Canister is doing, as I discovered here.
I think rather than transferAndCall a better name could be TransferAndNotify - you want to transfer monies and have the canister be aware that monies have a secondary canister be aware that monies have been transferred.
This is still an inter canister call, so care should be taken to ensure that this operation would succeed atomically.
This is actually a similar pattern to what we are using at InfinitySwap https://app.infinityswap.one/.
How can we do this?
→ represents a transmute operation - either a transfer from or a transfer and call. Care must be taken to ensure atomicity of this operation as it requires two canister calls.
Mint
t1 -> t1'
t2 -> t2'
-------------------
//synchronous
t2'.transfer()
t1'.transfer()
** mint(address to)
Burn
//synchronous
** burn(address to)
-------------------
t1' -> t1
t2' -> t2
Swap
t1 -> t1'
---------------------
//synchronous
t1'. approve()
swap(uint amount0Out, uint amount1Out, address to, bytes calldata data)
---------------------
t2' -> t2
May actually be better to separate TransferAndNotify as is done with the IC’s transfer and then notify because this would be a non-atomic operation and not make assumptions about whether a cross-canister call has succeeded.
Yea, that makes sense to me.
I’m not sure the ICP ledger canister model is developer-friendly or safe.
Having two separate methods transfer
and notify
just pushes the burden of cross-canister atomicity to the client. (Actually, notify
is being deprecated, and the sender will need to call the recipient directly.)
For example, let’s say a user wants to convert ICP to some other token using some DEX canister running on the IC.
- The user calls
transfer
on the ICP ledger canister to send ICP to a DEX canister. - The user calls some custom function on the DEX canister to notify it that some ICP was sent to it.
- The DEX canister then does some stuff with the ICP (i.e. swapping it for another token), which will require at least two inter-canister calls.
What if the DEX canister is out of cycles, or even worse, malicious? Then, after step 1 nothing else would happen, and the user just lost their ICP. Basically, we’re back to the Dark Ages of DeFi when only ERC-20 existed. This is the problem that ERC-223 and ERC-677 tried to solve.
I feel like a good token standard should take care of cross-canister atomicity, and clients shouldn’t have to care. Trusting clients to implement two-phase commits and saga transactions correctly is a recipe for disaster IMO. I would rather have the community implement that here in this token standard and save future developers/users a lot of pain.
I think that would work if transfer also reverts the transfer if the notify fails. Yep, that makes sense.
I agree. I’m starting an initiative to create a ledger abstraction that will work across ICP, BTC, ETH, and any SNS tokens. The design goals are atomicity and to make it trivial for canisters to transparently accept and make payments with an API that is more like the Stripe payment API than a ledger
Basically trying to be the best fungible token standard all things considered?
For all involved in creating a token standard, I encourage you to consider adding some kind of “transferSafe” function as described in this tweet thread: https://twitter.com/lastmjs/status/1460379362142863364
The basic idea is to allow each party in a transfer to verify and confirm the intent to send and receive a certain amount of cryptocurrency, hopefully drastically reducing the risk of sending to the wrong address.
If we can start off with this kind of functionality in our Internet Computer tokens, perhaps we can reduce a lot of lost funds.
Transferring large amounts of crypto with just a copy-paste of addresses is terrifying.
Absolutely agree with you on that! There are a lot of details to hash out for how we will design it, but safe transfers, plus simple ways to check your token balances and transaction history, are my top priorities in this design
Random thought - this could also leverage the ICs private state.
When staging a transaction the sender includes a “key”. The recipient is required to input that key when accepting the transaction, else the transaction rolls back.
Maybe this is a dumb question, but can’t recipients just verify all incoming transactions? After all, if things go wrong, they benefit from some extra tokens.
Also, I’d imagine that safe transfers would only be useful if a) the sender is a human (and not a smart contract), and b) whatever UI the sender is using requires them to type in an opaque address.
I think b) will slowly be solved with better UXs like ENS.
Correct, I don’t think safe transfers are always required, that’s why it should be in addition to the more bare-bones transfers.
Recipients could just confirm everything, though I kind of feel it would be rare to mistype an actual address with a recipient just waiting to approve. So, some kind of shared code might be necessary to mitigate that…could be optional.
I don’t think ENS fully solves it, since there are very similar ENS names and you could still misstype them.
I don’t think it’s necessary to add safeTransfer()
(1) This method can only avoid incorrectly transferring to a container that cannot handle the token. There are many situations where incorrect transfers occur, and avoiding errors relies mainly on the sender doing correctly.
(2) Wallets and dapp containers have multiple more ways to avoid errors, either by UI experience improvement or by the receiver canister providing a function.
https://github.com/iclighthouse/DRC_standards/issues/1
This is the Token standard drafted by our team, and an example of the implementation.
Improvements
- Compatible with Principal and Account-id as Address
- Using the pub/sub model for message notifications
- Improving transaction atomicity with a lock/execute two-phase commit structure
- Scalability of transaction records storage, temporary storage in token canister and permanent storage in external canisters