MVP swaps support for SLP tokens and PIRATE. #1067

artemii235 · 2021-09-15T11:14:26Z

# Conflicts: # mm2src/coins/lp_coins.rs # mm2src/coins/utxo/utxo_common.rs # mm2src/lp_swap/maker_swap.rs # mm2src/mm2_bitcoin/keys/src/lib.rs # mm2src/mm2_tests.rs

…work rules" on RICK side.

…MM2 entirely.

# Conflicts: # mm2src/mm2_tests.rs

…ove to docker_tests.

…send_htlc.

…send_outputs.

…validation.

Did that to avoid locking the mutex on each balance call. #1034

… HTLC on PIRATE.

# Conflicts: # Cargo.lock # Cargo.toml # mm2src/coins/lp_coins.rs # mm2src/coins/qrc20.rs # mm2src/coins/utxo.rs # mm2src/coins/utxo/qtum.rs # mm2src/coins/utxo/slp.rs # mm2src/coins/utxo/utxo_common.rs # mm2src/coins/utxo/utxo_standard.rs # mm2src/coins/utxo/utxo_tests.rs # mm2src/docker_tests.rs # mm2src/docker_tests/qrc20_tests.rs # mm2src/lp_ordermatch/best_orders.rs # mm2src/mm2_tests.rs

# Conflicts: # mm2src/coins/utxo/slp.rs # mm2src/coins/utxo/utxo_tests.rs # mm2src/lp_ordermatch/best_orders.rs

Milerius · 2021-09-15T11:22:43Z

mm2src/coins/eth.rs

@@ -745,6 +745,7 @@ impl SwapOps for EthCoin {
        fee_addr: &[u8],
        amount: &BigDecimal,
        min_block_number: u64,
+        _uuid: &[u8],


I assume will be used for later usage? Or it's simply to respect the interface for send_taker_fee?

As of now, it's simply to respect the interface, yes. But it will be useful in the future when we implement a more advanced swap protocol.

Milerius · 2021-09-15T11:25:47Z

mm2src/coins/lp_coins.rs

-        CoinProtocol::ZHTLC => try_s!(z_coin_from_conf_and_request(ctx, ticker, &coins_en, req, secret).await).into(),
+        CoinProtocol::ZHTLC => {
+            let dbdir = ctx.dbdir();
+            try_s!(z_coin_from_conf_and_request(ctx, ticker, &coins_en, req, secret, dbdir).await).into()


I think here the ? can be used if the trait is implemented.

We still have some old code depending on try_s! - I will try to get rid of it wherever possible, thanks!

# Conflicts: # etomic_build/client/enable_tBCH # mm2src/docker_tests.rs # mm2src/mm2_bitcoin/keys/src/cashaddress.rs

shamardy

LGTM! 🔥

sergeyboyko0791

Hi @artemii235, there are a few questions and suggestions. Please take a look at them

mm2src/coins/lp_coins.rs

mm2src/coins/utxo/bch.rs

mm2src/coins/utxo/slp.rs

mm2src/coins/utxo/utxo_common.rs

mm2src/coins/z_coin.rs

sergeyboyko0791 · 2021-09-28T10:15:42Z

mm2src/coins/z_coin.rs

+        self.rpc_client().send_raw_transaction(tx_bytes.into()).compat().await?;
+
+        self.rpc_client()
+            .wait_for_confirmations(


Why should we wait for confirmations every time we send a transaction?
As far as I understand, this method is only used by swap functions like z_send_tx_fee and z_send_htlc. But MakerSwap/TakerSwap is already waiting for confirmations if it's required.

Due to Sapling specifics, the shielded output becomes spendable only after it's confirmed. So we have to wait for tx to be mined to allow construction of a new one. This is a bit tricky now because it's better to not wait for that long here, but we have to wait somewhere for output to be confirmed and used in case of concurrent swaps.

PS I've realized that z_unspent_mutex should be locked during this process.

I'd suggest adding the sent transaction to a container inside BCH coin. So when any other SLP token tries to generate a new transaction, it will wait until every transaction from this container is mined.

Excuse me, did you intend to post this comment to another place? ZCoin has little relation to BCH and SLP.

mm2src/coins/z_coin.rs

mm2src/coins/z_coin/z_htlc.rs

sergeyboyko0791 · 2021-09-28T11:18:09Z

mm2src/lp_swap/maker_swap.rs

@@ -459,7 +459,7 @@ impl MakerSwap {
    }

    async fn wait_taker_fee(&self) -> Result<(Option<MakerSwapCommand>, Vec<MakerSwapEvent>), String> {
-        const TAKER_FEE_RECV_TIMEOUT: u64 = 180;
+        const TAKER_FEE_RECV_TIMEOUT: u64 = 600;


I guess, you increased the timeout because TakerSwap doesn't send the fee transaction hex until send_outputs waits for confirmations. What if we just don't wait for confirmations within send_outputs?

Yes, I provided more info on this in #1067 (comment). I should also add that I don't like it for now, but I keep thinking about how to make it better. Shielded transactions support and UX is full of tradeoffs in general.

sergeyboyko0791

There are no blockers since it's an MVP. Thanks!

artemii235 added 30 commits June 24, 2021 20:18

WIP. Made full order matching and swap test with RICK and FUSD.

e80440d

WIP. Implementing traits for SlpToken.

7f3a6fd

WIP. Implemented SLP withdraw. Added BchCoin.

4eab690

WIP. Avoiding usage of SLP UTXOs for normal BCH. Some tests fail.

d56561e

WIP. Added UtxoTxBuilder. All tests pass.

146957a

Merge remote-tracking branch 'origin/dev' into mm2.1-slp-swap

31a86ef

# Conflicts: # mm2src/coins/lp_coins.rs # mm2src/coins/utxo/utxo_common.rs # mm2src/lp_swap/maker_swap.rs # mm2src/mm2_bitcoin/keys/src/lib.rs # mm2src/mm2_tests.rs

Fixes after merge.

559e43c

WIP. Refactoring tx builder. Code doesn't compile.

525e133

WIP. RICK/FUSD trade failed with the "transaction was rejected by net…

19d070a

…work rules" on RICK side.

Merge remote-tracking branch 'origin/dev' into mm2.1-slp-swap

6fb2e95

WIP. Refactored BCH unspents retrieval.

73ca5f3

Fix Clippy and WASM.

a814811

WIP. Added tx_size arg to get_htlc_spend_fee.

64f41c1

WIP.Properly handle ActualTxFee::FixedPerKb case in get_htlc_spend_fee.

78328fb

WIP. Remove bitcoin-cash-slp dependency. Build SLP sending script in …

d95bf80

…MM2 entirely.

WIP. Add more checks in parse_slp_script.

1b296ac

Merge remote-tracking branch 'origin/dev' into mm2.1-slp-swap

85448b8

# Conflicts: # mm2src/mm2_tests.rs

WIP. Remove RICK/FUSD from trade_test_electrum_and_eth_coins - will m…

99eabf8

…ove to docker_tests.

WIP. Moving SLP trading tests to docker_tests.

3e1fea6

WIP. Added docker_tests_common and few SLP-related tests.

693cf04

Fix zhtlc feature compilation. Check that RICK/ZOMBIE trade test pass.

294e9e1

WIP. Researching tx building with Sapling spends.

2eb1bcc

WIP. Dummy add_sapling_spend that doesn't work yet.

3c79ee3

WIP. Sent the shielded spend transaction using pure Rust code.

03cecc1

Sent and refunded dex fee on ZOMBIE chain.

29d4896

WIP. Added send_outputs and started using it in z_send_dex_fee and z_…

015e3d3

…send_htlc.

Add SendOutputsErr::InsufficientBalance and return it accordingly in …

c39e439

…send_outputs.

WIP. Making dex fee a shielded output with the public OVK.

4e18aae

WIP. Made dex fee a shielded output with the public OVK. Implemented …

e76462c

…validation.

WIP. Try using cached commitment tree.

d34d04c

artemii235 added 16 commits August 9, 2021 19:27

WIP. ZCoin withdraw.

4a907e0

Add separate methods to get BCH/SLP unspents for balance display.

f2fa882

Did that to avoid locking the mutex on each balance call. #1034

Extract recently_spent from SlpTxPreimage to avoid occasional deadlocks.

2aae2b6

WIP. Implemented withdraw for ZCoin.

87da267

WIP. Building Sapling cache DB for ARRR.

22eeb00

Restore sapling_state_cache_loop. Add ARRRConsensusParams. Sent first…

0bcf72e

… HTLC on PIRATE.

Use ARRRConsensusParams to build tx in z_p2sh_spend.

8bb893d

Increase TAKER_FEE_RECV_TIMEOUT to 600.

4cd6ffc

WIP. Fixing after merging from dev.

ef3d05b

Fixes after merge.

99f8f6a

Fixes after merge.

e45795a

Fix WASM.

7da678f

Merge remote-tracking branch 'origin/dev' into mm2.1-slp-swap

2f9c175

# Conflicts: # mm2src/coins/utxo/slp.rs # mm2src/coins/utxo/utxo_tests.rs # mm2src/lp_ordermatch/best_orders.rs

Fix after merge.

4896ef0

One more fix.

e206da6

artemii235 requested review from Milerius and shamardy September 15, 2021 11:14

Milerius reviewed Sep 15, 2021

View reviewed changes

artemii235 requested a review from sergeyboyko0791 September 27, 2021 10:19

artemii235 added 3 commits September 27, 2021 18:13

Merge remote-tracking branch 'origin/dev' into mm2.1-slp-swap

db516b0

# Conflicts: # etomic_build/client/enable_tBCH # mm2src/docker_tests.rs # mm2src/mm2_bitcoin/keys/src/cashaddress.rs

Fixes after merge.

7efd2c8

Merge remote-tracking branch 'origin/dev' into mm2.1-slp-swap

f78069d

shamardy previously approved these changes Sep 27, 2021

View reviewed changes

sergeyboyko0791 suggested changes Sep 28, 2021

View reviewed changes

Review fixes.

4d79dc2

artemii235 dismissed shamardy’s stale review via 4d79dc2 September 29, 2021 08:28

sergeyboyko0791 approved these changes Sep 29, 2021

View reviewed changes

artemii235 merged commit 0b5809b into dev Sep 29, 2021

artemii235 deleted the mm2.1-slp-swap branch September 29, 2021 10:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MVP swaps support for SLP tokens and PIRATE. #1067

MVP swaps support for SLP tokens and PIRATE. #1067

artemii235 commented Sep 15, 2021

Milerius Sep 15, 2021

artemii235 Sep 15, 2021

Milerius Sep 15, 2021

artemii235 Sep 15, 2021

shamardy left a comment

sergeyboyko0791 left a comment

sergeyboyko0791 Sep 28, 2021

artemii235 Sep 28, 2021 •

edited

Loading

sergeyboyko0791 Sep 29, 2021

artemii235 Sep 29, 2021

sergeyboyko0791 Sep 28, 2021

artemii235 Sep 28, 2021

sergeyboyko0791 left a comment

MVP swaps support for SLP tokens and PIRATE. #1067

MVP swaps support for SLP tokens and PIRATE. #1067

Conversation

artemii235 commented Sep 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamardy left a comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artemii235 Sep 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

artemii235 Sep 28, 2021 •

edited

Loading