rpc: Extract scriptPubKey on getreceivedbyaddress #16655

1. 
   konez2k commented at 2:16 pm on August 19, 2019: none

   Update rpc method getreceivedbyaddress to extract the correct scriptPubKey when shifted with opcodes (f.e. OP_CHECKSIG). getreceivedbylabel is already implemented in the same way.
2. RPC: Extract scriptPubKey on getreceivedbyaddress 5553f5a14e
3. fanquake added the label RPC/REST/ZMQ on Aug 19, 2019
4. 
   MarcoFalke commented at 2:20 pm on August 19, 2019: member

   What is this? refactoring or a bugfix?
5. 
   konez2k commented at 2:21 pm on August 19, 2019: none

   bugfix, the expected result from getreceivedbyaddress is different from getreceivedbylabel when the transaction scriptPubKey contains shifted opcodes
6. 
   MarcoFalke commented at 2:23 pm on August 19, 2019: member

   Would be nice to include a test that fails before the changes and passes after
7. 
   DrahtBot commented at 3:08 pm on August 19, 2019: member

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

   Conflicts

   No conflicts as of last run.

8. Test: Add RPC getreivedbyaddress functional test b70a54c3fa
9. Test: Remove getreceivedbyaddress unused imports 6af0709fd4
10. 
    konez2k commented at 4:27 pm on August 19, 2019: none

    @MarcoFalke The issue is mainly with legacy addresses when sending a transaction using address pubkey
11. 
    sipa commented at 4:44 pm on August 19, 2019: member

    This will make P2PK outputs show up under the address for the corresponding P2PKH. Is that desirable?
12. 
    konez2k commented at 4:48 pm on August 19, 2019: none

    @sipa it is under getreceivedbylabel, shouldn’t the output be equal between the 2 methods?
13. 
    sipa commented at 5:02 pm on August 19, 2019: member

    Ah, for consistency that does make sense.
14. Add test to test_runner.py 7828f9b0ff
15. 
    konez2k commented at 10:47 am on August 20, 2019: none

    feature_block.py works for me (patch included/excluded), the error is probably due to a timeout on trevis side

    Edit: works on appveyor too

16. in test/functional/rpc_getreceivedbyaddress.py:54 in 7828f9b0ff outdated

    49+                    }
    50+                }
    51+            ],
    52+            "hex": "02000000000100ca9a3b00000000232102352974bf0da1e6c4652888f014446572669cf7f9fd54d2c3b3d26ef60f15bd4cac00000000"
    53+        }
    54+        """
    

    ---

    ---

    

    MarcoFalke commented at 12:02 pm on August 20, 2019:

    I think there is no need to copy paste the full output of the command
17. Remove bitcoin-tx output b45caf85c8
18. fanquake renamed this:
    RPC: Extract scriptPubKey on getreceivedbyaddress
    rpc: Extract scriptPubKey on getreceivedbyaddress
    on Aug 21, 2019
19. 
    luke-jr commented at 7:50 pm on August 23, 2019: member

    Concept NACK: p2pk aren’t addresses, so this is actually introducing a bug.

    Labels are [at least effectively] assigned for both p2pk and p2pkh, which is why there is a discrepancy.

20. 
    NicolasDorier commented at 1:51 pm on August 26, 2019: contributor

    Concept NACK for same reason than @luke-jr and same reason as #16725 .

    I lost so much time explaining why the genesis transaction output should not appear as an address in block explorers, only to get replied “but why bitcoin core does it?”. This in general cause lot’s of confusion to new developers and end up in numerous bugs which only happen in production when a malicious actor sends a payment as P2PK to the the same pubkey as the expected P2PKH.

    This is worse than a bug, this is a virus, because it spreads a bad practice in the mind of new developers.

    Not only this, but P2PK is deprecated, so it does not make sense to add support to something that anyway should not be used anymore.

21. 
    meshcollider commented at 3:36 am on September 10, 2019: contributor

    @konez2k thanks for contributing! I think consistency is really valuable, but in this case as the others above have pointed out, it is probably consistency in the wrong direction. Unless anyone else is in support of this, I think its better to close it, but I hope that does not discourage you from contributing more in the future 😄
22. meshcollider closed this on Sep 10, 2019

    ---

23. DrahtBot locked this on Dec 16, 2021

Contributors
konez2k MarcoFalke DrahtBot sipa luke-jr NicolasDorier meshcollider

Labels
RPC/REST/ZMQ