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.

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

[BUG]: Recipients displaying not properly for transaction to segwit addresses. Tx history is requested for legacy address. #1055

Closed
tonymorony opened this issue Aug 31, 2021 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@tonymorony
Copy link

tonymorony commented Aug 31, 2021

I've sent funds to ltc1q5x0h4gj5mnks59ujp2cu9t8knzzp7elkrw5uhr
https://blockexplorer.one/litecoin/mainnet/tx/9bcf7736e4cc46551a137ebda5e53f3bfd6f775b92c1155afcae7b77947cf33c

but app displaying change (sending) address as a recipient:

Screenshot 2021-08-31 at 23 27 27

it comes from API response, so transferred here:

curl --url "http://127.0.0.1:7783" --data "{\"userpass\":\"$userpass\",\"method\":\"my_tx_history\",\"coin\":\"LTC\",\"limit\":1}"

{
  "result": {
    "transactions": [
      {
        "tx_hex": "0100000001ff420a791fa4293ff13c659e226a24b337bc022e6312d633e7dca01b6942c48f020000006b483045022100c534d95e591e45f310c2505ab5d77eaad50bed8fce847a97e1d4ffeff7bd85c602200153c159014ad62d5731d7d606bfc6f5b76d580d8187efb91d5cab3c555519ce012103aa464529c190398d44017c3d7a71a854ad195a9d396bf84182679c242bb60085ffffffff0240420f0000000000160014a19f7aa254dced0a17920ab1c2acf698841f67f6b1ffae00000000001976a914a19f7aa254dced0a17920ab1c2acf698841f67f688ac30582e61",
        "tx_hash": "9bcf7736e4cc46551a137ebda5e53f3bfd6f775b92c1155afcae7b77947cf33c",
        "from": [
          "LZxY6tuAUYxUcjZ6Betq8Wt4BkAQyjvgdH"
        ],
        "to": [
          "LZxY6tuAUYxUcjZ6Betq8Wt4BkAQyjvgdH"
        ],
        "total_amount": "0.12468944",
        "spent_by_me": "0.12468944",
        "received_by_me": "0.11468721",
        "my_balance_change": "-0.01000223",
        "block_height": 2114643,
        "timestamp": 1630427291,
        "fee_details": {
          "type": "Utxo",
          "amount": "0.00000223"
        },
        "coin": "LTC",
        "internal_id": "9bcf7736e4cc46551a137ebda5e53f3bfd6f775b92c1155afcae7b77947cf33c",
        "confirmations": 17
      }
    ],
    "limit": 1,
    "skipped": 0,
    "from_id": null,
    "total": 76,
    "current_block": 2114659,
    "sync_status": {
      "state": "Finished"
    },
    "page_number": null,
    "total_pages": 76
  }
}

am I need to have segwit enabled to see segwit addresses in tx history?

@tonymorony tonymorony transferred this issue from KomodoPlatform/komodo-wallet Aug 31, 2021
@tonymorony tonymorony added bug Something isn't working question Further information is requested labels Aug 31, 2021
@shamardy
Copy link
Collaborator

shamardy commented Aug 31, 2021

This is definitely a bug as Segwit was not implemented for tx history.
I think this shows only the received_by_me without showing the Segwit address. This should show 2 addresses with segwit as the first one

"to": [
        "ltc1q5x0h4gj5mnks59ujp2cu9t8knzzp7elkrw5uhr",
        "LZxY6tuAUYxUcjZ6Betq8Wt4BkAQyjvgdH"
       ]

I can see the problem is in https://github.com/KomodoPlatform/atomicDEX-API/blob/41170748db961f23aa8a10fffc3de09a2bd81aa9/mm2src/coins/utxo/utxo_common.rs#L1909 where WitnessScript has not been implemented yet for extract_destinations https://github.com/KomodoPlatform/atomicDEX-API/blob/41170748db961f23aa8a10fffc3de09a2bd81aa9/mm2src/mm2_bitcoin/script/src/script.rs#L422-L424

Also, there will be a problem when using my_tx_history when Segwit is enabled as it will show tx history for the legacy address instead https://github.com/KomodoPlatform/atomicDEX-API/blob/41170748db961f23aa8a10fffc3de09a2bd81aa9/mm2src/coins/utxo/utxo_common.rs#L1801

Will work on solving this this week

@shamardy shamardy self-assigned this Aug 31, 2021
@tonymorony tonymorony removed the question Further information is requested label Aug 31, 2021
@artemii235 artemii235 changed the title [BUG]: Recipients displaying not properly for transaction to segwit addresses [BUG]: Recipients displaying not properly for transaction to segwit addresses. Tx history is requested for legacy address. Sep 2, 2021
artemii235 pushed a commit that referenced this issue Sep 6, 2021
* segwit transaction history

* extract_destinations p2wpkh test + cashaddress fix
@artemii235
Copy link
Member

@tonymorony Could you please test if the problems are fixed in the dev branch?

@tonymorony
Copy link
Author

@smk762 could you please help with re-testing it on the latest dev?

approx test-plan:

  1. perform few txs in non-segwit
    activation example: curl --url "http://127.0.0.1:7783" --data "{\"userpass\":\"$userpass\",\"method\":\"electrum\",\"coin\":\"LTC\",\"servers\":[{\"url\":\"electrum1.cipig.net:10063\"}],\\"tx_history\":true}"

  2. check that it's displaying correctly in the tx history call

  3. re-activate coin in segwit mode
    activation example:
    curl --url "http://127.0.0.1:7783" --data "{\"userpass\":\"$userpass\",\"method\":\"electrum\",\"coin\":\"LTC\",\"servers\":[{\"url\":\"electrum1.cipig.net:10063\"}],\"address_format\":{\"format\":\"segwit\"},\"tx_history\":true}"),
    check that there is no non-segwit txs

  4. perform a couple of transactions in segwit mode

  5. check that it's correctly displaying in segwit mode wallet tx history (txid, amount, sender/recepient)

  6. switch to legacy and check that there are no segwit txs in tx history and legacy txs still there

@smk762
Copy link

smk762 commented Sep 8, 2021

I loaded up a wallet which already had some tx for both segwit and legacy - both showed correct tx history for type out of the box.

Activating segwit address, I sent to both a legacy and a different segwit address - both were added to the history when segwit activated, and not shown when legacy activated.

Amounts, fees, and addresses in tx history all matching what is shown on explorer 👍

artemii235 added a commit that referenced this issue Sep 10, 2021
Several hot fixes: mem leak, approve gas limit, segwit tx history.
#1055 #1024 #941
@smk762
Copy link

smk762 commented Sep 20, 2021

@artemii235 Is further testing required here or can we close this issue?

@artemii235
Copy link
Member

Is further testing required here or can we close this issue?

I think no further testing is required, thanks! I'm closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants