rpc: Extract scriptPubKey on getreceivedbyaddress

[konez2k]

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.

[maflcko]

What is this? refactoring or a bugfix?

[konez2k]

bugfix, the expected result from getreceivedbyaddress is different from getreceivedbylabel when the transaction scriptPubKey contains shifted opcodes

[maflcko]

Would be nice to include a test that fails before the changes and passes after

[DrahtBot]

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

Conflicts

No conflicts as of last run.

[konez2k]

@MarcoFalke
The issue is mainly with legacy addresses when sending a transaction using address pubkey

[sipa]

This will make P2PK outputs show up under the address for the corresponding P2PKH. Is that desirable?

[konez2k]

@sipa it is under getreceivedbylabel, shouldn't the output be equal between the 2 methods?

[sipa]

Ah, for consistency that does make sense.

[konez2k]

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

[maflcko]

I think there is no need to copy paste the full output of the command

[luke-jr]

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.

[NicolasDorier]

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.

[meshcollider]

@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 😄