Segwit support. WASM P2P. Bug fixes. #1004

artemii235 · 2021-07-07T09:00:30Z

More details TBA.

* Add 'kmd_rewards' to UtxoCoin::tx_details_by_hash * TODO fill the 'kmd_rewards' filed in UtxoCoin::withdraw * Add 'kmd_rewards' to the UtxoCoin::withdraw result * Add the KMD rewards re-calculating on tx history * Add 'get_mut_verbose_transaction_from_map_or_rpc', 'calc_interest_of_tx' to 'UtxoCommonOps' * Add 'update_kmd_rewards' to 'UtxoStandardOps' * Return error on calc_interest_of_tx for coins that don't support transaction rewards * Pass 'HistoryUtxoTxMap' to 'tx_details_by_hash' * Replace Result<(), String> with UtxoRpcResult<()> * Fill and fix comments * Fix the broken 'test_process_cached_tx_transfer_map_updated' test * Fix rustfmt warnings * Add the 'claimed_by_me' field to 'kmd_rewards' details * Remove the confusing comment

# Conflicts: # mm2src/coins/utxo/utxo_tests.rs

* add option to disable order history * move save in history to orders structs * fix failing tests * fix makerorder deserialization

…r-history hotfix for disable order history #968

* WIP. Support multiple amounts in SEND. Start adding docker tests. * WIP. Added methods to generate SLP send transaction. * WIP. Sent and spent HTLC from MM2. * WIP. Sent and refunded HTLC from MM2. Preparing for swap demo. * WIP. Implementing SwapOps for SlpToken. * WIP. Implementing traits for SlpToken. * WIP. Implementing traits for SlpToken. * Fix tests compilation. * WIP. Implemented dex fee validation for SlpToken. * Fix clippy. * Review fixes and errors refactoring. * Use utxo_common::DEFAULT_SWAP_VOUT in ZCoin too.

* WIP. Trying to recreate the problem - not successful. * Bug seems to be fixed.

* Some helpers changes for notary helpers. * Explicitly require thread-pool feature for futures. * Re-export serde and serde-json from common. * Also re-export serde-derive. * Fix re-exports. * Add UtxoCoinFields::transaction_preimage.

#983

* display cancellation reason in order_status * fix wasm build issue * review fix

…973 (#992) * Cancel the kickstarted order immediately if balance is not sufficient. #973 * Delete the order from DB on kickstart balance check cancellation.

* Segwit Withdraw Implementation * fix build issue in test * change seed in tests for an address with balance * first review fixes * segwit spending swaps * fix rustfmt warning * fix failing tests * review fixes and some refactoring * address_from_str refactor * review fixes * more review fixes * fix typo mistake * use coins address_from_str implementation

* Start gossipsub in WASM * Fix p2p wasm compile errors * Establish connection between native and wasm nodes * Update rustup toolchain from nightly-2020-10-25 to nightly-2021-02-17 * Update libp2p to 0.38.0 * Update tokio from 0.2 to 1.7 * Update hyper from 0.13 to 0.14 * Fix clippy warnings * Replace NotifyHandler::Any using with NotifyHandler::One * Add Gossipsub::notify_primary and other methods * Add rust-toolchain.toml * Remove the 'common' crate from dependencies of mm2_libp2p, gossipsub * Remove the specified wasm32-support branch of libp2p * Fix gossipsub::tests * notify any connection if there is no peer connections in Gossipsub::peer_connections * Fix clippy warnings * Fix panic on gossipsub cache timeout * Fix parse_relay_address: add the '/ws' postfix

artemii235 · 2021-07-07T11:15:11Z

@tonymorony @cipig Please test the following:

Smoke test in GUIs. All basic operations should work properly.
Libp2p was updated - please check the compatibility between old and updated nodes in different combinations (new seeds - old GUI, old seeds - new GUI).
KMD rewards should be properly displayed in the tx_history.
max_taker_vol should return the coin ticker.
The Segwit address should be properly displayed in the orderbook if the order creator has this format activated.
For UTXO coins withdrawal to legacy addresses should always work. Withdrawal to P2SH and Bech32 addresses should not work if the coin doesn't have the segwit:true in the config. Please ensure that the resulting transaction sends funds to the selected address and the change is sent to the node's address by validating it in the explorer.
Please test Segwit swaps or post information about swaps that have been made already.

@Milerius @yurii-khi

To activate the Segwit please use the most recent coins file and new address_format arg for the enable/electrum RPC, for example:

{
  "userpass":"pass",
  "method":"electrum",
  "coin":"tBTC",
  "servers":[
    {
      "url":"electrum1.cipig.net:10068"
    },
    {
      "url":"electrum2.cipig.net:10068"
    },
    {
      "url":"electrum3.cipig.net:10068"
    }
  ],
  "address_format":{
    "format":"segwit"
  }
}

The default format is always legacy now. During the implementation, we assumed that the address format will be picked by users upon coin activation. If users want to switch to another format they can disable the coin and activate it back using a different option. If there is any problem with such a use case in existing GUIs UX please let me know, we can figure out possible workarounds.

PS. Old nodes are not able to trade with new nodes using the Segwit address. We are figuring out how to deny swaps between them in MM2.

* fix segwit display balance for native client * fix formating * add bech32_hrp check for segwit address format

cipig · 2021-07-07T13:33:40Z

KMD/LTC swap 725533a5-26bd-40f7-aa7a-a9c5758f0561 successfull, taker was non-segwit and maker was segwit
LTC makerpayment (segwit): https://blockchair.com/litecoin/transaction/74187173f35f5b994e3ae3b91ea1e40ad72b8f79d72585fab12a2dfa911f3264
LTC makerpaymentspent (not segwit): https://blockchair.com/litecoin/transaction/c68e3dff7de19a0ee7b13b57eb51c6d56ed6385e8c619e66b6e5ea699336a438

Milerius · 2021-07-07T17:30:23Z

cc @cipig this pr also fix all Firo hidden transaction

cipig · 2021-07-07T17:40:05Z

https://github.com/cipig/atomicDEX-Desktop/actions/runs/1008681807
has Desktop build with this mm2... can be used for further testing
i actually use mm2 from dev branch since a longer time on my LP nodes and in the Desktop builds, so i don't expect any major flaws

Milerius · 2021-07-07T18:12:20Z

https://github.com/KomodoPlatform/atomicDEX-Desktop/actions/runs/1008994845 @tonymorony @smk762 to test on all platform

cipig · 2021-07-07T18:31:30Z

@Milerius

when trying to withdraw SYS to sys1q5y43e8tguhu9p2yah0q8mr0p5nkaqh3wc0e6nl (Binance) with this settings:

        {
                "coin": "SYS",
                "name": "syscoin",
                "fname": "Syscoin",
                "rpcport": 8370,
                "pubtype": 63,
                "p2shtype": 5,
                "wiftype": 128,
                "txfee": 10000,
                "dust": 1820,
                "segwit": true,
                "bech32_hrp": "sys",
                "required_confirmations": 5,
                "avg_blocktime": 1,
                "mm2": 1,
                "protocol": {
                        "type": "UTXO"
                }
        }

works fine on CLI: curl --url "http://127.0.0.1:7783" --data "{\"userpass\":\"$userpass\",\"method\":\"withdraw\",\"coin\":\"SYS\",\"to\":\"sys1q5y43e8tguhu9p2yah0q8mr0p5nkaqh3wc0e6nl\",\"amount\":1}"
https://chainz.cryptoid.info/sys/tx.dws?d9f57e0c9fa263774417ff0f34b16665c78fdad73a318bb14a38071e62eecc5e.htm
output goes to Binance segwit address, change back to mm2 legacy

Milerius · 2021-07-07T19:14:41Z

validate address seems to not work with segwit address, not sure i can do anything ?

cipig · 2021-07-07T19:20:51Z

i see:

curl -s --url "http://127.0.0.1:7783" --data "{\"userpass\":\"$userpass\",\"method\":\"validateaddress\",\"coin\":\"SYS\",\"address\":\"sys1q5y43e8tguhu9p2yah0q8mr0p5nkaqh3wc0e6nl\"}" | json_pp
{
   "result" : {
      "reason" : "utxo_common:1529] Address 1FhBSLvRyZ5NXsTzR5VP2ymaimZmjT8YGM has invalid prefixes",
      "is_valid" : false
   }
}

artemii235 · 2021-07-09T12:53:05Z

Blocked temporary by this PR: #1009. Approximate resolve ETA: today or Monday next week 🙂

* segwit validateaddress * fix clippy warning * review and test fixes * fix convert to legacy address * add segwit to address_from_any_format * convertaddress fixes * move failing tests to docker tests * review fixes * move "QTUM" inside match statement * review fix

* fix withdraw to p2sh * fix fmt warning * review fixes

* WIP. Updating deps. * WIP. Fixing new clippy. * WIP. Fixing new clippy. * WIP. Checking error stack traces. * Remove some code. * WIP. Rollback toolchain to fix MmError stack trace collection. * Removed outdated libsecp256k1 versions from deps. Added more electrums for tBCH tests. * Replaced bigint with uint and term with crossterm. * Fix fmt. * Fix WASM. Enable resolver = "2". * Add deny.toml and cargo deny CI task. * Remove unneeded #![feature(try_trait_v2)].

* version p2p request * fix wasm errors * first review fixes * second review fixes * fix wasm errors * avoid reserved peers disconnect in maintain relays

smk762 · 2021-08-12T08:45:48Z

Comparing versions {"result":"7489372e9","datetime":"2021-08-11T04:33:17+00:00"} vs {"result":"2.1.3731_mm2.1_e98c61217_Linux_Release","datetime":"2021-06-16T18:54:52+07:00"}

Smoke test in GUIs. All basic operations should work properly. ✔️

Loads and does simple swap. More intensive segwit related testing pending.

Libp2p was updated - please check the compatibility between old and updated nodes in different combinations (new seeds - old GUI, old seeds - new GUI). ✔️

Same seed on both versions in CLI = same address
New seed used on old gui = same address
Old private key used as seed = same address on old and new gui.

KMD rewards should be properly displayed in the tx_history. ✔️

History and rewards value matching what is shown on komodod.com

max_taker_vol should return the coin ticker. ✔️

Confirmed

tonymorony · 2021-08-12T09:53:03Z

Smoke test in GUIs. All basic operations should work properly.

Passed on desktop ( https://github.com/KomodoPlatform/atomicDEX-Desktop/actions/runs/1119665975 ).
Can see full orderbooks, performed non-segwit LTC tx, swap KMD<->LTC

Libp2p was updated - please check the compatibility between old and updated nodes in different combinations (new seeds - old GUI, old seeds - new GUI).

a) New GUI <-> Old seed works, passed (point 1 test actually)
b) Old GUI <-> New seed (GUI with "old" mm2 pointed to updated test seed: https://github.com/KomodoPlatform/atomicDEX-Desktop/actions/runs/1123212193), passed

KMD rewards should be properly displayed in the tx_history.

Do have concerns in some cases, e.g. :

"coin":"KMD","internal_id":"72a679bc75c4f3691e9733e5074c08c35aec9d068817dccb2bf514694788b062","kmd_rewards":{"amount":"0.00187866","claimed_by_me":false},"confirmations":1373}

rewards amount is "amount":"0.00187866" but it doesn't look like in this payment spending tx were claimed any rewards
https://www.kmdexplorer.io/tx/72a679bc75c4f3691e9733e5074c08c35aec9d068817dccb2bf514694788b062

tx details from full KMD node: https://paste.ubuntu.com/p/Fz33qVZvRR/
I see interest in vout, yep. However, it's 3 times less than "amount":"0.00187866"

  "vout": [
    {
      "value": 19.89667262,
      "interest": 0.00064827,

max_taker_vol should return the coin ticker.

it returns a ticker now

Works as expected it seems

"bids":[{"coin":"LTC","address":"ltc1qfqdanp8j0cgfnnp0qxw7mk4qzyv6dk007v4dxt","price":"2","price_rat":
...

6) For UTXO coins withdrawal to legacy addresses should always work. Withdrawal to P2SH and Bech32 addresses should not work if the coin doesn't have the segwit:true in the config. Please ensure that the resulting transaction sends funds to the selected address and the change is sent to the node's address by validating it in the explorer.

a) withdrawal from segwit to legacy: https://blockchair.com/litecoin/transaction/1cf64f95dc0bb508c9e7c7e0654cf68c9f04f507d835a7c1c1783304f17c895d (max: true so no change, change was sent correctly when withdrawn from legacy to segwit)

b) confirmed:

curl --url "http://127.0.0.1:7783" --data "{"method":"withdraw","coin":"LTC","to":"ltc1q5x0h4gj5mnks59ujp2cu9t8knzzp7elkrw5uhr","amount":"0.01","userpass":"$userpass"}"
{"error":"rpc:173] dispatcher_legacy:155] dispatcher_legacy:165] utxo_common:1380] Invalid address: utxo_common:251] utxo:525] Segwit not activated in the config for LTC"}


7)

Performed RICK <-> LTC (segwit) swap

maker payment:
https://blockchair.com/litecoin/transaction/3c8d091b4a23fd70649695213de93e0b2194d60451e99a9ffde838a3474fcf25

taker payment:
https://rick.explorer.dexstats.info/tx/da5f14715029aa12fca4b7763a8156d12db14d4dde2cd793d402e75d4343b9b3

smk762 · 2021-08-12T13:46:49Z

Swaps:
7489372e9 GUI KMD to 7489372e9 CLI segwit LTC
swap completes ✔️
KMD received in CLI
LTC received in GUI (to legacy address)

16ae4923-22bf-4eb4-b1b0-6f6c7f3339c3.txt
https://kmdexplorer.io/tx/4127a65cc0cad0096ff2e0743791d7fc2fef6ec3609ca1303f6032ec0c5119ff
https://litecoinblockexplorer.net/tx/b17b877e7f7bb3012c078584c4e73f36fa1ec868136e92df01be6790bce10472
https://litecoinblockexplorer.net/tx/4e0a391e9f216e04be19558ed0c482ea9cac8b7b10fbab4470fadbd7c392aadb

0.4.3 GUI KMD to 7489372e9 CLI segwit LTC
order fails to match ⛔

tonymorony

tested trough the checklist - lgtm

tonymorony · 2021-08-12T14:15:35Z

0.4.3 GUI KMD to 7489372e9 CLI segwit LTC
order fails to match ⛔

great news that such swap has not started! that's expected behavior (old mm will not able to validate segwit transaction that's why swap does not kick start to prevent funds loss)

…1037 (#1038) * Fix trade_preimage, withdraw if max and force_min_relay_fee are set * Add a test * Add 'test_generate_transaction_relay_fee_is_used_when_dynamic_fee_is_lower_and_deduct_from_output' * Fix utxo_tests after rebasing the dev branch

* Move best_orders logs to debug level. Update Hyper. Temporary comment getrandom. * Fix cyclic package dependency Co-authored-by: Sergey O. Boyko <sergey.boyko0791@gmail.com>

* fix pair trie to be same for old and new nodes * rename function to clone_without_protocol_info * old nodes to see segwit orders but can not match * Remove protocol_info from OrderbookItem/MakerOrder

sergeyboyko0791 · 2021-08-20T10:46:50Z

In the last merged PR #1046 we decided to remove recently added fields from the Ordermatching protocol. It allowed us to fix orders disappearing from the Orderbook.
Unfortunately, we had to get rid of the ability to determine the format of the address (Legacy or Segwit) displayed in the Orderbook.

What we have now:

Old nodes will see orders of the makers who support only Segwit transactions, but they can't match such orders (matching should be failed, swap shouldn't start);
Maker address is displayed as a Legacy address even if the Maker supports only Segwit transactions;

Thanks @shamardy for the proposed solutions and for the fix.
He also proposed a work around for displaying the correct address for Segwit maker orders: #1046 (comment)

I think we can merge the dev branch to mm2.1 now without the ability to determine the correct address (Legacy or Segwit). After that, we'll brainstorm with @artemii235 to figure out what is the best solution.

* Make 'test_match_maker_order_and_taker_request' red * Don't match a taker order if the result base volume greater than max base volume * Check if the resulting base_amount doesn't violate min_base_vol * Extend test_match_maker_order_and_taker_request

sergeyboyko0791 · 2021-08-23T08:33:13Z

Please test if these issues don't reproduce:
#1025
#1031

To test them it's needed to run seednodes from the dev branch (locally or even update main seednodes).

tonymorony

performed the following tests on the latest commit:

old client <-> updated seed: see full order books, traded fine
updated client <-> old seed: see full order books, traded fine

swaps:

new client with segwit enabled <-> new client with not enabled segwit (LTC/KMD swap), performed fine

new client with segwit enabled <-> old client (LTC/KMD swap), swap not matched as expected

also can confirm that only legacy addresses are visible in order books in all cases

best orders worked correctly for me on 5 pairs from 5 checked

all test session swaps statuses were broadcasted to seeds

at the moment checking with @sergeyboyko0791 tx_history rewards

tonymorony

lgtm

sergeyboyko0791 and others added 17 commits June 9, 2021 13:52

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

9bef01e

# Conflicts: # mm2src/coins/utxo/utxo_tests.rs

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

b477ede

add option to disable order history #968 (#972)

11a78a2

* add option to disable order history * move save in history to orders structs * fix failing tests * fix makerorder deserialization

hotfix for disable order history

4ef0bd3

Merge pull request #984 from KomodoPlatform/mm2.1-hotfix-disable-orde…

3e21f42

…r-history hotfix for disable order history #968

Support FIRO Lelantus verbose tx #983 (#986)

4a40e5e

Add QRC20_PAYMENT_GAS_LIMIT of 200000 #980 (#988)

1e75b09

Fix the duplicates in best_orders response #989 (#990)

7b6e773

* WIP. Trying to recreate the problem - not successful. * Bug seems to be fixed.

Add coin field to max_taker_vol response #991 (#996)

eae9bf8

Properly display fees for FIRO Lelantus txs in history. (#994)

c36a84a

#983

display cancellation reason in order_status RPC #987 (#997)

1bb6111

* display cancellation reason in order_status * fix wasm build issue * review fix

Cancel the kickstarted order immediately if balance is not sufficient #…

728fe3d

…973 (#992) * Cancel the kickstarted order immediately if balance is not sufficient. #973 * Delete the order from DB on kickstart balance check cancellation.

artemii235 requested review from tonymorony and cipig July 7, 2021 09:37

Hotfix display balance for segwit addresses in native client (#1005)

41c6ce1

* fix segwit display balance for native client * fix formating * add bech32_hrp check for segwit address format

shamardy and others added 3 commits August 4, 2021 13:11

Hotfix for withdraw to p2sh addresses #1027 (#1029)

535afd8

* fix withdraw to p2sh * fix fmt warning * review fixes

version p2p request #931 (#1026)

7489372

* version p2p request * fix wasm errors * first review fixes * second review fixes * fix wasm errors * avoid reserved peers disconnect in maintain relays

tonymorony mentioned this pull request Aug 11, 2021

update mm2 for smoke testing KomodoPlatform/komodo-wallet#1184

Closed

tonymorony requested a review from smk762 August 11, 2021 12:37

tonymorony previously approved these changes Aug 12, 2021

View reviewed changes

sergeyboyko0791 dismissed tonymorony’s stale review via 8a1680f August 17, 2021 09:21

shamardy and others added 3 commits August 17, 2021 20:14

fix signature for non BCH fork_id coins

10d007b

Remove excessive warnings. Update Hyper. (#1043)

67706d0

* Move best_orders logs to debug level. Update Hyper. Temporary comment getrandom. * Fix cyclic package dependency Co-authored-by: Sergey O. Boyko <sergey.boyko0791@gmail.com>

Fix orders disappearing from orderbook (#1046)

66bbde2

* fix pair trie to be same for old and new nodes * rename function to clone_without_protocol_info * old nodes to see segwit orders but can not match * Remove protocol_info from OrderbookItem/MakerOrder

This was referenced Aug 22, 2021

[Bug]: Some swaps are not broadcasted #1031

Closed

[bug]: best orders inconsistent behavior #1025

Closed

add signature_version to coin configurations

a1cb0a4

tonymorony mentioned this pull request Aug 23, 2021

update mm2 to a1cb0a4c5 KomodoPlatform/komodo-wallet#1204

Closed

sergeyboyko0791 mentioned this pull request Aug 23, 2021

Unexpected KMD rewards amount #1050

Closed

tonymorony self-requested a review August 23, 2021 10:30

tonymorony reviewed Aug 23, 2021

View reviewed changes

sergeyboyko0791 and others added 2 commits August 23, 2021 19:41

Temporary disable calculating kmd_rewards for tx history

32865e7

Merge branch 'mm2.1' into dev

ddebc9c

tonymorony self-requested a review August 23, 2021 14:24

tonymorony approved these changes Aug 23, 2021

View reviewed changes

tonymorony merged commit 4117074 into mm2.1 Aug 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Segwit support. WASM P2P. Bug fixes. #1004

Segwit support. WASM P2P. Bug fixes. #1004

artemii235 commented Jul 7, 2021

artemii235 commented Jul 7, 2021

cipig commented Jul 7, 2021

Milerius commented Jul 7, 2021

cipig commented Jul 7, 2021

Milerius commented Jul 7, 2021

cipig commented Jul 7, 2021

Milerius commented Jul 7, 2021

cipig commented Jul 7, 2021

artemii235 commented Jul 9, 2021

smk762 commented Aug 12, 2021 •

edited

Loading

tonymorony commented Aug 12, 2021 •

edited

Loading

smk762 commented Aug 12, 2021 •

edited

Loading

tonymorony left a comment

tonymorony commented Aug 12, 2021

sergeyboyko0791 commented Aug 20, 2021

sergeyboyko0791 commented Aug 23, 2021

tonymorony left a comment

tonymorony left a comment

Segwit support. WASM P2P. Bug fixes. #1004

Segwit support. WASM P2P. Bug fixes. #1004

Conversation

artemii235 commented Jul 7, 2021

artemii235 commented Jul 7, 2021

cipig commented Jul 7, 2021

Milerius commented Jul 7, 2021

cipig commented Jul 7, 2021

Milerius commented Jul 7, 2021

cipig commented Jul 7, 2021

Milerius commented Jul 7, 2021

cipig commented Jul 7, 2021

artemii235 commented Jul 9, 2021

smk762 commented Aug 12, 2021 • edited Loading

tonymorony commented Aug 12, 2021 • edited Loading

smk762 commented Aug 12, 2021 • edited Loading

tonymorony left a comment

Choose a reason for hiding this comment

tonymorony commented Aug 12, 2021

sergeyboyko0791 commented Aug 20, 2021

sergeyboyko0791 commented Aug 23, 2021

tonymorony left a comment

Choose a reason for hiding this comment

tonymorony left a comment

Choose a reason for hiding this comment

smk762 commented Aug 12, 2021 •

edited

Loading

tonymorony commented Aug 12, 2021 •

edited

Loading

smk762 commented Aug 12, 2021 •

edited

Loading