Enable 2.0 skeleton #1099
Enable 2.0 skeleton #1099
Conversation
| pub enum EnableProtocolParams { | ||
| Bch(BchActivationParams), | ||
| SlpToken, | ||
| Eth { | ||
| urls: Vec<String>, | ||
| swap_contract_address: Address, | ||
| fallback_swap_contract: Option<Address>, | ||
| with_tokens: Vec<String>, | ||
| }, | ||
| Erc20, | ||
| Utxo(UtxoActivationParams), | ||
| Qrc20(Qrc20ActivationParams), | ||
| Zcoin { | ||
| mode: ZcoinActivationMode, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Here I plan an alias parameter for the tokens (to allow a token to be just an alias towards the parent's client). I don't know if it's planned with the with_tokens, but I imagine it will be possible to share the same client across all the tokens.
I also imagine something like that in coins:
{
"coin": "SOL",
"name": "solana",
"protocol": {
"type": "SOL",
"tokens": {
"USDC": {
"decimals": 6,
"address": "CpMah17kQEL2wqyMKt3mZBdTnZbkbfx4nqmQMFDP5vwp"
}
}
},
"rpcport": 80,
"mm2": 1
}In order to facilitate the token additions and reduce the duplication of entry in the coins file. Maybe out of scope of this PR, but I just try to keep that in mind.
I imagine that when parsing the coins file we can make lp_coinfind find entries like those one
There was a problem hiding this comment.
but I imagine it will be possible to share the same client across all the tokens.
Yes, I've already done it this way for SLP tokens.
{
"coin": "SOL",
"name": "solana",
"protocol": {
"type": "SOL",
"tokens": {
"USDC": {
"decimals": 6,
"address": "CpMah17kQEL2wqyMKt3mZBdTnZbkbfx4nqmQMFDP5vwp"
}
}
},
"rpcport": 80,
"mm2": 1
}Such config format seems problematic to me: some RPCs (orderbook, best_orders, and maybe others) can work without corresponding coin activation. If we add tokens to the coins, we will have to iterate over all records and also through all the tokens when we need to find a specific config. Having a separate config record for tokens might look excessive a bit, but it has more pros than cons IMHO.
| let req: EnableRpcRequest = json::from_value(req).map_to_mm(EnableError::RequestDeserializationErr)?; | ||
|
|
||
| if let Ok(Some(_)) = lp_coinfind(&ctx, &req.coin).await { | ||
| return MmError::err(EnableError::CoinIsAlreadyActivated(req.coin)); |
There was a problem hiding this comment.
This is just a question for future implementation. What if a coin is already activated and we want to batch activate more tokens related to it. Do you think it's worth adding another method in the future to batch activate tokens for an already activated protocol coin or will the enable_v2 be used for this, not returning an error here but activating the non-activated tokens? Is this even a scenario we can run into?
There was a problem hiding this comment.
MM2 RPC supports batch requests so API users can construct a batch with multiple enable_v2 calls whenever required. It should cover the case with multiple tokens activation when platform coin is already enabled.
| }, | ||
| CoinProtocol::QTUM => try_s!(qtum_coin_from_conf_and_request(ctx, ticker, &coins_en, req, secret).await).into(), | ||
| CoinProtocol::ETH | CoinProtocol::ERC20 { .. } => { | ||
| try_s!(eth_coin_from_conf_and_request(ctx, ticker, &coins_en, req, secret, protocol).await).into() |
There was a problem hiding this comment.
Why is eth_coin_from_conf_and_params is not implemented now? is there a special case for Eth that prevents adding EthActivationParams in this PR? It can be added in the future of course :)
There was a problem hiding this comment.
I wanted to implement UTXO coins first as I'm doing this in the context of SLP support. I will refactor ETH coin activation too on the next iteration 🙂
| let null = Json::Null; | ||
| let conf_builder = UtxoConfBuilder::new(conf, &null, coin); | ||
| // using a reasonable default here | ||
| let params = UtxoActivationParams { |
There was a problem hiding this comment.
I don't know if Default should be implemented for UtxoActivationParams and used here. It's not a big deal as default params are only used in this function.
There was a problem hiding this comment.
Yes, as this is the only use case, I think we can keep it like this.
mm2src/coins/enable_v2.rs
Outdated
| } | ||
|
|
||
| #[allow(unused_variables)] | ||
| pub async fn enable_v2(ctx: MmArc, req: Json) -> Result<EnableResult, MmError<EnableError>> { |
There was a problem hiding this comment.
As I see, enable_v2 is not used yet, so you can ignore the following comment (at least now :D).
dispatcher_legacy and dispatcher_v2 can deserialize the request and pass it as an argument.
For example, withdraw takes WithdrawRequest.
There was a problem hiding this comment.
Thanks, I have not noticed it 🙂 I will check withdraw and refactor enable_v2.
|
@sergeyboyko0791 I've pushed the update, could you recheck the PR, please? |
#1006
I prefer to open this PR as soon as possible to open a possibility for @Milerius to implement enable 2.0 for Solana. This may also help to avoid many git conflicts for the ongoing Lightning integration @shamardy.