Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Full SegWit support. #826

Closed
4 tasks
artemii235 opened this issue Feb 17, 2021 · 10 comments
Closed
4 tasks

Full SegWit support. #826

artemii235 opened this issue Feb 17, 2021 · 10 comments
Assignees
@artemii235 @tonymorony @smk762 @shamardy

Comments

@artemii235
Copy link
Member

artemii235 commented Feb 17, 2021 •
edited by shamardy
Loading

  • Add full_segwit param to enable/electrum, which forces MM2 to generate SegWit address instead of legacy. The SegWit address is returned in response
  • Implement SegWit tx signing. Changes are possibly required in parity-bitcoin dependency: https://github.com/artemii235/parity-bitcoin
  • Integrate SegWit signing to withdraw and atomic swaps transactions generation.
  • If the coin is reactivated on restart using a different address format it may affect the created orders. We should put a warning in docs in regards to it.
@mmacedoeu mmacedoeu mentioned this issue Mar 15, 2021
Closed
@Milerius Milerius mentioned this issue Apr 9, 2021
Closed
This was referenced Apr 22, 2021
Closed
Closed
mmacedoeu added a commit that referenced this issue Apr 23, 2021
@mmacedoeu
ADD segwit support #826
9c258f2
@artemii235 artemii235 assigned mmacedoeu and shamardy and unassigned mmacedoeu Apr 30, 2021
@shamardy shamardy mentioned this issue May 26, 2021
Merged
7 tasks
artemii235 pushed a commit that referenced this issue Jun 1, 2021
@shamardy
full segwit support #826 (#953)
500650d 
* port "parity-bitcoin" to "mm2_bitcoin"

* remove unnecessary files from bitcoin lib

* fix compiler warnings

* fix clippy warnings

* fix formatting

* p2wpkh support for electrum and my_balance

* remove unnecessary code and minor fixes

* add "hrp" for addresses other than btc

* add "address_format" to electrum request

* Fix clippy warning

* fix test build issue
@artemii235 artemii235 mentioned this issue Jun 2, 2021
Closed
@shamardy shamardy mentioned this issue Jun 8, 2021
Merged
artemii235 pushed a commit that referenced this issue Jun 30, 2021
@shamardy
Segwit Withdraw and Transaction Signing #826 (#971)
1861116 
* 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
@artemii235 artemii235 self-assigned this Sep 13, 2021
@artemii235 artemii235 mentioned this issue Sep 13, 2021
Merged
artemii235 added a commit that referenced this issue Sep 14, 2021
@artemii235
Support Segwit addresses in orderbooks #826 (#1064)
7173ae2 
* WIP.

* WIP.

* WIP.

* WIP. Some tests fail.

* Fix tests and add more.

* Fix WASM tests and clippy.

* Review fixes. Return Option<_> from order_getter to avoid unwrap/panic.
@artemii235
Copy link
Member Author

@tonymorony I hurried and occasionally merged the last PR with order books support to mm2.1. It's not released automatically so I think it's fine to test things directly in the release branch this time 🙂
Could you please check the following:

  1. Activate any coin in Segwit mode on the new node. Create a maker order.
  2. The new node displays the address in a proper format for this order.
  3. Older nodes can see this order - the address will be displayed as a legacy.
  4. Order books should not "flash"/order should not disappear while the maker is running. Please check this on different devices - it's better if they run different MM2 versions.

@smk762
Copy link

smk762 commented Sep 15, 2021

AtomicDex Desktop 0.5.0-beta (mm2 version 9b69b90) vs CLI (7173ae2)

Trade 1

  • placed maker order in CLI with latest mm2.1 binary (7173ae2)
  • Base LTC(segwit) / Rel KMD
  • order seen in desktop app orderbook on different PC (mm2 version 9b69b90)
  • order initiated (cb4dc41f-72c1-4811-a2eb-06d62a1e7862) and completed
  • GUI received LTC into legacy address

Trade 2

  • placed maker order in CLI with latest mm2.1 binary (7173ae2)
  • Base KMD / Rel LTC(segwit)
  • order seen in desktop app orderbook on different PC (mm2 version 9b69b90)
  • order initiated (d5e5ee69-1062-4a12-9551-3fe292bde340) and completed
  • CLI received LTC into segwit address

AtomicDex Desktop 0.4.3-beta (mm2 version 419f36e) vs CLI (7173ae2)

Trade 1

  • placed maker order in CLI with latest mm2.1 binary (7173ae2)
  • Base LTC(segwit) / Rel KMD
  • order seen in desktop app orderbook on different PC (mm2 version 2adfc36)
  • order does not match

Trade 2

  • placed maker order in CLI with latest mm2.1 binary (7173ae2)
  • Base KMD / Rel LTC(segwit)
  • order seen in desktop app orderbook on different PC (mm2 version 2adfc36)
  • order does not match

AtomicDex Desktop 0.4.2-beta (mm2 version 2adfc36) vs CLI (7173ae2)

Trade 1

  • placed maker order in CLI with latest mm2.1 binary (7173ae2)
  • Base LTC(segwit) / Rel KMD
  • order seen in desktop app orderbook on different PC (mm2 version 2adfc36)
  • order does not match

Trade 2

  • placed maker order in CLI with latest mm2.1 binary (7173ae2)
  • Base KMD / Rel LTC(segwit)
  • order seen in desktop app orderbook on different PC (mm2 version 2adfc36)
  • order does not match

@tonymorony
Copy link

tyvm for the testing @smk762 !

@artemii235
Copy link
Member Author

Thanks for the comprehensive testing! Have there been any issues like in #1044?

@smk762
Copy link

smk762 commented Sep 15, 2021

Stopped mm2 CLI to test other things, will restart and continue to monitor this evening soon

@smk762
Copy link

smk762 commented Sep 17, 2021

Thanks for the comprehensive testing! Have there been any issues like in #1044?

Not seen here 👍

@artemii235
Copy link
Member Author

Thanks! I will publish a new release today.

@artemii235
Copy link
Member Author

@tonymorony @smk762 Is testing finished? 🙂

@smk762
Copy link

smk762 commented Jun 20, 2022 •
edited
Loading

I tested this a while ago in gui - Roman was working on it. The code is present in desktop, but not active in front end. Referring to my last comment in this issue, testing appears to be complete, though I'm happy to revisit it for confirmation if required.

@smk762
Copy link

smk762 commented Jan 20, 2023

@artemii235 I think we can close this one now

@smk762 smk762 closed this as completed Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@artemii235 artemii235

@tonymorony tonymorony

@smk762 smk762

@shamardy shamardy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mmacedoeu @artemii235 @tonymorony @smk762 @shamardy