-
Notifications
You must be signed in to change notification settings - Fork 94
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
SLP and ARRR integration WIP. #954
Conversation
# Conflicts: # mm2src/coins/Cargo.toml # mm2src/coins/lp_coins.rs # mm2src/coins/z_coin.rs # mm2src/coins/z_coin/z_coin_tests.rs # mm2src/coins/z_coin/z_htlc.rs # mm2src/coins/z_coin/z_rpc.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved! 🚀
script_data: Script, | ||
) -> Result<UtxoTx, MmError<ZP2SHSpendError>> { | ||
let current_block = coin.utxo_arc.rpc_client.get_block_count().compat().await? as u32; | ||
let mut tx_builder = ZTxBuilder::new(consensus::MAIN_NETWORK, current_block.into()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one question, do we need to support TEST_NETWORK
for zcoin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for now, it's fine to stick with mainnet only and I plan to make the implementation more generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the great work! I've left a few reminders. I guess there are no blockers now.
} | ||
|
||
impl SwapOps for ZCoin { | ||
fn send_taker_fee(&self, _fee_addr: &[u8], _amount: BigDecimal) -> TransactionFut { todo!() } | ||
fn send_taker_fee(&self, _fee_addr: &[u8], amount: BigDecimal) -> TransactionFut { | ||
// TODO replace dummy locktime and watcher pub |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you do this TODO in the next iteration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, dexfee-related methods have to be changed significantly for ZCoin - will do it later.
|
||
let outpoint = ZCashOutpoint::new(p2sh_tx.hash().into(), 0); | ||
let tx_out = TxOut { | ||
value: Amount::from_u64(p2sh_tx.outputs[0].value).expect("p2sh_tx transaction always contains valid amount"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have to check the count of the transaction outputs here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, we can occasionally get the transaction not having transparent inputs and outputs at all. Will do it in the next iterations.
@@ -67,6 +67,8 @@ async fn enable_coins_eth_electrum( | |||
); | |||
replies.insert("ETH", enable_native(mm, "ETH", eth_urls).await); | |||
replies.insert("JST", enable_native(mm, "JST", eth_urls).await); | |||
#[cfg(feature = "zhtlc")] | |||
replies.insert("ZOMBIE", enable_native(mm, "ZOMBIE", eth_urls).await); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it correct to specify the same urls as we do for the ETH coin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ETH URLs are simply ignored for native UTXO mode so it's fine 🙂
#927 #701