rpc: deprecate banscore field in getpeerinfo

[jonatack]

Per #19219 (comment) and #19219 (comment), this PR deprecates returning the banscore field in the getpeerinfo RPC, updates the help, adds a test, and updates the release notes. Related to #19464.

[maflcko]

reviewed on GitHub ACK 454a873

[laanwj]

Code review ACK 8c7647b

[amitiuttarwar]

ACK 8c7647b

[fanquake]

@laanwj & @amitiuttarwar: it would seem you've both ACK'd the wrong commit hash here?

Locally I'm seeing:

bitcoin git:(master) git fetch upstream pull/19469/head:19469 
remote: Enumerating objects: 8, done.
remote: Counting objects: 100% (8/8), done.
remote: Total 15 (delta 8), reused 8 (delta 8), pack-reused 7
Unpacking objects: 100% (15/15), 5.72 KiB | 234.00 KiB/s, done.
From https://github.com/bitcoin/bitcoin
 * [new ref]             refs/pull/19469/head -> 19469
bitcoin git:(master) git checkout 19469                      
Switched to branch '19469'
bitcoin git:(19469) git log
...
commit 41d55d30579358c805036201664ad6a1c1d48681 (HEAD -> 19469)
Author: Jon Atack <jon@atack.com>
Date:   Wed Jul 8 12:54:28 2020 +0200

    doc: getpeerinfo banscore deprecation release note

commit dd54e3796e633cfdf6954af306afd26eadc25116
Author: Jon Atack <jon@atack.com>
Date:   Wed Jul 8 12:33:14 2020 +0200

    test: getpeerinfo banscore deprecation test

commit 8c7647b3fbbab03ea84071cf3cd2d0d2bf8be255
Author: Jon Atack <jon@atack.com>
Date:   Wed Jul 8 13:00:58 2020 +0200

    rpc: deprecate banscore field in rpc getpeerinfo

Regardless, these changes are straightforward and I'm happy to go ahead and merge this.

Also cc @jnewbery & @sipa. I know you both expressed a desire for the removal of -banscore.

[fanquake]

ACK 41d55d3

[jnewbery]

This feels a little premature. I think it's not helped by a confusing collision in terminology:

• -banscore is a configuration option that we might better call "configurable discouragement threshold". That's what sipa and I were referring to in Replace automatic bans with discouragement filter #19219. It should be removed because we have no good advice for users on how to set it.
• banscore in getpeerinfo returns the peer's current nMisbehavior score. Longer term, it'd be good to replace nMisbehavior with prioritizing/scheduling peer work based on how much resource the peer is using (see https://bitcoincore.reviews/19109.html#l-99 and http://www.erisian.com.au/bitcoin-core-dev/log-2020-07-09.html#l-134).

Until we've actually removed the nMisbehavior functionality, it seems useful to be able to check a peer's nMisbehavior score.

[jonatack]

* `-banscore` ... should be removed because we have no good advice for users on how to set it.

Indeed, the first PR in this series, #19464, is a very small change that does just that. Presumably that PR, or another version, will be merged before the next release. I'm happy to work on internal peer prioritisation changes after that first step if people aren't already doing it.

The confusion around the naming of the banscore field in getpeerinfo was not addressed because it should be removed. In #19219 (comment), I suggested it be renamed.

This PR gives users until at least v0.22 to no longer depend on it (if any were). I reckon it will no longer be used internally before that, but in the worst case if this PR does turn out to be premature, it can be reverted before the next release.

[jnewbery]

Just realised I never responded to @jonatack's last comment. I don't think this should be reverted.

[jonatack]

Thanks @jnewbery. At the next p2p meeting, one topic to discuss could be if someone is working on replacing nMisbehavior with prioritizing/scheduling peer work based on how much resource the peer is using.

[jnewbery]

Sure. @troygiorshev is soon to open a PR that adds tracking for some peer metrics, which would be the start of that work.