Don’t show addresses or P2PK in decoderawtransaction #16725

pull NicolasDorier wants to merge 1 commits into bitcoin:master from NicolasDorier:fix/p2pk-not-p2pkh changing 2 files +3 −7
  1. NicolasDorier commented at 5:00 am on August 26, 2019: contributor

    I spent significant amount of time explaining to people that satoshi did not had any “bitcoin address”, because bitcoin address was not existing at the time.

    Then I need to explain them that all blockchain explorer are wrong. Then I understood that the source of this widespread mistake come from Bitcoin Core itself.

    For:

    0bitcoin-cli -regtest decoderawtransaction 01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff4d04ffff001d0104455468652054696d65732030332f4a616e2f32303039204368616e63656c6c6f72206f6e206272696e6b206f66207365636f6e64206261696c6f757420666f722062616e6b73ffffffff0100f2052a01000000434104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac00000000

    Before:

     0{
 1  "txid": "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b",
 2  "hash": "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b",
 3  "version": 1,
 4  "size": 204,
 5  "vsize": 204,
 6  "weight": 816,
 7  "locktime": 0,
 8  "vin": [
 9    {
10      "coinbase": "04ffff001d0104455468652054696d65732030332f4a616e2f32303039204368616e63656c6c6f72206f6e206272696e6b206f66207365636f6e64206261696c6f757420666f722062616e6b73",
11      "sequence": 4294967295
12    }
13  ],
14  "vout": [
15    {
16      "value": 50.00000000,
17      "n": 0,
18      "scriptPubKey": {
19        "asm": "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f OP_CHECKSIG",
20        "hex": "4104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac",
21        "reqSigs": 1,
22        "type": "pubkey",
23        "addresses": [
24          "mpXwg4jMtRhuSpVq4xS3HFHmCmWp9NyGKt"
25        ]
26      }
27    }
28  ]
29}

    After

     0{
 1  "txid": "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b",
 2  "hash": "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b",
 3  "version": 1,
 4  "size": 204,
 5  "vsize": 204,
 6  "weight": 816,
 7  "locktime": 0,
 8  "vin": [
 9    {
10      "coinbase": "04ffff001d0104455468652054696d65732030332f4a616e2f32303039204368616e63656c6c6f72206f6e206272696e6b206f66207365636f6e64206261696c6f757420666f722062616e6b73",
11      "sequence": 4294967295
12    }
13  ],
14  "vout": [
15    {
16      "value": 50.00000000,
17      "n": 0,
18      "scriptPubKey": {
19        "asm": "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f OP_CHECKSIG",
20        "hex": "4104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac",
21        "type": "pubkey"
22      }
23    }
24  ]
25}

    This mistake is having widespread impact, as developer thinks P2PK are addresses, they start running into issues when somebody send a P2PK payment to them and then they don’t understand why they can’t sign it like a P2PKH.

  2. NicolasDorier force-pushed on Aug 26, 2019
  3. NicolasDorier renamed this:
    P2PK should not have address
    [WIP] P2PK should not have address
    on Aug 26, 2019
  4. NicolasDorier force-pushed on Aug 26, 2019
  5. NicolasDorier force-pushed on Aug 26, 2019
  6. NicolasDorier commented at 8:26 am on August 26, 2019: contributor
    Trying to refactor so that ExtractDestination don’t return addresses is a can of worm… I will instead limit this change to RPC return only. It is enough to prevent people from copying this bad design, while making sure nothing else break.
  7. NicolasDorier force-pushed on Aug 26, 2019
  8. NicolasDorier force-pushed on Aug 26, 2019
  9. NicolasDorier renamed this:
    [WIP] P2PK should not have address
    P2PK should not have address
    on Aug 26, 2019
  10. NicolasDorier commented at 9:44 am on August 26, 2019: contributor
    This is ready to review.
  11. NicolasDorier commented at 9:45 am on August 26, 2019: contributor
    Note that I keep addresses as empty array instead of removing the array completely. This will minimize chances to break clients.
  12. laanwj added the label RPC/REST/ZMQ on Aug 26, 2019
  13. MarcoFalke commented at 12:38 pm on August 26, 2019: member
    Does it also remove addresses from getreceivedbylabel? #16655 (comment)
  14. promag commented at 12:53 pm on August 26, 2019: member
    It doesn’t break the interface but probably will break clients as they are expecting one address. Probably better is to remove the field and add a deprecation option?
  15. NicolasDorier commented at 1:44 pm on August 26, 2019: contributor

    It doesn’t break the interface but probably will break clients as they are expecting one address. Probably better is to remove the field and add a deprecation option?

    Though call. I am unsure about the deprecation though, program using addresses and expecting one address are necessarily broken already anyway.

    Does it also remove addresses from getreceivedbylabel?

    It does not. I tried to change the logic of ExtractDestination so that this change is more general, but it turned out to be a can of worms that would require some deeper refactoring, which I don’t think really justify the end goal.

    That said getreceivedbylabel should not show P2PK as addresses, I will comment directly on this PR.

  16. Sjors commented at 4:51 pm on August 27, 2019: member

    Concept ACK

    The convention for outputs without an address is to remove the array, rather than make it empty. See e.g. this OP_RETURN example: 8bae12b5f4c088d940733dcd1455efc6a3a69cf9340e17a981286d3778615684. If we follow that pattern imo there’s no need to deprecate anything.

  17. NicolasDorier commented at 4:11 am on August 28, 2019: contributor
    @Sjors good point, will do.
  18. egp approved
  19. egp commented at 1:35 pm on August 28, 2019: none
    LGTM, thanks
  20. NicolasDorier force-pushed on Aug 28, 2019
  21. NicolasDorier force-pushed on Aug 28, 2019
  22. NicolasDorier force-pushed on Aug 28, 2019
  23. NicolasDorier commented at 2:39 pm on August 28, 2019: contributor
    Ok I improved the PR as @Sjors suggested. It is actually less line of code changed and clearer.
  24. laanwj commented at 2:47 pm on August 29, 2019: member

    Concept ACK. This is less risky than changing ExtractDestinations, of which it’d be much harder to oversee the consequences. On the other hand it’s possible that addresses for P2PK still show up in other places.

    Edit: so it might be better to rename this PR to more specifically don't show addresses or P2PK in decoderawtransaction.

  25. NicolasDorier renamed this:
    P2PK should not have address
    Don't show addresses or P2PK in decoderawtransaction
    on Aug 30, 2019
  26. Don't show addresses or P2PK in decoderawtransaction 6d803494b5
  27. NicolasDorier force-pushed on Aug 30, 2019
  28. NicolasDorier commented at 2:30 am on August 30, 2019: contributor
    Changed commit and PR title. That’s fine if it still show at other place. I am trying at least to change the most obvious to prevent the mistake to spread into the mind of new devs.
  29. Sjors commented at 12:18 pm on August 30, 2019: member

    Code review ACK 6d80349.

    In the future we could add the inferred descriptor (though not always useful, without looking at the transaction that spent it to see the full script).

  30. kristapsk approved
  31. kristapsk commented at 1:01 am on August 31, 2019: contributor
    ACK 6d803494b59ab5520079b6a72d97790d86d2a015 (applied changes except test, ran tests, then applied changes to test also)
  32. sipa commented at 1:24 am on August 31, 2019: member

    Code review ACK, and Concept ACK on moving away from showing P2PK as addresses. However, Does this need an RPC deprecation flag? I think it’s unlikely, but someone may be relying on this odd behavior. @Sjors Inferring descriptors isn’t expensive; we should indeed just do that.

    Also, I think we should get rid entirely of the ‘addresses’ field in decode* RPCs; it’s very confusing that UTXOs are shown as having a list of addresses; a UTXO has at most one address.

  33. NicolasDorier commented at 6:02 am on August 31, 2019: contributor

    I don’t think depreciation flag is needed, as people relying on this behavior (if any) were necessarily broken.

    Also, addresses does not exists already for some types of script like OP_RETURN, so I suppose consumer are already handling well the case where this field does not exist.

    Also, I think we should get rid entirely of the ‘addresses’ field in decode* RPCs

    I agree, but I prefer to do it in a followup PR, as it will invalidate current reviews and P2PK is the one that caused me the most trouble to explain to noobs. I can work on removing addresses completely after this one is merged.

  34. DrahtBot commented at 5:39 pm on September 3, 2019: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16795 (rpc: have raw transaction decoding infer output descriptors by instagibbs)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  35. instagibbs commented at 8:19 pm on September 3, 2019: member

    In the future we could add the inferred descriptor (though not always useful, without looking at the transaction that spent it to see the full script).

    I went ahead and implemented this in #16795 since it’s relatively orthogonal.

  36. sipa commented at 0:19 am on September 4, 2019: member

    Also, addresses does not exists already for some types of script like OP_RETURN, so I suppose consumer are already handling well the case where this field does not exist.

    I think you’re incorrectly assuming that the hypothetical person using this functionality is using it on arbitrary transactions, instead of perhaps just transactions he knows are paying to P2PK already.

    That said, I agree this is unlikely.

  37. in src/core_write.cpp:147 in 6d803494b5 
    143@@ -144,7 +144,7 @@ void ScriptToUniv(const CScript& script, UniValue& out, bool include_address)
144     out.pushKV("type", GetTxnOutputType(type));
145 
146     CTxDestination address;
147-    if (include_address && ExtractDestination(script, address)) {
148+    if (include_address && ExtractDestination(script, address) && type != TX_PUBKEY) {
    MarcoFalke commented at 0:34 am on September 5, 2019:
    This code branch is never exercised in the tests. Would be nice to add one or at the very least some release notes.
    NicolasDorier commented at 2:37 am on September 5, 2019:

    ScriptToUniv and ScriptPubKeyToUniv are almost similar. The only real difference is that one has reqSigs and addresses, but the other only has address.

    I think the best way to fix this issue is just modify ScriptPubKeyToUniv to stop returning an array (it makes no sense to show the individual P2PKH of multisig pubkeys) and returning address instead, and then removing ScriptToUniv completely.

  38. MarcoFalke commented at 0:34 am on September 5, 2019: member

    ACK 6d803494b59ab5520079b6a72d97790d86d2a015

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK 6d803494b59ab5520079b6a72d97790d86d2a015
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUizSwv8D4wzMx6dwvUplbocsmZ/+nDCENN8oGB/dhu5VFLZRMh8thvClCVnHw6W
 8t0Ai0KEKP1GuUg1nBwp5zzDwDpZpJCLnyE2+dpfO565etITXmTRzS0S8lzhtwrLR
 9rz+e7seOwyP2VREL1Uo5VHIcjtWJHJXdmEABMgd2N3IYnBml3sFA8atnCuroCAQx
10Fm/HbU+TGCuF3O8V+Va8qBcV/CI+AycX3dbRhhR+mwkGTdk9ybItE93xJAxM5C8k
11A/5iLYx0wDvwGjqIWqspiagWFay1dSaMDqZ8p1wMZ/muGhv3g92XihUmFofmX8Rn
12I4BFxj1Mm6RLVQ3k6Z9J2Cn4tPesepeiTcTqY9lWj9gkl2dYftZQga5nrFElV0FA
13llKH6nT5obh9Cflv0gdADDqDiRW3C3DlHglE5ym9vWeNRmVIzFj5r4DudWxfhChO
14Y6CwGyKx8RZB/zffSWgyA665giiI1fKU7Yip/kGwU/OqC7jYXk+c8DANz/CUU+dU
15XIrACUno
16=qfiR
17-----END PGP SIGNATURE-----

    Timestamp of file with hash c784f2e9ac5c0b2f8a5cd084e2b21f4a349ea8362775e84f8c964f6276ec2420 -

  39. meshcollider commented at 11:40 pm on September 9, 2019: contributor
    utACK 6d803494b59ab5520079b6a72d97790d86d2a015
  40. meshcollider referenced this in commit 335b34c30c on Sep 9, 2019
  41. meshcollider merged this on Sep 9, 2019
  42. meshcollider closed this on Sep 9, 2019
  43. sidhujag referenced this in commit b31e045486 on Sep 10, 2019
  44. sidhujag referenced this in commit b74b1ddf93 on Oct 17, 2019
  45. MarcoFalke commented at 2:55 pm on March 23, 2020: member
    We forgot to add release notes for this. Now it is too late.
  46. andronoob commented at 4:27 pm on June 3, 2020: none

    It seems that I’m too late… @NicolasDorier

    because bitcoin address was not existing at the time.

    screen3 Source: https://web.archive.org/web/20090303195936im_/http://bitcoin.org/screen3.png

    significant amount of time explaining

    IMHO it’s probably better to put something like a reminder/note/extra option etc, instead of making such aggressive change.

  47. andronoob commented at 4:58 am on June 4, 2020: none

    Besides, this “ambiguous” “confusion” logic exists not only in the decoderawtransaction RPC call (oh it has been removed in this pr), but also in the GUI.

    I didn’t do a full rescan, but it’s already enough to illustrate (taken from the freshly released Bitcoin Core 0.20.0):

    fakesatoshiwallet

    I don’t have the time and skills to do “commit archaeology”, but however, I don’t think it’s good to treat this problem in such an aggressive way.

  48. NicolasDorier commented at 5:29 am on June 22, 2020: contributor

    I think this should be fixed at the Bitcoin QT level as well, addresses for P2PK should not show up there.

    I don’t think it’s good to treat this problem in such an aggressive way.

    I had to deal with situations where one address receive a P2PK payment and wallet just crash unable to sign it. Then people see that difference “balances” show up on different block explorers, as some of them consider P2PK have an address and other not.

    P2PK is dead. When somebody share an address he does not expect to be paid in P2PK (neither was expecting it at satoshi time!). This was a long time bug that caused too much bug, and misunderstanding.

  49. NicolasDorier deleted the branch on Jun 22, 2020
  50. andronoob commented at 6:10 am on June 22, 2020: none

    P2PK is dead

    Of course P2PK is ancient thing which is no longer used at all nowadays. However, those ancient P2PK UTXOs still consist of significant fraction of all existing bitcoins. There are also many historical posts mentioning them as 1-starting Base58Check-encoded addresses. Therefore I think it’s still necessary to treat them properly. I don’t think simply refusing to display them is a good way to treat them.

    I had to deal with situations where one address receive a P2PK payment and wallet just crash unable to sign it.

    Is P2PK still spendable? If the answer is still “yes”, then probably it’s the fault of that crashing wallet itself - to my understanding the wallet should fix the bug either by supporting signing P2PK properply or simply to ignore P2PK outputs at all. To me the former seems to be better, because the user would be probably confused that some funds would seem to be “missing”.

    Then people see that difference “balances” show up on different block explorers, as some of them consider P2PK have an address and other not.

    Yeah it’s bad. However I think providing a proper, unambiguous presentation for P2PK should be the correct way to fix this problem.

  51. achow101 commented at 6:40 am on June 22, 2020: member

    Therefore I think it’s still necessary to treat them properly.

    We are treating them properly. The proper way to treat P2PK output is to not show a P2PKH address for them. To show a P2PKH address for a P2PK script is literally incorrect and improper.

    There are also many historical posts mentioning them as 1-starting Base58Check-encoded addresses.

    And these should be corrected. Just because this is something that people do and have done does not mean that it is something we should continue to do. It is incorrect and misleading to show a P2PKH address for a P2PK script. This is a misconception that needs correcting, not appeasing.

    However I think providing a proper, unambiguous presentation for P2PK should be the correct way to fix this problem.

    Then P2PKH addresses are not that. They are literally unambiguously not P2PK. If you want to represent them as a P2PKH address, then those addresses become ambiguous which is not helpful either.

    We already have an unambiguous, though verbose, representation of P2PK scripts: pk() descriptors. We could display a descriptor for P2PK (and maybe bare multisigs too) which are unambiguous representations of those scripts. However they can be very long and unwieldy so the user experience won’t be that great.

  52. NicolasDorier commented at 6:42 am on June 22, 2020: contributor

    Is P2PK still spendable? If the answer is still “yes”, then probably it’s the fault of that crashing wallet itself

    The problem with this argument is that the number of spendable scripts by a public key is unlimited. What about OP_NOP <pubkey> OP_CHECKSIG ? What about 1 <pubkey> 1 OP_MULTISIG ? Why those should not be spendable as well by the wallet and be treated as address?

  53. andronoob commented at 7:26 am on June 22, 2020: none

    The problem with this argument is that the number of spendable scripts by a public key is unlimited.

    Of course.

    Then why is Bitcoin Core still scanning out both P2PK and P2PKH transactions & counting them into the balance, in the case that the corresponding pubkey or privkey is imported? Is this a bug which needs to be fixed as well, by considering P2PK “nonstandard” so that they should be simply ignored/excluded from scanning result?

    To show a P2PKH address for a P2PK script is literally incorrect and improper.

    What’s “correct” seems to be subjective here. What is P2PKH? It is in fact Base58Check-encoded HASH160 of a public key (together with a version number), then why can’t it be used to represent a public key?

    What about 1 1 OP_MULTISIG ? Why those should not be spendable as well by the wallet and be treated as address?

    Aug.31th 2020: Note that BTC.COM has changed its behavoir (I don’t know when they did this) that P2MS is no longer represented with (multiple) 1-starting P2PKH address(es), it shows an error message instead.

    BTC.COM already did this for bare multisigs: https://btc.com/581d30e2a73a2db683ac2f15d53590bd0cd72de52555c2722d9d6a78e9fea510

    Obviously this doesn’t look very elegant & normative. However this is quite clear to me, in other words, it provides better usability than other block explorers which don’t do this.

    Just because this is something that people do and have done does not mean that it is something we should continue to do.

    I never said we should continue to confuse P2PK and P2PKH. I completely agree with the point that the 1-starting P2PKH address itself should not match P2PK outputs. I just think the history should also be respected, just like the bare multisig transaction above, that those pubkeys were reused in other transactions by multiple times - I agree that it’s not good behavoir & it should be discouraged. However it’s just “fait accompli” which cannot be changed anymore.

    And these should be corrected.

    I’m afraid that there would always be some posts which can never be corrected. Simply refusing to display corresponding P2PKH address seems to contradict the history, thus making much more confusion & harming usability.

  54. NicolasDorier commented at 7:36 am on June 22, 2020: contributor

    contradict the history, thus making much more confusion & harming usability.

    The addresses that you were seeing in the satoshi’s version were P2PKH, not P2PK by the way.

    Then why is Bitcoin Core still scanning out both P2PK and P2PKH transactions & counting them into the balance, in the case that the corresponding pubkey or privkey is imported?

    Backward compatibility, new wallet should not care.

  55. andronoob commented at 7:58 am on June 22, 2020: none

    The addresses that you were seeing in the satoshi’s version were P2PKH, not P2PK by the way.

    Yeah, to my knowledge P2PK only appears in ancient coinbase/generating transactions and pay-to-IP transactions (which was a deprecated & then removed feature). However it’s still unclear how Satoshi himself would consider this problem.

    But, anyway, just like what I have said multiple times, the community has been considering things like “the genesis address” “early miner address” from years ago up till now.

    As we all know a hash is not reversible, so that the currently existing pk(KEY) and combo(KEY) descriptor is not an option if the user only knows the P2PKH address, which is essentially a hash. However it’s totally feasible to match P2PK outputs with merely a P2PKH address. I never meant the P2PKH itself should match P2PK outputs. I just meant there should be an option to allow the user to do this, without making the ambiguity/confusion problem worse, probably with extended pk()/combo() output descriptor that they can accept either KEY or (P2PKH) ADDR.

  56. MarcoFalke commented at 11:04 am on June 22, 2020: member
    There is always the descriptor for raw bytes that can be used for any scriptPubKey
  57. andronoob commented at 1:44 pm on June 22, 2020: none

    There is always the descriptor for raw bytes that can be used for any scriptPubKey

    The problem is that the pubkey itself is not yet known, what’s known is only its HASH160.

  58. MarcoFalke commented at 1:45 pm on June 25, 2020: member

    Not sure what you mean. The raw descriptor works fine for me locally:

    0getdescriptorinfo "raw(4104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac)"
1
2{
3  "descriptor": "raw(4104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac)#6zf7atrr",
4  "checksum": "6zf7atrr",
5  "isrange": false,
6  "issolvable": false,
7  "hasprivatekeys": false
8}

    0generatetodescriptor 1 "raw(4104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac)"

     0getblock(getbestblockhash(),2)
 1
 2...
 3
 4      "vin": [
 5        {
 6          "coinbase": "029b0e0101",
 7          "txinwitness": [
 8            "0000000000000000000000000000000000000000000000000000000000000000"
 9          ],
10          "sequence": 4294967295
11        }
12      ],
13      "vout": [
14        {
15          "value": 0.00001194,
16          "n": 0,
17          "scriptPubKey": {
18            "asm": "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f OP_CHECKSIG",
19            "hex": "4104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac",
20            "type": "pubkey"
21          }
22        },
23...
  59. andronoob commented at 2:49 am on June 26, 2020: none

    The raw descriptor works fine for me

    Of course, but only if you know the pubkey itself. If you only know corresponding P2PKH address, which is the hash of the pubkey, I can’t see how the current defined output descriptors can match it.

  60. sipa commented at 2:53 am on June 26, 2020: member

    @andronoob Descriptors (apart from addr() and raw()) are designed to encapsulate all information necessary to spend an output (apart from secret keys). As such, a descriptor for a P2PK output without knowing the full pubkey makes no sense - you’ll need the pubkey to spend it.

    And if you don’t have the pubkey, why do you care about the output in the first place?

  61. andronoob commented at 2:57 am on June 26, 2020: none

    don’t have the pubkey

    Obviously it can be known if any corresponding P2PK output does exist. It will be known once the corresponding P2PK output is matched.

    why do you care about the output

    Because P2PK and P2PKH share exactly the same ownership.

  62. andronoob commented at 3:18 am on June 26, 2020: none
    To be short, pk(1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa) distinguishes from addr(1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa). That’s it.
  63. andronoob commented at 4:34 am on June 26, 2020: none

    I tried decodescript <compressed_p2pk_output_script> and then I found that the response data contained SegWit addresses converted from the pubkey,

    0decodescript 21037be71bdada01429453828831bdedc9a8b773ca9b7cb806a5c7eb9b1520504cbfac

     0{
 1  "asm": "037be71bdada01429453828831bdedc9a8b773ca9b7cb806a5c7eb9b1520504cbf OP_CHECKSIG",
 2  "type": "pubkey",
 3  "p2sh": "3E74dX1LvF6JcCyiCPK3XSaXjk2NFP11Z4",
 4  "segwit": {
 5    "asm": "0 d96e69a24dd5e55a927e8855a4d2c7a57853d870",
 6    "hex": "0014d96e69a24dd5e55a927e8855a4d2c7a57853d870",
 7    "reqSigs": 1,
 8    "type": "witness_v0_keyhash",
 9    "addresses": [
10      "bc1qm9hxngjd6hj44yn73p26f5k854u98krsm0fn5n"
11    ],
12    "p2sh-segwit": "3BLEhRQy2YpqXjmGnD5s1MxG1M5eiMMQcu"
13  }
14}

    while decodescript <uncompressed_p2pk_output_script> won’t do such conversion.

    0decodescript 4104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac

    0{
1  "asm": "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f OP_CHECKSIG",
2  "type": "pubkey",
3  "p2sh": "3DjjKyU38gSfuVxajV43MUy4vHkg1JVL7T"
4}

    Such behavoir seems correct to me. However, if showing SegWit addresses is for convenience, why shouldn’t that convenience include a P2PKH address as well?

    I also found that decodescript would convert a P2PKH output script into SegWit by default. I think that may cause footguns, so I have submitted a new issue: https://github.com/bitcoin/bitcoin/issues/19383

  64. shesek commented at 6:06 am on June 26, 2020: contributor
    Related: #19236
  65. fairglu commented at 7:23 pm on June 30, 2020: none

    As a coder of blockchain explorers, I would like to chime in: would it be possible to decide on a standard way to represent these outputs ?

    If no standard is defined, then this will just breed confusion with every wallet or explorer picking their own representations.

    IMHO the RPC API is a good place to establish such a standard, as it is common ground (while QT UI f.i., is not) Thanks for your consideration!

  66. sipa commented at 9:08 pm on June 30, 2020: member
    The only reasonable and unambiguous way to represent those is as a raw hex script, in my opinion.
  67. MarcoFalke deleted a comment on Jun 30, 2020
  68. fairglu commented at 6:57 am on July 1, 2020: none

    Raw hex could work, but while these are not addresses, the public key can be the same as the one behind an address (and which becomes “public” when an address output is spent), so maintaining some relationship could be useful in terms of user comprehension.

    Address reuse is problematic but it it is widespread (with miners, exchanges or PoS in alts), so it is common for the pubkey behind an address to be… public.

    Would a prefixing or postfixing of the address associated to the pubkey work ?

    Instead of “mpXwg4jMtRhuSpVq4xS3HFHmCmWp9NyGKt” it could be reported as “p2pk_mpXwg4jMtRhuSpVq4xS3HFHmCmWp9NyGKt” or something similar ?

  69. andronoob commented at 8:54 am on July 1, 2020: none

    In my opinion, P2PK should be represent as output descriptor like pk(PUBKEY), because that’s what it technically is.

    I think it’s also good to allow the user to match P2PK output with its corresponding P2PKH address like (this is not yet supported) pk(ADDR). However pk(ADDR) is not so “transparent” like pk(PUBKEY).

  70. andronoob commented at 8:59 am on July 1, 2020: none
    pk(1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa) distinguishes from addr(1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa) or 1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa. I think that’s good enough, although it still requires some explaination.
  71. jasonbcox referenced this in commit ea7128a744 on Oct 16, 2020
  72. etherx-dev commented at 10:45 pm on November 16, 2021: none
    wait so if i import a pubkey into the client and it says “solvable” true does this mean the wallet client can spend the funds? if so why does it contradict and not show up in the balance but on watch only?
  73. sipa commented at 10:47 pm on November 16, 2021: member
    @etherx-dev Please don’t ask questions on random, unrelated, old, closed, pull requests. You’ll probably find better information on https://bitcoin.stackexchange.com.
  74. MarcoFalke locked this on Nov 17, 2021


NicolasDorier MarcoFalke promag Sjors egp laanwj kristapsk sipa DrahtBot instagibbs meshcollider andronoob achow101 shesek fairglu etherx-dev

Labels
RPC/REST/ZMQ

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 13:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me